juju-core

Merge lp:~thumper/juju-core/bootstrap-state-no-constraints-trunk into lp:~go-bot/juju-core/trunk

  1. bootstrap-state-no-constraints-trunk
  2. Merge into trunk
Proposed by Tim Penhey
Status: Merged
Merged at revision: 1982
Proposed branch: lp:~thumper/juju-core/bootstrap-state-no-constraints-trunk
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 102 lines (+74/-1)
2 files modified
environs/cloudinit/cloudinit.go (+5/-1)
environs/cloudinit/cloudinit_test.go (+69/-0)
To merge this branch: bzr merge lp:~thumper/juju-core/bootstrap-state-no-constraints-trunk
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+190851@code.launchpad.net

Commit message

Don't pass through empty constraints

If there are no constraints to pass through, cloud init has
  --constraints ''
It seems that somewhere in the process, the empty param is
getting missed and the following --debug param is getting
parsed as a constraint value.

The test is horrible, and all we really care about is the
jujud bootstrap-state line.

See also: lp:1239508 and lp:1239509

https://codereview.appspot.com/14494054/

Description of the change

Don't pass through empty constraints

If there are no constraints to pass through, cloud init has
  --constraints ''
It seems that somewhere in the process, the empty param is
getting missed and the following --debug param is getting
parsed as a constraint value.

The test is horrible, and all we really care about is the
jujud bootstrap-state line.

https://codereview.appspot.com/14494054/

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

Reviewers: mp+190851_code.launchpad.net,

Message:
Please take a look.

Description:
Don't pass through empty constraints

If there are no constraints to pass through, cloud init has
   --constraints ''
It seems that somewhere in the process, the empty param is
getting missed and the following --debug param is getting
parsed as a constraint value.

The test is horrible, and all we really care about is the
jujud bootstrap-state line.

https://code.launchpad.net/~thumper/juju-core/bootstrap-state-no-constraints/+merge/190851

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/14494054/

Affected files (+1542, -235 lines):
   A [revision details]
   M charm/repo.go
   M cmd/juju/addmachine.go
   M cmd/juju/addunit.go
   M cmd/juju/bootstrap.go
   M cmd/juju/bootstrap_test.go
   M cmd/juju/cmd_test.go
   M cmd/juju/constraints.go
   M cmd/juju/deploy.go
   M cmd/juju/get.go
   M cmd/juju/help_topics.go
   M cmd/juju/main.go
   M cmd/juju/main_test.go
   M cmd/juju/plugin.go
   M cmd/juju/plugin_test.go
   M cmd/output_test.go
   M cmd/plugins/juju-metadata/metadata.go
   M cmd/plugins/juju-metadata/metadataplugin_test.go
   A cmd/plugins/juju-metadata/signmetadata.go
   A cmd/plugins/juju-metadata/signmetadata_test.go
   M cmd/plugins/juju-metadata/validateimagemetadata.go
   M cmd/plugins/juju-metadata/validateimagemetadata_test.go
   M cmd/plugins/juju-metadata/validatetoolsmetadata.go
   M cmd/plugins/juju-metadata/validatetoolsmetadata_test.go
   M environs/cloudinit/cloudinit.go
   M environs/cloudinit/cloudinit_test.go
   M environs/config/config.go
   M environs/config/config_test.go
   M environs/configstore/disk.go
   M environs/filestorage/filestorage.go
   M environs/httpstorage/backend.go
   M environs/httpstorage/backend_test.go
   A environs/imagemetadata/export_test.go
   M environs/imagemetadata/simplestreams.go
   M environs/imagemetadata/simplestreams_test.go
   M environs/simplestreams/decode.go
   A environs/simplestreams/encode.go
   M environs/simplestreams/export_test.go
   M environs/simplestreams/simplestreams.go
   M environs/simplestreams/simplestreams_test.go
   M environs/simplestreams/testing/testing.go
   M environs/sshstorage/storage.go
   M environs/sshstorage/storage_test.go
   M environs/sync/sync.go
   M environs/sync/sync_test.go
   M environs/tools/export_test.go
   M environs/tools/simplestreams.go
   M environs/tools/simplestreams_test.go
   M juju/osenv/vars_nix.go
   M provider/maas/instance.go
   M provider/maas/instance_test.go
   M provider/openstack/provider.go
   A utils/file_unix.go
   A utils/file_windows.go
   M utils/fslock/fslock.go
   M utils/trivial.go
   A utils/zfile_windows.go
   M version/version.go
   M worker/addressupdater/machine_test.go
   M worker/addressupdater/updater.go
   M worker/uniter/jujuc/relation-get.go
   M worker/uniter/jujuc/relation-get_test.go

Revision history for this message
Tim Penhey (thumper) wrote :

Please take a look.

https://codereview.appspot.com/14494054/

Revision history for this message
Andrew Wilkins (axwalk) wrote :

LGTM. Should log a couple of bugs: one to investigate why the quotes are
lost, and one one to fix the testing situation.

https://codereview.appspot.com/14494054/diff/4001/environs/cloudinit/cloudinit_test.go
File environs/cloudinit/cloudinit_test.go (right):

https://codereview.appspot.com/14494054/diff/4001/environs/cloudinit/cloudinit_test.go#newcode123
environs/cloudinit/cloudinit_test.go:123: // NOTE: this is terrible,
only want to test part of the results...
Indeed :(

https://codereview.appspot.com/14494054/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'environs/cloudinit/cloudinit.go'
--- environs/cloudinit/cloudinit.go 2013-10-04 17:10:00 +0000
+++ environs/cloudinit/cloudinit.go 2013-10-14 03:14:27 +0000
@@ -221,13 +221,17 @@
221 if err != nil {221 if err != nil {
222 return nil, err222 return nil, err
223 }223 }
224 cons := cfg.Constraints.String()
225 if cons != "" {
226 cons = " --constraints " + shquote(cons)
227 }
224 c.AddScripts(228 c.AddScripts(
225 fmt.Sprintf("echo %s > %s", shquote(cfg.StateInfoURL), BootstrapStateURLFile),229 fmt.Sprintf("echo %s > %s", shquote(cfg.StateInfoURL), BootstrapStateURLFile),
226 // The bootstrapping is always run with debug on.230 // The bootstrapping is always run with debug on.
227 cfg.jujuTools()+"/jujud bootstrap-state"+231 cfg.jujuTools()+"/jujud bootstrap-state"+
228 " --data-dir "+shquote(cfg.DataDir)+232 " --data-dir "+shquote(cfg.DataDir)+
229 " --env-config "+shquote(base64yaml(cfg.Config))+233 " --env-config "+shquote(base64yaml(cfg.Config))+
230 " --constraints "+shquote(cfg.Constraints.String())+234 cons+
231 " --debug",235 " --debug",
232 "rm -rf "+shquote(acfg.Dir()),236 "rm -rf "+shquote(acfg.Dir()),
233 )237 )
234238
=== modified file 'environs/cloudinit/cloudinit_test.go'
--- environs/cloudinit/cloudinit_test.go 2013-10-03 09:45:30 +0000
+++ environs/cloudinit/cloudinit_test.go 2013-10-14 03:14:27 +0000
@@ -120,6 +120,75 @@
120start jujud-machine-0120start jujud-machine-0
121`,121`,
122 }, {122 }, {
123 // NOTE: this is terrible, only want to test part of the results...
124 // precise state server - no constraints
125 cfg: cloudinit.MachineConfig{
126 MachineId: "0",
127 AuthorizedKeys: "sshkey1",
128 AgentEnvironment: map[string]string{agent.ProviderType: "dummy"},
129 // precise currently needs mongo from PPA
130 Tools: newSimpleTools("1.2.3-precise-amd64"),
131 StateServer: true,
132 StateServerCert: serverCert,
133 StateServerKey: serverKey,
134 StatePort: 37017,
135 APIPort: 17070,
136 MachineNonce: "FAKE_NONCE",
137 StateInfo: &state.Info{
138 Password: "arble",
139 CACert: []byte("CA CERT\n" + testing.CACert),
140 },
141 APIInfo: &api.Info{
142 Password: "bletch",
143 CACert: []byte("CA CERT\n" + testing.CACert),
144 },
145 DataDir: environs.DataDir,
146 StateInfoURL: "some-url",
147 },
148 setEnvConfig: true,
149 expectScripts: `
150echo ENABLE_MONGODB="no" > /etc/default/mongodb
151set -xe
152mkdir -p /var/lib/juju
153mkdir -p /var/log/juju
154bin='/var/lib/juju/tools/1\.2\.3-precise-amd64'
155mkdir -p \$bin
156wget --no-verbose -O \$bin/tools\.tar\.gz 'http://foo\.com/tools/releases/juju1\.2\.3-precise-amd64\.tgz'
157sha256sum \$bin/tools\.tar\.gz > \$bin/juju1\.2\.3-precise-amd64\.sha256
158grep '1234' \$bin/juju1\.2\.3-precise-amd64.sha256 \|\| \(echo "Tools checksum mismatch"; exit 1\)
159tar zxf \$bin/tools.tar.gz -C \$bin
160rm \$bin/tools\.tar\.gz && rm \$bin/juju1\.2\.3-precise-amd64\.sha256
161printf %s '{"version":"1\.2\.3-precise-amd64","url":"http://foo\.com/tools/releases/juju1\.2\.3-precise-amd64\.tgz","sha256":"1234","size":10}' > \$bin/downloaded-tools\.txt
162install -m 600 /dev/null '/etc/rsyslog\.d/25-juju\.conf'
163printf '%s\\n' '\\n\$ModLoad imfile\\n\\n\$InputFileStateFile /var/spool/rsyslog/juju-machine-0-state\\n\$InputFilePersistStateInterval 50\\n\$InputFilePollInterval 5\\n\$InputFileName /var/log/juju/machine-0\.log\\n\$InputFileTag local-juju-machine-0:\\n\$InputFileStateFile machine-0\\n\$InputRunFileMonitor\\n\\n\$ModLoad imudp\\n\$UDPServerRun 514\\n\\n# Messages received from remote rsyslog machines contain a leading space so we\\n# need to account for that.\\n\$template JujuLogFormatLocal,\"%HOSTNAME%:%msg:::drop-last-lf%\\n\"\\n\$template JujuLogFormat,\"%HOSTNAME%:%msg:2:2048:drop-last-lf%\\n\"\\n\\n:syslogtag, startswith, \"juju-\" /var/log/juju/all-machines\.log;JujuLogFormat\\n& ~\\n:syslogtag, startswith, \"local-juju-\" /var/log/juju/all-machines\.log;JujuLogFormatLocal\\n& ~\\n' > '/etc/rsyslog\.d/25-juju\.conf'
164restart rsyslog
165mkdir -p '/var/lib/juju/agents/machine-0'
166install -m 644 /dev/null '/var/lib/juju/agents/machine-0/format'
167printf '%s\\n' '.*' > '/var/lib/juju/agents/machine-0/format'
168install -m 600 /dev/null '/var/lib/juju/agents/machine-0/agent\.conf'
169printf '%s\\n' '.*' > '/var/lib/juju/agents/machine-0/agent\.conf'
170install -m 600 /dev/null '/var/lib/juju/server\.pem'
171printf '%s\\n' 'SERVER CERT\\n[^']*SERVER KEY\\n[^']*' > '/var/lib/juju/server\.pem'
172mkdir -p /var/lib/juju/db/journal
173chmod 0700 /var/lib/juju/db
174dd bs=1M count=1 if=/dev/zero of=/var/lib/juju/db/journal/prealloc\.0
175dd bs=1M count=1 if=/dev/zero of=/var/lib/juju/db/journal/prealloc\.1
176dd bs=1M count=1 if=/dev/zero of=/var/lib/juju/db/journal/prealloc\.2
177cat >> /etc/init/juju-db\.conf << 'EOF'\\ndescription "juju state database"\\nauthor "Juju Team <juju@lists\.ubuntu\.com>"\\nstart on runlevel \[2345\]\\nstop on runlevel \[!2345\]\\nrespawn\\nnormal exit 0\\n\\nlimit nofile 65000 65000\\nlimit nproc 20000 20000\\n\\nexec /usr/bin/mongod --auth --dbpath=/var/lib/juju/db --sslOnNormalPorts --sslPEMKeyFile '/var/lib/juju/server\.pem' --sslPEMKeyPassword ignored --bind_ip 0\.0\.0\.0 --port 37017 --noprealloc --syslog --smallfiles\\nEOF\\n
178start juju-db
179mkdir -p '/var/lib/juju/agents/bootstrap'
180install -m 644 /dev/null '/var/lib/juju/agents/bootstrap/format'
181printf '%s\\n' '.*' > '/var/lib/juju/agents/bootstrap/format'
182install -m 600 /dev/null '/var/lib/juju/agents/bootstrap/agent\.conf'
183printf '%s\\n' '.*' > '/var/lib/juju/agents/bootstrap/agent\.conf'
184echo 'some-url' > /tmp/provider-state-url
185/var/lib/juju/tools/1\.2\.3-precise-amd64/jujud bootstrap-state --data-dir '/var/lib/juju' --env-config '[^']*' --debug
186rm -rf '/var/lib/juju/agents/bootstrap'
187ln -s 1\.2\.3-precise-amd64 '/var/lib/juju/tools/machine-0'
188cat >> /etc/init/jujud-machine-0\.conf << 'EOF'\\ndescription "juju machine-0 agent"\\nauthor "Juju Team <juju@lists\.ubuntu\.com>"\\nstart on runlevel \[2345\]\\nstop on runlevel \[!2345\]\\nrespawn\\nnormal exit 0\\n\\nlimit nofile 20000 20000\\n\\nexec /var/lib/juju/tools/machine-0/jujud machine --data-dir '/var/lib/juju' --machine-id 0 --debug >> /var/log/juju/machine-0\.log 2>&1\\nEOF\\n
189start jujud-machine-0
190`,
191 }, {
123 // raring state server192 // raring state server
124 cfg: cloudinit.MachineConfig{193 cfg: cloudinit.MachineConfig{
125 MachineId: "0",194 MachineId: "0",

Subscribers

People subscribed via source and target branches

to status/vote changes: