Merge lp:~thumper/juju-core/bootstrap-state-no-constraints into lp:juju-core/1.16

Proposed by Tim Penhey
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merged at revision: 1972
Proposed branch: lp:~thumper/juju-core/bootstrap-state-no-constraints
Merge into: lp:juju-core/1.16
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
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+190852@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/14516056/

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.

See also: lp:1239508 and lp:1239509

https://codereview.appspot.com/14516056/

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

Reviewers: mp+190852_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.

See also: lp:1239508 and lp:1239509

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

(do not edit description out of merge proposal)

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

Affected files (+76, -1 lines):
   A [revision details]
   M environs/cloudinit/cloudinit.go
   M environs/cloudinit/cloudinit_test.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20131010031125-8lgphpzdl93berr7
+New revision: <email address hidden>

Index: environs/cloudinit/cloudinit.go
=== modified file 'environs/cloudinit/cloudinit.go'
--- environs/cloudinit/cloudinit.go 2013-10-03 09:45:30 +0000
+++ environs/cloudinit/cloudinit.go 2013-10-14 03:13:55 +0000
@@ -217,13 +217,17 @@
    if err != nil {
     return nil, err
    }
+ cons := cfg.Constraints.String()
+ if cons != "" {
+ cons = " --constraints " + shquote(cons)
+ }
    c.AddScripts(
     fmt.Sprintf("echo %s > %s", shquote(cfg.StateInfoURL),
BootstrapStateURLFile),
     // The bootstrapping is always run with debug on.
     cfg.jujuTools()+"/jujud bootstrap-state"+
      " --data-dir "+shquote(cfg.DataDir)+
      " --env-config "+shquote(base64yaml(cfg.Config))+
- " --constraints "+shquote(cfg.Constraints.String())+
+ cons+
      " --debug",
     "rm -rf "+shquote(acfg.Dir()),
    )

Index: environs/cloudinit/cloudinit_test.go
=== 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:13:55 +0000
@@ -120,6 +120,75 @@
  start jujud-machine-0
  `,
   }, {
+ // NOTE: this is terrible, only want to test part of the results...
+ // precise state server - no constraints
+ cfg: cloudinit.MachineConfig{
+ MachineId: "0",
+ AuthorizedKeys: "sshkey1",
+ AgentEnvironment: map[string]string{agent.ProviderType: "dummy"},
+ // precise currently needs mongo from PPA
+ Tools: newSimpleTools("1.2.3-precise-amd64"),
+ StateServer: true,
+ StateServerCert: serverCert,
+ StateServerKey: serverKey,
+ StatePort: 37017,
+ APIPort: 17070,
+ MachineNonce: "FAKE_NONCE",
+ StateInfo: &state.Info{
+ Password: "arble",
+ CACert: []byte("CA CERT\n" + testing.CACert),
+ },
+ APIInfo: &api.Info{
+ Password: "bletch",
+ CACert: []byte("CA CERT\n" + testing.CACert),
+ },
+ DataDir: environs.DataDir,
+ StateInfoURL: "some-url",
+ },
+ setEnvConfig: true,
+ expectScripts: `
+echo ENABLE_MONGODB="no" > /etc/default/mongodb
+set -xe
...

Read more...

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

On 2013/10/14 04:12:35, thumper wrote:
> Please take a look.

LGTM

https://codereview.appspot.com/14516056/

Revision history for this message
Go Bot (go-bot) wrote :
Download full text (6.7 KiB)

The attempt to merge lp:~thumper/juju-core/bootstrap-state-no-constraints into lp:juju-core/1.16 failed. Below is the output from the failed tests.

ok launchpad.net/juju-core/agent 0.388s
ok launchpad.net/juju-core/agent/tools 0.222s
ok launchpad.net/juju-core/bzr 6.855s
ok launchpad.net/juju-core/cert 3.131s
ok launchpad.net/juju-core/charm 0.547s
? launchpad.net/juju-core/charm/hooks [no test files]
ok launchpad.net/juju-core/cloudinit 0.026s
ok launchpad.net/juju-core/cmd 0.200s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/juju 171.130s
ok launchpad.net/juju-core/cmd/jujud 48.122s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 2.460s
ok launchpad.net/juju-core/constraints 0.049s
ok launchpad.net/juju-core/container/lxc 0.279s
? launchpad.net/juju-core/container/lxc/mock [no test files]
ok launchpad.net/juju-core/downloader 5.307s
ok launchpad.net/juju-core/environs 3.183s
ok launchpad.net/juju-core/environs/bootstrap 5.001s
ok launchpad.net/juju-core/environs/cloudinit 0.534s
ok launchpad.net/juju-core/environs/config 0.806s
ok launchpad.net/juju-core/environs/configstore 0.043s
ok launchpad.net/juju-core/environs/filestorage 0.031s
ok launchpad.net/juju-core/environs/httpstorage 0.929s
ok launchpad.net/juju-core/environs/imagemetadata 0.500s
ok launchpad.net/juju-core/environs/instances 0.051s
ok launchpad.net/juju-core/environs/jujutest 0.224s
ok launchpad.net/juju-core/environs/manual 4.450s
ok launchpad.net/juju-core/environs/simplestreams 0.318s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 1.225s
ok launchpad.net/juju-core/environs/storage 1.102s
ok launchpad.net/juju-core/environs/sync 28.518s
ok launchpad.net/juju-core/environs/testing 0.190s
ok launchpad.net/juju-core/environs/tools 6.919s
? launchpad.net/juju-core/environs/tools/testing [no test files]
ok launchpad.net/juju-core/errors 0.016s
ok launchpad.net/juju-core/instance 0.022s
? launchpad.net/juju-core/instance/testing [no test files]
ok launchpad.net/juju-core/juju 17.846s
ok launchpad.net/juju-core/juju/osenv 0.018s
? launchpad.net/juju-core/juju/testing [no test files]
ok launchpad.net/juju-core/log 0.015s
ok launchpad.net/juju-core/log/syslog 0.024s
ok launchpad.net/juju-core/names 0.024s
? launchpad.net/juju-core/provider [no test files]
? launchpad.net/juju-core/provider/all [no test files]
ok launchpad.net/juju-core/provider/azure 6.635s
ok launchpad.net/juju-core/provider/common 0.334s
ok launchpad.net/juju-core/provider/dummy 21.272s
ok launchpad.net/juju-core/provider/ec2 5.418s
ok launchpad.net/juju-core/provider/ec2/httpstorage 0.203s
ok launchpad.net/juju-core/provider/local 2.238s
ok launchpad.net/juju-core/provider/maas 3.712s
ok launchpad.net/juju-core/provider/null 1.198s
ok launchpad.net/juju-core/provider/openstack 13.582s
ok launchpad.net/juju-core/rpc 0.085s
ok launchpad.net/juju-core/rpc/jsoncodec 0.028s
? launchpad.net/juju-core/rpc/rpcreflect [no te...

Read more...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'environs/cloudinit/cloudinit.go'
2--- environs/cloudinit/cloudinit.go 2013-10-03 09:45:30 +0000
3+++ environs/cloudinit/cloudinit.go 2013-10-14 04:10:39 +0000
4@@ -217,13 +217,17 @@
5 if err != nil {
6 return nil, err
7 }
8+ cons := cfg.Constraints.String()
9+ if cons != "" {
10+ cons = " --constraints " + shquote(cons)
11+ }
12 c.AddScripts(
13 fmt.Sprintf("echo %s > %s", shquote(cfg.StateInfoURL), BootstrapStateURLFile),
14 // The bootstrapping is always run with debug on.
15 cfg.jujuTools()+"/jujud bootstrap-state"+
16 " --data-dir "+shquote(cfg.DataDir)+
17 " --env-config "+shquote(base64yaml(cfg.Config))+
18- " --constraints "+shquote(cfg.Constraints.String())+
19+ cons+
20 " --debug",
21 "rm -rf "+shquote(acfg.Dir()),
22 )
23
24=== modified file 'environs/cloudinit/cloudinit_test.go'
25--- environs/cloudinit/cloudinit_test.go 2013-10-03 09:45:30 +0000
26+++ environs/cloudinit/cloudinit_test.go 2013-10-14 04:10:39 +0000
27@@ -120,6 +120,75 @@
28 start jujud-machine-0
29 `,
30 }, {
31+ // NOTE: this is terrible, only want to test part of the results...
32+ // precise state server - no constraints
33+ cfg: cloudinit.MachineConfig{
34+ MachineId: "0",
35+ AuthorizedKeys: "sshkey1",
36+ AgentEnvironment: map[string]string{agent.ProviderType: "dummy"},
37+ // precise currently needs mongo from PPA
38+ Tools: newSimpleTools("1.2.3-precise-amd64"),
39+ StateServer: true,
40+ StateServerCert: serverCert,
41+ StateServerKey: serverKey,
42+ StatePort: 37017,
43+ APIPort: 17070,
44+ MachineNonce: "FAKE_NONCE",
45+ StateInfo: &state.Info{
46+ Password: "arble",
47+ CACert: []byte("CA CERT\n" + testing.CACert),
48+ },
49+ APIInfo: &api.Info{
50+ Password: "bletch",
51+ CACert: []byte("CA CERT\n" + testing.CACert),
52+ },
53+ DataDir: environs.DataDir,
54+ StateInfoURL: "some-url",
55+ },
56+ setEnvConfig: true,
57+ expectScripts: `
58+echo ENABLE_MONGODB="no" > /etc/default/mongodb
59+set -xe
60+mkdir -p /var/lib/juju
61+mkdir -p /var/log/juju
62+bin='/var/lib/juju/tools/1\.2\.3-precise-amd64'
63+mkdir -p \$bin
64+wget --no-verbose -O \$bin/tools\.tar\.gz 'http://foo\.com/tools/releases/juju1\.2\.3-precise-amd64\.tgz'
65+sha256sum \$bin/tools\.tar\.gz > \$bin/juju1\.2\.3-precise-amd64\.sha256
66+grep '1234' \$bin/juju1\.2\.3-precise-amd64.sha256 \|\| \(echo "Tools checksum mismatch"; exit 1\)
67+tar zxf \$bin/tools.tar.gz -C \$bin
68+rm \$bin/tools\.tar\.gz && rm \$bin/juju1\.2\.3-precise-amd64\.sha256
69+printf %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
70+install -m 600 /dev/null '/etc/rsyslog\.d/25-juju\.conf'
71+printf '%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'
72+restart rsyslog
73+mkdir -p '/var/lib/juju/agents/machine-0'
74+install -m 644 /dev/null '/var/lib/juju/agents/machine-0/format'
75+printf '%s\\n' '.*' > '/var/lib/juju/agents/machine-0/format'
76+install -m 600 /dev/null '/var/lib/juju/agents/machine-0/agent\.conf'
77+printf '%s\\n' '.*' > '/var/lib/juju/agents/machine-0/agent\.conf'
78+install -m 600 /dev/null '/var/lib/juju/server\.pem'
79+printf '%s\\n' 'SERVER CERT\\n[^']*SERVER KEY\\n[^']*' > '/var/lib/juju/server\.pem'
80+mkdir -p /var/lib/juju/db/journal
81+chmod 0700 /var/lib/juju/db
82+dd bs=1M count=1 if=/dev/zero of=/var/lib/juju/db/journal/prealloc\.0
83+dd bs=1M count=1 if=/dev/zero of=/var/lib/juju/db/journal/prealloc\.1
84+dd bs=1M count=1 if=/dev/zero of=/var/lib/juju/db/journal/prealloc\.2
85+cat >> /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
86+start juju-db
87+mkdir -p '/var/lib/juju/agents/bootstrap'
88+install -m 644 /dev/null '/var/lib/juju/agents/bootstrap/format'
89+printf '%s\\n' '.*' > '/var/lib/juju/agents/bootstrap/format'
90+install -m 600 /dev/null '/var/lib/juju/agents/bootstrap/agent\.conf'
91+printf '%s\\n' '.*' > '/var/lib/juju/agents/bootstrap/agent\.conf'
92+echo 'some-url' > /tmp/provider-state-url
93+/var/lib/juju/tools/1\.2\.3-precise-amd64/jujud bootstrap-state --data-dir '/var/lib/juju' --env-config '[^']*' --debug
94+rm -rf '/var/lib/juju/agents/bootstrap'
95+ln -s 1\.2\.3-precise-amd64 '/var/lib/juju/tools/machine-0'
96+cat >> /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
97+start jujud-machine-0
98+`,
99+ }, {
100 // raring state server
101 cfg: cloudinit.MachineConfig{
102 MachineId: "0",

Subscribers

People subscribed via source and target branches

to all changes: