Merge lp:~axwalk/juju-core/lp1291292-jujud-bootstrap-nostateurl into lp:~go-bot/juju-core/trunk
- lp1291292-jujud-bootstrap-nostateurl
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Andrew Wilkins |
Approved revision: | no longer in the source branch. |
Merged at revision: | 2464 |
Proposed branch: | lp:~axwalk/juju-core/lp1291292-jujud-bootstrap-nostateurl |
Merge into: | lp:~go-bot/juju-core/trunk |
Diff against target: |
791 lines (+138/-188) 14 files modified
cloudinit/sshinit/configure_test.go (+2/-1) cmd/jujud/bootstrap.go (+10/-25) cmd/jujud/bootstrap_test.go (+72/-52) cmd/jujud/main_test.go (+1/-0) environs/bootstrap/state.go (+0/-20) environs/bootstrap/state_test.go (+0/-27) environs/cloudinit.go (+1/-3) environs/cloudinit/cloudinit.go (+20/-10) environs/cloudinit/cloudinit_test.go (+11/-8) environs/manual/bootstrap.go (+3/-2) instance/instance.go (+10/-0) provider/common/bootstrap.go (+3/-9) provider/common/bootstrap_test.go (+3/-26) provider/local/environ.go (+2/-5) |
To merge this branch: | bzr merge lp:~axwalk/juju-core/lp1291292-jujud-bootstrap-nostateurl |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju Engineering | Pending | ||
Review via email: mp+212172@code.launchpad.net |
Commit message
Pass instance-
The bootstrap-state agent currently loads its
instance ID and hardware characteristics by
fetching a URL that is written to a file on
disk. This is no longer necessary with
synchronous bootstrap.
I have changed jujud and bootstrap script
generation to pass the instance-id and hardware
characteristics directly to jujud as command
line arguments.
Fixes part of lp:1291292
Description of the change
Pass instance-
The bootstrap-state agent currently loads its
instance ID and hardware characteristics by
fetching a URL that is written to a file on
disk. This is no longer necessary with
synchronous bootstrap.
I have changed jujud and bootstrap script
generation to pass the instance-id and hardware
characteristics directly to jujud as command
line arguments.
Fixes part of lp:1291292
Andrew Wilkins (axwalk) wrote : | # |
Dimiter Naydenov (dimitern) wrote : | # |
This is great, LGTM, assuming you tested it live.
https:/
File cmd/jujud/
https:/
cmd/jujud/
{
s/=/:=/
Andrew Wilkins (axwalk) wrote : | # |
Please take a look.
https:/
File cmd/jujud/
https:/
cmd/jujud/
{
On 2014/03/21 15:03:24, dimitern wrote:
> s/=/:=/
Done.
Andrew Wilkins (axwalk) wrote : | # |
Please take a look.
Andrew Wilkins (axwalk) wrote : | # |
On 2014/03/21 15:03:23, dimitern wrote:
> This is great, LGTM, assuming you tested it live.
Thanks, yes I have tested with local, manual and openstack.
I realised I could remove LoadStateFromURL altogether, so I went ahead
and did that.
> https:/
> File cmd/jujud/
https:/
> cmd/jujud/
nil {
> s/=/:=/
Preview Diff
1 | === modified file 'cloudinit/sshinit/configure_test.go' |
2 | --- cloudinit/sshinit/configure_test.go 2014-03-06 12:14:26 +0000 |
3 | +++ cloudinit/sshinit/configure_test.go 2014-03-24 01:53:33 +0000 |
4 | @@ -55,7 +55,8 @@ |
5 | func (s *configureSuite) getCloudConfig(c *gc.C, stateServer bool, vers version.Binary) *cloudinit.Config { |
6 | var mcfg *envcloudinit.MachineConfig |
7 | if stateServer { |
8 | - mcfg = environs.NewBootstrapMachineConfig("http://whatever/dotcom", "private-key") |
9 | + mcfg = environs.NewBootstrapMachineConfig("private-key") |
10 | + mcfg.InstanceId = "instance-id" |
11 | mcfg.Jobs = []params.MachineJob{params.JobManageEnviron, params.JobHostUnits} |
12 | } else { |
13 | mcfg = environs.NewMachineConfig("0", "ya", nil, nil) |
14 | |
15 | === modified file 'cmd/jujud/bootstrap.go' |
16 | --- cmd/jujud/bootstrap.go 2014-03-05 16:30:01 +0000 |
17 | +++ cmd/jujud/bootstrap.go 2014-03-24 01:53:33 +0000 |
18 | @@ -6,8 +6,6 @@ |
19 | import ( |
20 | "encoding/base64" |
21 | "fmt" |
22 | - "io/ioutil" |
23 | - "strings" |
24 | |
25 | "launchpad.net/gnuflag" |
26 | "launchpad.net/goyaml" |
27 | @@ -16,23 +14,19 @@ |
28 | "launchpad.net/juju-core/cmd" |
29 | "launchpad.net/juju-core/constraints" |
30 | "launchpad.net/juju-core/environs" |
31 | - "launchpad.net/juju-core/environs/bootstrap" |
32 | - "launchpad.net/juju-core/environs/cloudinit" |
33 | "launchpad.net/juju-core/environs/config" |
34 | "launchpad.net/juju-core/instance" |
35 | "launchpad.net/juju-core/state" |
36 | "launchpad.net/juju-core/state/api/params" |
37 | ) |
38 | |
39 | -// Cloud-init write the URL to be used to load the bootstrap state into this file. |
40 | -// A variable is used here to allow tests to override. |
41 | -var providerStateURLFile = cloudinit.BootstrapStateURLFile |
42 | - |
43 | type BootstrapCommand struct { |
44 | cmd.CommandBase |
45 | Conf AgentConf |
46 | EnvConfig map[string]interface{} |
47 | Constraints constraints.Value |
48 | + Hardware instance.HardwareCharacteristics |
49 | + InstanceId string |
50 | } |
51 | |
52 | // Info returns a decription of the command. |
53 | @@ -47,6 +41,8 @@ |
54 | c.Conf.addFlags(f) |
55 | yamlBase64Var(f, &c.EnvConfig, "env-config", "", "initial environment configuration (yaml, base64 encoded)") |
56 | f.Var(constraints.ConstraintsValue{&c.Constraints}, "constraints", "initial environment constraints (space-separated strings)") |
57 | + f.Var(&c.Hardware, "hardware", "hardware characteristics (space-separated strings)") |
58 | + f.StringVar(&c.InstanceId, "instance-id", "", "unique instance-id for bootstrap machine") |
59 | } |
60 | |
61 | // Init initializes the command for running. |
62 | @@ -54,26 +50,19 @@ |
63 | if len(c.EnvConfig) == 0 { |
64 | return requiredError("env-config") |
65 | } |
66 | + if c.InstanceId == "" { |
67 | + return requiredError("instance-id") |
68 | + } |
69 | return c.Conf.checkArgs(args) |
70 | } |
71 | |
72 | // Run initializes state for an environment. |
73 | func (c *BootstrapCommand) Run(_ *cmd.Context) error { |
74 | - data, err := ioutil.ReadFile(providerStateURLFile) |
75 | - if err != nil { |
76 | - return fmt.Errorf("cannot read provider-state-url file: %v", err) |
77 | - } |
78 | envCfg, err := config.New(config.NoDefaults, c.EnvConfig) |
79 | if err != nil { |
80 | return err |
81 | } |
82 | - stateInfoURL := strings.Split(string(data), "\n")[0] |
83 | - bsState, err := bootstrap.LoadStateFromURL(stateInfoURL, !envCfg.SSLHostnameVerification()) |
84 | - if err != nil { |
85 | - return fmt.Errorf("cannot load state from URL %q (read from %q): %v", stateInfoURL, providerStateURLFile, err) |
86 | - } |
87 | - err = c.Conf.read("machine-0") |
88 | - if err != nil { |
89 | + if err := c.Conf.read("machine-0"); err != nil { |
90 | return err |
91 | } |
92 | // agent.Jobs is an optional field in the agent config, and was |
93 | @@ -86,15 +75,11 @@ |
94 | params.JobHostUnits, |
95 | } |
96 | } |
97 | - var characteristics instance.HardwareCharacteristics |
98 | - if len(bsState.Characteristics) > 0 { |
99 | - characteristics = bsState.Characteristics[0] |
100 | - } |
101 | st, _, err := c.Conf.config.InitializeState(envCfg, agent.BootstrapMachineConfig{ |
102 | Constraints: c.Constraints, |
103 | Jobs: jobs, |
104 | - InstanceId: bsState.StateInstances[0], |
105 | - Characteristics: characteristics, |
106 | + InstanceId: instance.Id(c.InstanceId), |
107 | + Characteristics: c.Hardware, |
108 | }, state.DefaultDialOpts(), environs.NewStatePolicy()) |
109 | if err != nil { |
110 | return err |
111 | |
112 | === modified file 'cmd/jujud/bootstrap_test.go' |
113 | --- cmd/jujud/bootstrap_test.go 2014-03-13 07:54:56 +0000 |
114 | +++ cmd/jujud/bootstrap_test.go 2014-03-24 01:53:33 +0000 |
115 | @@ -5,8 +5,6 @@ |
116 | |
117 | import ( |
118 | "encoding/base64" |
119 | - "io/ioutil" |
120 | - "path/filepath" |
121 | |
122 | jc "github.com/juju/testing/checkers" |
123 | gc "launchpad.net/gocheck" |
124 | @@ -15,8 +13,6 @@ |
125 | "launchpad.net/juju-core/agent" |
126 | "launchpad.net/juju-core/constraints" |
127 | "launchpad.net/juju-core/environs" |
128 | - "launchpad.net/juju-core/environs/bootstrap" |
129 | - "launchpad.net/juju-core/environs/jujutest" |
130 | "launchpad.net/juju-core/errors" |
131 | "launchpad.net/juju-core/instance" |
132 | "launchpad.net/juju-core/provider/dummy" |
133 | @@ -33,32 +29,15 @@ |
134 | type BootstrapSuite struct { |
135 | testbase.LoggingSuite |
136 | testing.MgoSuite |
137 | - dataDir string |
138 | - logDir string |
139 | - providerStateURLFile string |
140 | + dataDir string |
141 | + logDir string |
142 | } |
143 | |
144 | var _ = gc.Suite(&BootstrapSuite{}) |
145 | |
146 | -var testRoundTripper = &jujutest.ProxyRoundTripper{} |
147 | - |
148 | -func init() { |
149 | - // Prepare mock http transport for provider-state output in tests. |
150 | - testRoundTripper.RegisterForScheme("test") |
151 | -} |
152 | - |
153 | func (s *BootstrapSuite) SetUpSuite(c *gc.C) { |
154 | s.LoggingSuite.SetUpSuite(c) |
155 | s.MgoSuite.SetUpSuite(c) |
156 | - stateInfo := bootstrap.BootstrapState{ |
157 | - StateInstances: []instance.Id{instance.Id("dummy.instance.id")}, |
158 | - } |
159 | - stateData, err := goyaml.Marshal(stateInfo) |
160 | - c.Assert(err, gc.IsNil) |
161 | - content := map[string]string{"/" + bootstrap.StateFile: string(stateData)} |
162 | - testRoundTripper.Sub = jujutest.NewCannedRoundTripper(content, nil) |
163 | - s.providerStateURLFile = filepath.Join(c.MkDir(), "provider-state-url") |
164 | - providerStateURLFile = s.providerStateURLFile |
165 | } |
166 | |
167 | func (s *BootstrapSuite) TearDownSuite(c *gc.C) { |
168 | @@ -85,7 +64,6 @@ |
169 | } |
170 | |
171 | func (s *BootstrapSuite) initBootstrapCommand(c *gc.C, jobs []params.MachineJob, args ...string) (machineConf agent.Config, cmd *BootstrapCommand, err error) { |
172 | - ioutil.WriteFile(s.providerStateURLFile, []byte("test://localhost/provider-state\n"), 0600) |
173 | if len(jobs) == 0 { |
174 | // Add default jobs. |
175 | jobs = []params.MachineJob{ |
176 | @@ -123,7 +101,8 @@ |
177 | } |
178 | |
179 | func (s *BootstrapSuite) TestInitializeEnvironment(c *gc.C) { |
180 | - _, cmd, err := s.initBootstrapCommand(c, nil, "--env-config", testConfig) |
181 | + hw := instance.MustParseHardware("arch=amd64 mem=8G") |
182 | + _, cmd, err := s.initBootstrapCommand(c, nil, "--env-config", testConfig, "--instance-id", "anything", "--hardware", hw.String()) |
183 | c.Assert(err, gc.IsNil) |
184 | err = cmd.Run(nil) |
185 | c.Assert(err, gc.IsNil) |
186 | @@ -141,7 +120,12 @@ |
187 | |
188 | instid, err := machines[0].InstanceId() |
189 | c.Assert(err, gc.IsNil) |
190 | - c.Assert(instid, gc.Equals, instance.Id("dummy.instance.id")) |
191 | + c.Assert(instid, gc.Equals, instance.Id("anything")) |
192 | + |
193 | + stateHw, err := machines[0].HardwareCharacteristics() |
194 | + c.Assert(err, gc.IsNil) |
195 | + c.Assert(stateHw, gc.NotNil) |
196 | + c.Assert(*stateHw, gc.DeepEquals, hw) |
197 | |
198 | cons, err := st.EnvironConstraints() |
199 | c.Assert(err, gc.IsNil) |
200 | @@ -150,7 +134,11 @@ |
201 | |
202 | func (s *BootstrapSuite) TestSetConstraints(c *gc.C) { |
203 | tcons := constraints.Value{Mem: uint64p(2048), CpuCores: uint64p(2)} |
204 | - _, cmd, err := s.initBootstrapCommand(c, nil, "--env-config", testConfig, "--constraints", tcons.String()) |
205 | + _, cmd, err := s.initBootstrapCommand(c, nil, |
206 | + "--env-config", testConfig, |
207 | + "--instance-id", "anything", |
208 | + "--constraints", tcons.String(), |
209 | + ) |
210 | c.Assert(err, gc.IsNil) |
211 | err = cmd.Run(nil) |
212 | c.Assert(err, gc.IsNil) |
213 | @@ -182,7 +170,7 @@ |
214 | expectedJobs := []state.MachineJob{ |
215 | state.JobManageEnviron, state.JobHostUnits, |
216 | } |
217 | - _, cmd, err := s.initBootstrapCommand(c, nil, "--env-config", testConfig) |
218 | + _, cmd, err := s.initBootstrapCommand(c, nil, "--env-config", testConfig, "--instance-id", "anything") |
219 | c.Assert(err, gc.IsNil) |
220 | err = cmd.Run(nil) |
221 | c.Assert(err, gc.IsNil) |
222 | @@ -201,7 +189,7 @@ |
223 | |
224 | func (s *BootstrapSuite) TestConfiguredMachineJobs(c *gc.C) { |
225 | jobs := []params.MachineJob{params.JobManageEnviron} |
226 | - _, cmd, err := s.initBootstrapCommand(c, jobs, "--env-config", testConfig) |
227 | + _, cmd, err := s.initBootstrapCommand(c, jobs, "--env-config", testConfig, "--instance-id", "anything") |
228 | c.Assert(err, gc.IsNil) |
229 | err = cmd.Run(nil) |
230 | c.Assert(err, gc.IsNil) |
231 | @@ -231,7 +219,7 @@ |
232 | } |
233 | |
234 | func (s *BootstrapSuite) TestInitialPassword(c *gc.C) { |
235 | - machineConf, cmd, err := s.initBootstrapCommand(c, nil, "--env-config", testConfig) |
236 | + machineConf, cmd, err := s.initBootstrapCommand(c, nil, "--env-config", testConfig, "--instance-id", "anything") |
237 | c.Assert(err, gc.IsNil) |
238 | |
239 | err = cmd.Run(nil) |
240 | @@ -270,35 +258,65 @@ |
241 | defer st.Close() |
242 | } |
243 | |
244 | -var base64ConfigTests = []struct { |
245 | - input []string |
246 | - err string |
247 | - expected map[string]interface{} |
248 | +var bootstrapArgTests = []struct { |
249 | + input []string |
250 | + err string |
251 | + expectedInstanceId string |
252 | + expectedHardware instance.HardwareCharacteristics |
253 | + expectedConfig map[string]interface{} |
254 | }{ |
255 | { |
256 | - // no value supplied |
257 | - nil, |
258 | - "--env-config option must be set", |
259 | - nil, |
260 | + // no value supplied for env-config |
261 | + err: "--env-config option must be set", |
262 | }, { |
263 | - // empty |
264 | - []string{"--env-config", ""}, |
265 | - "--env-config option must be set", |
266 | - nil, |
267 | + // empty env-config |
268 | + input: []string{"--env-config", ""}, |
269 | + err: "--env-config option must be set", |
270 | }, { |
271 | // wrong, should be base64 |
272 | - []string{"--env-config", "name: banana\n"}, |
273 | - ".*illegal base64 data at input byte.*", |
274 | - nil, |
275 | - }, { |
276 | - []string{"--env-config", base64.StdEncoding.EncodeToString([]byte("name: banana\n"))}, |
277 | - "", |
278 | - map[string]interface{}{"name": "banana"}, |
279 | + input: []string{"--env-config", "name: banana\n"}, |
280 | + err: ".*illegal base64 data at input byte.*", |
281 | + }, { |
282 | + // no value supplied for instance-id |
283 | + input: []string{ |
284 | + "--env-config", base64.StdEncoding.EncodeToString([]byte("name: banana\n")), |
285 | + }, |
286 | + err: "--instance-id option must be set", |
287 | + }, { |
288 | + // empty instance-id |
289 | + input: []string{ |
290 | + "--env-config", base64.StdEncoding.EncodeToString([]byte("name: banana\n")), |
291 | + "--instance-id", "", |
292 | + }, |
293 | + err: "--instance-id option must be set", |
294 | + }, { |
295 | + input: []string{ |
296 | + "--env-config", base64.StdEncoding.EncodeToString([]byte("name: banana\n")), |
297 | + "--instance-id", "anything", |
298 | + }, |
299 | + expectedInstanceId: "anything", |
300 | + expectedConfig: map[string]interface{}{"name": "banana"}, |
301 | + }, { |
302 | + input: []string{ |
303 | + "--env-config", base64.StdEncoding.EncodeToString([]byte("name: banana\n")), |
304 | + "--instance-id", "anything", |
305 | + "--hardware", "nonsense", |
306 | + }, |
307 | + err: `invalid value "nonsense" for flag --hardware: malformed characteristic "nonsense"`, |
308 | + }, { |
309 | + input: []string{ |
310 | + "--env-config", base64.StdEncoding.EncodeToString([]byte("name: banana\n")), |
311 | + "--instance-id", "anything", |
312 | + "--hardware", "arch=amd64 cpu-cores=4 root-disk=2T", |
313 | + }, |
314 | + expectedInstanceId: "anything", |
315 | + expectedHardware: instance.MustParseHardware("arch=amd64 cpu-cores=4 root-disk=2T"), |
316 | + expectedConfig: map[string]interface{}{"name": "banana"}, |
317 | }, |
318 | } |
319 | |
320 | -func (s *BootstrapSuite) TestBase64Config(c *gc.C) { |
321 | - for i, t := range base64ConfigTests { |
322 | +func (s *BootstrapSuite) TestBootstrapArgs(c *gc.C) { |
323 | + for i, t := range bootstrapArgTests { |
324 | c.Logf("test %d", i) |
325 | var args []string |
326 | args = append(args, t.input...) |
327 | @@ -306,7 +324,9 @@ |
328 | if t.err == "" { |
329 | c.Assert(cmd, gc.NotNil) |
330 | c.Assert(err, gc.IsNil) |
331 | - c.Assert(cmd.EnvConfig, gc.DeepEquals, t.expected) |
332 | + c.Assert(cmd.EnvConfig, gc.DeepEquals, t.expectedConfig) |
333 | + c.Assert(cmd.InstanceId, gc.Equals, t.expectedInstanceId) |
334 | + c.Assert(cmd.Hardware, gc.DeepEquals, t.expectedHardware) |
335 | } else { |
336 | c.Assert(err, gc.ErrorMatches, t.err) |
337 | } |
338 | |
339 | === modified file 'cmd/jujud/main_test.go' |
340 | --- cmd/jujud/main_test.go 2014-03-19 03:48:12 +0000 |
341 | +++ cmd/jujud/main_test.go 2014-03-24 01:53:33 +0000 |
342 | @@ -110,6 +110,7 @@ |
343 | checkMessage(c, msga, |
344 | "bootstrap-state", |
345 | "--env-config", b64yaml{"blah": "blah"}.encode(), |
346 | + "--instance-id", "inst", |
347 | "toastie") |
348 | checkMessage(c, msga, "unit", |
349 | "--unit-name", "un/0", |
350 | |
351 | === modified file 'environs/bootstrap/state.go' |
352 | --- environs/bootstrap/state.go 2014-03-12 10:59:17 +0000 |
353 | +++ environs/bootstrap/state.go 2014-03-24 01:53:33 +0000 |
354 | @@ -8,7 +8,6 @@ |
355 | "fmt" |
356 | "io" |
357 | "io/ioutil" |
358 | - "net/http" |
359 | |
360 | "launchpad.net/goyaml" |
361 | |
362 | @@ -16,7 +15,6 @@ |
363 | "launchpad.net/juju-core/environs/storage" |
364 | coreerrors "launchpad.net/juju-core/errors" |
365 | "launchpad.net/juju-core/instance" |
366 | - "launchpad.net/juju-core/utils" |
367 | ) |
368 | |
369 | // StateFile is the name of the file where the provider's state is stored. |
370 | @@ -68,24 +66,6 @@ |
371 | return putState(storage, data) |
372 | } |
373 | |
374 | -// LoadStateFromURL reads state from the given URL. |
375 | -func LoadStateFromURL(url string, disableSSLHostnameVerification bool) (*BootstrapState, error) { |
376 | - logger.Debugf("loading %q from %q", StateFile, url) |
377 | - client := http.DefaultClient |
378 | - if disableSSLHostnameVerification { |
379 | - logger.Infof("hostname SSL verification disabled") |
380 | - client = utils.GetNonValidatingHTTPClient() |
381 | - } |
382 | - resp, err := client.Get(url) |
383 | - if err != nil { |
384 | - return nil, err |
385 | - } |
386 | - if resp.StatusCode != http.StatusOK { |
387 | - return nil, fmt.Errorf("could not load state from url: %v %s", url, resp.Status) |
388 | - } |
389 | - return loadState(resp.Body) |
390 | -} |
391 | - |
392 | // LoadState reads state from the given storage. |
393 | func LoadState(stor storage.StorageReader) (*BootstrapState, error) { |
394 | r, err := storage.Get(stor, StateFile) |
395 | |
396 | === modified file 'environs/bootstrap/state_test.go' |
397 | --- environs/bootstrap/state_test.go 2014-03-13 07:54:56 +0000 |
398 | +++ environs/bootstrap/state_test.go 2014-03-24 01:53:33 +0000 |
399 | @@ -125,33 +125,6 @@ |
400 | c.Check(*storedState, gc.DeepEquals, state) |
401 | } |
402 | |
403 | -func (suite *StateSuite) TestLoadStateFromURLReadsStateFile(c *gc.C) { |
404 | - storage, dataDir := suite.newStorageWithDataDir(c) |
405 | - state := suite.setUpSavedState(c, dataDir) |
406 | - url, err := storage.URL(bootstrap.StateFile) |
407 | - c.Assert(err, gc.IsNil) |
408 | - storedState, err := bootstrap.LoadStateFromURL(url, false) |
409 | - c.Assert(err, gc.IsNil) |
410 | - c.Check(*storedState, gc.DeepEquals, state) |
411 | -} |
412 | - |
413 | -func (suite *StateSuite) TestLoadStateFromURLBadCert(c *gc.C) { |
414 | - baseURL, _ := suite.testingHTTPSServer(c) |
415 | - url := baseURL + "/" + bootstrap.StateFile |
416 | - storedState, err := bootstrap.LoadStateFromURL(url, false) |
417 | - c.Assert(err, gc.ErrorMatches, ".*/provider-state:.* certificate signed by unknown authority") |
418 | - c.Assert(storedState, gc.IsNil) |
419 | -} |
420 | - |
421 | -func (suite *StateSuite) TestLoadStateFromURLBadCertPermitted(c *gc.C) { |
422 | - baseURL, dataDir := suite.testingHTTPSServer(c) |
423 | - state := suite.setUpSavedState(c, dataDir) |
424 | - url := baseURL + "/" + bootstrap.StateFile |
425 | - storedState, err := bootstrap.LoadStateFromURL(url, true) |
426 | - c.Assert(err, gc.IsNil) |
427 | - c.Check(*storedState, gc.DeepEquals, state) |
428 | -} |
429 | - |
430 | func (suite *StateSuite) TestLoadStateMissingFile(c *gc.C) { |
431 | stor := suite.newStorage(c) |
432 | _, err := bootstrap.LoadState(stor) |
433 | |
434 | === modified file 'environs/cloudinit.go' |
435 | --- environs/cloudinit.go 2014-03-12 02:28:30 +0000 |
436 | +++ environs/cloudinit.go 2014-03-24 01:53:33 +0000 |
437 | @@ -57,13 +57,11 @@ |
438 | // NewBootstrapMachineConfig sets up a basic machine configuration for a |
439 | // bootstrap node. You'll still need to supply more information, but this |
440 | // takes care of the fixed entries and the ones that are always needed. |
441 | -// stateInfoURL is the storage URL for the environment's state file. |
442 | -func NewBootstrapMachineConfig(stateInfoURL string, privateSystemSSHKey string) *cloudinit.MachineConfig { |
443 | +func NewBootstrapMachineConfig(privateSystemSSHKey string) *cloudinit.MachineConfig { |
444 | // For a bootstrap instance, FinishMachineConfig will provide the |
445 | // state.Info and the api.Info. The machine id must *always* be "0". |
446 | mcfg := NewMachineConfig("0", state.BootstrapNonce, nil, nil) |
447 | mcfg.StateServer = true |
448 | - mcfg.StateInfoURL = stateInfoURL |
449 | mcfg.SystemPrivateSSHKey = privateSystemSSHKey |
450 | mcfg.Jobs = []params.MachineJob{params.JobManageEnviron, params.JobHostUnits} |
451 | return mcfg |
452 | |
453 | === modified file 'environs/cloudinit/cloudinit.go' |
454 | --- environs/cloudinit/cloudinit.go 2014-03-18 22:20:40 +0000 |
455 | +++ environs/cloudinit/cloudinit.go 2014-03-24 01:53:33 +0000 |
456 | @@ -31,12 +31,6 @@ |
457 | "launchpad.net/juju-core/version" |
458 | ) |
459 | |
460 | -// BootstrapStateURLFile is used to communicate to the first bootstrap node |
461 | -// the URL from which to obtain important state information (instance id and |
462 | -// hardware characteristics). It is a transient file, only used as the node |
463 | -// is bootstrapping. |
464 | -const BootstrapStateURLFile = "/tmp/provider-state-url" |
465 | - |
466 | // SystemIdentity is the name of the file where the environment SSH key is kept. |
467 | const SystemIdentity = "system-identity" |
468 | |
469 | @@ -79,6 +73,15 @@ |
470 | // or be empty when starting a state server. |
471 | APIInfo *api.Info |
472 | |
473 | + // InstanceId is the instance ID of the machine being initialised. |
474 | + // This is required when bootstrapping, and ignored otherwise. |
475 | + InstanceId instance.Id |
476 | + |
477 | + // HardwareCharacteristics contains the harrdware characteristics of |
478 | + // the machine being initialised. This optional, and is only used by |
479 | + // the bootstrap agent during state initialisation. |
480 | + HardwareCharacteristics *instance.HardwareCharacteristics |
481 | + |
482 | // MachineNonce is set at provisioning/bootstrap time and used to |
483 | // ensure the agent is running on the correct instance. |
484 | MachineNonce string |
485 | @@ -128,9 +131,6 @@ |
486 | // Constraints holds the initial environment constraints. |
487 | Constraints constraints.Value |
488 | |
489 | - // StateInfoURL is the URL of a file which contains information about the state server machines. |
490 | - StateInfoURL string |
491 | - |
492 | // DisableSSLHostnameVerification can be set to true to tell cloud-init |
493 | // that it shouldn't verify SSL certificates |
494 | DisableSSLHostnameVerification bool |
495 | @@ -393,13 +393,20 @@ |
496 | if cons != "" { |
497 | cons = " --constraints " + shquote(cons) |
498 | } |
499 | + var hardware string |
500 | + if cfg.HardwareCharacteristics != nil { |
501 | + if hardware = cfg.HardwareCharacteristics.String(); hardware != "" { |
502 | + hardware = " --hardware " + shquote(hardware) |
503 | + } |
504 | + } |
505 | c.AddRunCmd(cloudinit.LogProgressCmd("Bootstrapping Juju machine agent")) |
506 | c.AddScripts( |
507 | - fmt.Sprintf("echo %s > %s", shquote(cfg.StateInfoURL), BootstrapStateURLFile), |
508 | // The bootstrapping is always run with debug on. |
509 | cfg.jujuTools()+"/jujud bootstrap-state"+ |
510 | " --data-dir "+shquote(cfg.DataDir)+ |
511 | " --env-config "+shquote(base64yaml(cfg.Config))+ |
512 | + " --instance-id "+shquote(string(cfg.InstanceId))+ |
513 | + hardware+ |
514 | cons+ |
515 | " --debug", |
516 | "rm -rf "+shquote(acfg.Dir()), |
517 | @@ -701,6 +708,9 @@ |
518 | if cfg.SystemPrivateSSHKey == "" { |
519 | return fmt.Errorf("missing system ssh identity") |
520 | } |
521 | + if cfg.InstanceId == "" { |
522 | + return fmt.Errorf("missing instance-id") |
523 | + } |
524 | } else { |
525 | if len(cfg.StateInfo.Addrs) == 0 { |
526 | return fmt.Errorf("missing state hosts") |
527 | |
528 | === modified file 'environs/cloudinit/cloudinit_test.go' |
529 | --- environs/cloudinit/cloudinit_test.go 2014-03-19 02:27:03 +0000 |
530 | +++ environs/cloudinit/cloudinit_test.go 2014-03-24 01:53:33 +0000 |
531 | @@ -101,7 +101,7 @@ |
532 | LogDir: agent.DefaultLogDir, |
533 | Jobs: allMachineJobs, |
534 | CloudInitOutputLog: environs.CloudInitOutputLog, |
535 | - StateInfoURL: "some-url", |
536 | + InstanceId: "i-bootstrap", |
537 | SystemPrivateSSHKey: "private rsa key", |
538 | MachineAgentServiceName: "jujud-machine-0", |
539 | MongoServiceName: "juju-db", |
540 | @@ -147,8 +147,7 @@ |
541 | install -m 600 /dev/null '/var/lib/juju/agents/bootstrap/agent\.conf' |
542 | printf '%s\\n' '.*' > '/var/lib/juju/agents/bootstrap/agent\.conf' |
543 | echo 'Bootstrapping Juju machine agent'.* |
544 | -echo 'some-url' > /tmp/provider-state-url |
545 | -/var/lib/juju/tools/1\.2\.3-precise-amd64/jujud bootstrap-state --data-dir '/var/lib/juju' --env-config '[^']*' --constraints 'mem=2048M' --debug |
546 | +/var/lib/juju/tools/1\.2\.3-precise-amd64/jujud bootstrap-state --data-dir '/var/lib/juju' --env-config '[^']*' --instance-id 'i-bootstrap' --constraints 'mem=2048M' --debug |
547 | rm -rf '/var/lib/juju/agents/bootstrap' |
548 | ln -s 1\.2\.3-precise-amd64 '/var/lib/juju/tools/machine-0' |
549 | echo 'Starting Juju machine agent \(jujud-machine-0\)'.* |
550 | @@ -182,7 +181,7 @@ |
551 | LogDir: agent.DefaultLogDir, |
552 | Jobs: allMachineJobs, |
553 | CloudInitOutputLog: environs.CloudInitOutputLog, |
554 | - StateInfoURL: "some-url", |
555 | + InstanceId: "i-bootstrap", |
556 | SystemPrivateSSHKey: "private rsa key", |
557 | MachineAgentServiceName: "jujud-machine-0", |
558 | MongoServiceName: "juju-db", |
559 | @@ -196,7 +195,7 @@ |
560 | grep '1234' \$bin/juju1\.2\.3-raring-amd64.sha256 \|\| \(echo "Tools checksum mismatch"; exit 1\) |
561 | rm \$bin/tools\.tar\.gz && rm \$bin/juju1\.2\.3-raring-amd64\.sha256 |
562 | printf %s '{"version":"1\.2\.3-raring-amd64","url":"http://foo\.com/tools/releases/juju1\.2\.3-raring-amd64\.tgz","sha256":"1234","size":10}' > \$bin/downloaded-tools\.txt |
563 | -/var/lib/juju/tools/1\.2\.3-raring-amd64/jujud bootstrap-state --data-dir '/var/lib/juju' --env-config '[^']*' --constraints 'mem=2048M' --debug |
564 | +/var/lib/juju/tools/1\.2\.3-raring-amd64/jujud bootstrap-state --data-dir '/var/lib/juju' --env-config '[^']*' --instance-id 'i-bootstrap' --constraints 'mem=2048M' --debug |
565 | rm -rf '/var/lib/juju/agents/bootstrap' |
566 | ln -s 1\.2\.3-raring-amd64 '/var/lib/juju/tools/machine-0' |
567 | `, |
568 | @@ -227,7 +226,7 @@ |
569 | LogDir: agent.DefaultLogDir, |
570 | Jobs: allMachineJobs, |
571 | CloudInitOutputLog: environs.CloudInitOutputLog, |
572 | - StateInfoURL: "some-url", |
573 | + InstanceId: "i-bootstrap", |
574 | SystemPrivateSSHKey: "private rsa key", |
575 | MachineAgentServiceName: "jujud-machine-0", |
576 | MongoServiceName: "juju-db", |
577 | @@ -386,7 +385,7 @@ |
578 | LogDir: agent.DefaultLogDir, |
579 | Jobs: allMachineJobs, |
580 | CloudInitOutputLog: environs.CloudInitOutputLog, |
581 | - StateInfoURL: "some-url", |
582 | + InstanceId: "i-bootstrap", |
583 | SystemPrivateSSHKey: "private rsa key", |
584 | MachineAgentServiceName: "jujud-machine-0", |
585 | MongoServiceName: "juju-db", |
586 | @@ -394,7 +393,7 @@ |
587 | setEnvConfig: true, |
588 | inexactMatch: true, |
589 | expectScripts: ` |
590 | -/var/lib/juju/tools/1\.2\.3-precise-amd64/jujud bootstrap-state --data-dir '/var/lib/juju' --env-config '[^']*' --debug |
591 | +/var/lib/juju/tools/1\.2\.3-precise-amd64/jujud bootstrap-state --data-dir '/var/lib/juju' --env-config '[^']*' --instance-id 'i-bootstrap' --debug |
592 | `, |
593 | }, |
594 | } |
595 | @@ -783,6 +782,9 @@ |
596 | {"missing mongo service name", func(cfg *cloudinit.MachineConfig) { |
597 | cfg.MongoServiceName = "" |
598 | }}, |
599 | + {"missing instance-id", func(cfg *cloudinit.MachineConfig) { |
600 | + cfg.InstanceId = "" |
601 | + }}, |
602 | } |
603 | |
604 | // TestCloudInitVerify checks that required fields are appropriately |
605 | @@ -812,6 +814,7 @@ |
606 | LogDir: agent.DefaultLogDir, |
607 | Jobs: normalMachineJobs, |
608 | CloudInitOutputLog: environs.CloudInitOutputLog, |
609 | + InstanceId: "i-bootstrap", |
610 | MachineNonce: "FAKE_NONCE", |
611 | SystemPrivateSSHKey: "private rsa key", |
612 | MachineAgentServiceName: "jujud-machine-99", |
613 | |
614 | === modified file 'environs/manual/bootstrap.go' |
615 | --- environs/manual/bootstrap.go 2014-02-20 02:24:46 +0000 |
616 | +++ environs/manual/bootstrap.go 2014-03-24 01:53:33 +0000 |
617 | @@ -124,8 +124,9 @@ |
618 | } |
619 | |
620 | // Finally, provision the machine agent. |
621 | - stateFileURL := fmt.Sprintf("file://%s/%s", storageDir, bootstrap.StateFile) |
622 | - mcfg := environs.NewBootstrapMachineConfig(stateFileURL, privateKey) |
623 | + mcfg := environs.NewBootstrapMachineConfig(privateKey) |
624 | + mcfg.InstanceId = BootstrapInstanceId |
625 | + mcfg.HardwareCharacteristics = args.HardwareCharacteristics |
626 | if args.DataDir != "" { |
627 | mcfg.DataDir = args.DataDir |
628 | } |
629 | |
630 | === modified file 'instance/instance.go' |
631 | --- instance/instance.go 2014-03-21 11:00:41 +0000 |
632 | +++ instance/instance.go 2014-03-24 01:53:33 +0000 |
633 | @@ -112,6 +112,16 @@ |
634 | return strings.Join(strs, " ") |
635 | } |
636 | |
637 | +// Implement gnuflag.Value |
638 | +func (hc *HardwareCharacteristics) Set(s string) error { |
639 | + parsed, err := ParseHardware(s) |
640 | + if err != nil { |
641 | + return err |
642 | + } |
643 | + *hc = parsed |
644 | + return nil |
645 | +} |
646 | + |
647 | // MustParseHardware constructs a HardwareCharacteristics from the supplied arguments, |
648 | // as Parse, but panics on failure. |
649 | func MustParseHardware(args ...string) HardwareCharacteristics { |
650 | |
651 | === modified file 'provider/common/bootstrap.go' |
652 | --- provider/common/bootstrap.go 2014-03-19 02:55:15 +0000 |
653 | +++ provider/common/bootstrap.go 2014-03-24 01:53:33 +0000 |
654 | @@ -56,19 +56,11 @@ |
655 | return fmt.Errorf("no SSH client available") |
656 | } |
657 | |
658 | - // Create an empty bootstrap state file so we can get its URL. |
659 | - // It will be updated with the instance id and hardware characteristics |
660 | - // after the bootstrap instance is started. |
661 | - stateFileURL, err := bootstrap.CreateStateFile(env.Storage()) |
662 | - if err != nil { |
663 | - return err |
664 | - } |
665 | - |
666 | privateKey, err := GenerateSystemSSHKey(env) |
667 | if err != nil { |
668 | return err |
669 | } |
670 | - machineConfig := environs.NewBootstrapMachineConfig(stateFileURL, privateKey) |
671 | + machineConfig := environs.NewBootstrapMachineConfig(privateKey) |
672 | |
673 | fmt.Fprintln(ctx.GetStderr(), "Launching instance") |
674 | inst, hw, err := env.StartInstance(environs.StartInstanceParams{ |
675 | @@ -80,6 +72,8 @@ |
676 | return fmt.Errorf("cannot start bootstrap instance: %v", err) |
677 | } |
678 | fmt.Fprintf(ctx.GetStderr(), " - %s\n", inst.Id()) |
679 | + machineConfig.InstanceId = inst.Id() |
680 | + machineConfig.HardwareCharacteristics = hw |
681 | |
682 | var characteristics []instance.HardwareCharacteristics |
683 | if hw != nil { |
684 | |
685 | === modified file 'provider/common/bootstrap_test.go' |
686 | --- provider/common/bootstrap_test.go 2014-03-19 22:16:15 +0000 |
687 | +++ provider/common/bootstrap_test.go 2014-03-24 01:53:33 +0000 |
688 | @@ -15,7 +15,6 @@ |
689 | |
690 | "launchpad.net/juju-core/constraints" |
691 | "launchpad.net/juju-core/environs" |
692 | - "launchpad.net/juju-core/environs/bootstrap" |
693 | "launchpad.net/juju-core/environs/cloudinit" |
694 | "launchpad.net/juju-core/environs/config" |
695 | "launchpad.net/juju-core/environs/storage" |
696 | @@ -76,21 +75,7 @@ |
697 | return func() *config.Config { return cfg } |
698 | } |
699 | |
700 | -func (s *BootstrapSuite) TestCannotWriteStateFile(c *gc.C) { |
701 | - brokenStorage := &mockStorage{ |
702 | - Storage: newStorage(s, c), |
703 | - putErr: fmt.Errorf("noes!"), |
704 | - } |
705 | - env := &mockEnviron{storage: brokenStorage, config: configGetter(c)} |
706 | - ctx := coretesting.Context(c) |
707 | - err := common.Bootstrap(ctx, env, constraints.Value{}) |
708 | - c.Assert(err, gc.ErrorMatches, "cannot create initial state file: noes!") |
709 | -} |
710 | - |
711 | func (s *BootstrapSuite) TestCannotStartInstance(c *gc.C) { |
712 | - stor := newStorage(s, c) |
713 | - checkURL, err := stor.URL(bootstrap.StateFile) |
714 | - c.Assert(err, gc.IsNil) |
715 | checkCons := constraints.MustParse("mem=8G") |
716 | |
717 | startInstance := func( |
718 | @@ -99,18 +84,18 @@ |
719 | instance.Instance, *instance.HardwareCharacteristics, error, |
720 | ) { |
721 | c.Assert(cons, gc.DeepEquals, checkCons) |
722 | - c.Assert(mcfg, gc.DeepEquals, environs.NewBootstrapMachineConfig(checkURL, mcfg.SystemPrivateSSHKey)) |
723 | + c.Assert(mcfg, gc.DeepEquals, environs.NewBootstrapMachineConfig(mcfg.SystemPrivateSSHKey)) |
724 | return nil, nil, fmt.Errorf("meh, not started") |
725 | } |
726 | |
727 | env := &mockEnviron{ |
728 | - storage: stor, |
729 | + storage: newStorage(s, c), |
730 | startInstance: startInstance, |
731 | config: configGetter(c), |
732 | } |
733 | |
734 | ctx := coretesting.Context(c) |
735 | - err = common.Bootstrap(ctx, env, checkCons) |
736 | + err := common.Bootstrap(ctx, env, checkCons) |
737 | c.Assert(err, gc.ErrorMatches, "cannot start bootstrap instance: meh, not started") |
738 | } |
739 | |
740 | @@ -192,13 +177,11 @@ |
741 | checkInstanceId := "i-success" |
742 | checkHardware := instance.MustParseHardware("mem=2T") |
743 | |
744 | - checkURL := "" |
745 | startInstance := func( |
746 | _ constraints.Value, _ environs.Networks, _ tools.List, mcfg *cloudinit.MachineConfig, |
747 | ) ( |
748 | instance.Instance, *instance.HardwareCharacteristics, error, |
749 | ) { |
750 | - checkURL = mcfg.StateInfoURL |
751 | return &mockInstance{id: checkInstanceId}, &checkHardware, nil |
752 | } |
753 | var mocksConfig = minimalConfig(c) |
754 | @@ -226,12 +209,6 @@ |
755 | err := common.Bootstrap(ctx, env, constraints.Value{}) |
756 | c.Assert(err, gc.IsNil) |
757 | |
758 | - savedState, err := bootstrap.LoadStateFromURL(checkURL, false) |
759 | - c.Assert(err, gc.IsNil) |
760 | - c.Assert(savedState, gc.DeepEquals, &bootstrap.BootstrapState{ |
761 | - StateInstances: []instance.Id{instance.Id(checkInstanceId)}, |
762 | - Characteristics: []instance.HardwareCharacteristics{checkHardware}, |
763 | - }) |
764 | authKeys := env.Config().AuthorizedKeys() |
765 | c.Assert(authKeys, gc.Not(gc.Equals), originalAuthKeys) |
766 | c.Assert(authKeys, jc.HasSuffix, "juju-system-key\n") |
767 | |
768 | === modified file 'provider/local/environ.go' |
769 | --- provider/local/environ.go 2014-03-19 21:08:58 +0000 |
770 | +++ provider/local/environ.go 2014-03-24 01:53:33 +0000 |
771 | @@ -109,10 +109,6 @@ |
772 | |
773 | // Before we write the agent config file, we need to make sure the |
774 | // instance is saved in the StateInfo. |
775 | - stateFileURL, err := bootstrap.CreateStateFile(env.Storage()) |
776 | - if err != nil { |
777 | - return err |
778 | - } |
779 | if err := bootstrap.SaveState(env.Storage(), &bootstrap.BootstrapState{ |
780 | StateInstances: []instance.Id{bootstrapInstanceId}, |
781 | }); err != nil { |
782 | @@ -138,7 +134,8 @@ |
783 | return err |
784 | } |
785 | |
786 | - mcfg := environs.NewBootstrapMachineConfig(stateFileURL, privateKey) |
787 | + mcfg := environs.NewBootstrapMachineConfig(privateKey) |
788 | + mcfg.InstanceId = bootstrapInstanceId |
789 | mcfg.Tools = selectedTools[0] |
790 | mcfg.DataDir = env.config.rootDir() |
791 | mcfg.LogDir = fmt.Sprintf("/var/log/juju-%s", env.config.namespace()) |
Reviewers: mp+212172_ code.launchpad. net,
Message:
Please take a look.
Description: id/hardware directly to jujud
Pass instance-
The bootstrap-state agent currently loads its
instance ID and hardware characteristics by
fetching a URL that is written to a file on
disk. This is no longer necessary with
synchronous bootstrap.
I have changed jujud and bootstrap script
generation to pass the instance-id and hardware
characteristics directly to jujud as command
line arguments.
Fixes part of lp:1291292
https:/ /code.launchpad .net/~axwalk/ juju-core/ lp1291292- jujud-bootstrap -nostateurl/ +merge/ 212172
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/78840044/
Affected files (+140, -141 lines): sshinit/ configure_ test.go bootstrap. go bootstrap_ test.go main_test. go cloudinit. go cloudinit/ cloudinit. go cloudinit/ cloudinit_ test.go manual/ bootstrap. go instance. go common/ bootstrap. go common/ bootstrap_ test.go local/environ. go
A [revision details]
M cloudinit/
M cmd/jujud/
M cmd/jujud/
M cmd/jujud/
M environs/
M environs/
M environs/
M environs/
M instance/
M provider/
M provider/
M provider/