Merge lp:~thumper/juju-core/agent-config-formatters into lp:~go-bot/juju-core/trunk
- agent-config-formatters
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Tim Penhey |
Approved revision: | no longer in the source branch. |
Merged at revision: | 1748 |
Proposed branch: | lp:~thumper/juju-core/agent-config-formatters |
Merge into: | lp:~go-bot/juju-core/trunk |
Diff against target: |
815 lines (+462/-169) 5 files modified
agent/agent.go (+108/-169) agent/format-1.12.go (+175/-0) agent/format-1.12_whitebox_test.go (+96/-0) agent/format.go (+51/-0) agent/format_whitebox_test.go (+32/-0) |
To merge this branch: | bzr merge lp:~thumper/juju-core/agent-config-formatters |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju Engineering | Pending | ||
Review via email:
|
Commit message
Add the formatter concept to agent config.
In order to decouple the internal structure of the agent config from how it is
stored on disk, we introduce formatters. They are responsible for moving the
internal config to and from a serialization format.
The existing format is called "format 1.12", as that is the stable release
that the format was introduced.
Some of the methods are purposefully trivial at this stage, and they are
flushed out in the next branch.
Description of the change
Add the formatter concept to agent config.
In order to decouple the internal structure of the agent config from how it is
stored on disk, we introduce formatters. They are responsible for moving the
internal config to and from a serialization format.
The existing format is called "format 1.12", as that is the stable release
that the format was introduced.
Some of the methods are purposefully trivial at this stage, and they are
flushed out in the next branch.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Tim Penhey (thumper) wrote : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
William Reade (fwereade) wrote : | # |
LGTM, just quibbles
https:/
File agent/format-
https:/
agent/format-
1_12 throughout?
https:/
File agent/format-
https:/
agent/format-
I don't personally feel we need to make a big deal out of this.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Tim Penhey (thumper) wrote : | # |
Please take a look.
https:/
File agent/format-
https:/
agent/format-
On 2013/08/30 07:06:22, fwereade wrote:
> 1_12 throughout?
I have renamed "format112" to "format_1_12" because "format1_12" looked
too weird.
https:/
agent/format-
formatter112 becomes formatter_1_12
https:/
agent/format-
format112Serial
https:/
File agent/format-
https:/
agent/format-
On 2013/08/30 07:06:22, fwereade wrote:
> I don't personally feel we need to make a big deal out of this.
Removed then.
Preview Diff
1 | === modified file 'agent/agent.go' |
2 | --- agent/agent.go 2013-08-28 22:59:45 +0000 |
3 | +++ agent/agent.go 2013-09-02 21:47:29 +0000 |
4 | @@ -5,12 +5,9 @@ |
5 | |
6 | import ( |
7 | "fmt" |
8 | - "io/ioutil" |
9 | - "os" |
10 | "path" |
11 | "regexp" |
12 | |
13 | - "launchpad.net/goyaml" |
14 | "launchpad.net/loggo" |
15 | |
16 | "launchpad.net/juju-core/environs/config" |
17 | @@ -69,46 +66,27 @@ |
18 | APIServerDetails() (port int, cert, key []byte) |
19 | } |
20 | |
21 | -// conf holds information for a given agent. |
22 | -type conf struct { |
23 | - // DataDir specifies the path of the data directory used by all |
24 | - // agents |
25 | - dataDir string |
26 | - |
27 | - // StateServerCert and StateServerKey hold the state server |
28 | - // certificate and private key in PEM format. |
29 | - StateServerCert []byte `yaml:",omitempty"` |
30 | - StateServerKey []byte `yaml:",omitempty"` |
31 | - |
32 | - StatePort int `yaml:",omitempty"` |
33 | - APIPort int `yaml:",omitempty"` |
34 | - |
35 | - // OldPassword specifies a password that should be |
36 | - // used to connect to the state if StateInfo.Password |
37 | - // is blank or invalid. |
38 | - OldPassword string |
39 | - |
40 | - // MachineNonce is set at provisioning/bootstrap time and used to |
41 | - // ensure the agent is running on the correct instance. |
42 | - MachineNonce string |
43 | - |
44 | - // StateInfo specifies how the agent should connect to the |
45 | - // state. The password may be empty if an old password is |
46 | - // specified, or when bootstrapping. |
47 | - StateInfo *state.Info `yaml:",omitempty"` |
48 | - |
49 | - // OldAPIPassword specifies a password that should |
50 | - // be used to connect to the API if APIInfo.Password |
51 | - // is blank or invalid. |
52 | - OldAPIPassword string |
53 | - |
54 | - // APIInfo specifies how the agent should connect to the |
55 | - // state through the API. |
56 | - APIInfo *api.Info `yaml:",omitempty"` |
57 | -} |
58 | - |
59 | -// Ensure that conf implements Config. |
60 | -var _ Config = (*conf)(nil) |
61 | +// Ensure that the configInternal struct implements the Config interface. |
62 | +var _ Config = (*configInternal)(nil) |
63 | + |
64 | +type connectionDetails struct { |
65 | + addresses []string |
66 | + password string |
67 | +} |
68 | + |
69 | +type configInternal struct { |
70 | + dataDir string |
71 | + tag string |
72 | + nonce string |
73 | + caCert []byte |
74 | + stateDetails *connectionDetails |
75 | + apiDetails *connectionDetails |
76 | + oldPassword string |
77 | + stateServerCert []byte |
78 | + stateServerKey []byte |
79 | + statePort int |
80 | + apiPort int |
81 | +} |
82 | |
83 | type AgentConfigParams struct { |
84 | DataDir string |
85 | @@ -120,7 +98,7 @@ |
86 | CACert []byte |
87 | } |
88 | |
89 | -func newConfig(params AgentConfigParams) (*conf, error) { |
90 | +func newConfig(params AgentConfigParams) (*configInternal, error) { |
91 | if params.DataDir == "" { |
92 | return nil, requiredError("data directory") |
93 | } |
94 | @@ -135,33 +113,27 @@ |
95 | } |
96 | // Note that the password parts of the state and api information are |
97 | // blank. This is by design. |
98 | - var stateInfo *state.Info |
99 | + config := &configInternal{ |
100 | + dataDir: params.DataDir, |
101 | + tag: params.Tag, |
102 | + nonce: params.Nonce, |
103 | + caCert: params.CACert, |
104 | + oldPassword: params.Password, |
105 | + } |
106 | if len(params.StateAddresses) > 0 { |
107 | - stateInfo = &state.Info{ |
108 | - Addrs: params.StateAddresses, |
109 | - Tag: params.Tag, |
110 | - CACert: params.CACert, |
111 | + config.stateDetails = &connectionDetails{ |
112 | + addresses: params.StateAddresses, |
113 | } |
114 | } |
115 | - var apiInfo *api.Info |
116 | if len(params.APIAddresses) > 0 { |
117 | - apiInfo = &api.Info{ |
118 | - Addrs: params.APIAddresses, |
119 | - Tag: params.Tag, |
120 | - CACert: params.CACert, |
121 | + config.apiDetails = &connectionDetails{ |
122 | + addresses: params.APIAddresses, |
123 | } |
124 | } |
125 | - conf := &conf{ |
126 | - dataDir: params.DataDir, |
127 | - OldPassword: params.Password, |
128 | - StateInfo: stateInfo, |
129 | - APIInfo: apiInfo, |
130 | - MachineNonce: params.Nonce, |
131 | - } |
132 | - if err := conf.check(); err != nil { |
133 | + if err := config.check(); err != nil { |
134 | return nil, err |
135 | } |
136 | - return conf, nil |
137 | + return config, nil |
138 | } |
139 | |
140 | // NewAgentConfig returns a new config object suitable for use for a unit |
141 | @@ -186,15 +158,15 @@ |
142 | if params.StateServerKey == nil { |
143 | return nil, requiredError("state server key") |
144 | } |
145 | - conf, err := newConfig(params.AgentConfigParams) |
146 | + config, err := newConfig(params.AgentConfigParams) |
147 | if err != nil { |
148 | return nil, err |
149 | } |
150 | - conf.StateServerCert = params.StateServerCert |
151 | - conf.StateServerKey = params.StateServerKey |
152 | - conf.StatePort = params.StatePort |
153 | - conf.APIPort = params.APIPort |
154 | - return conf, nil |
155 | + config.stateServerCert = params.StateServerCert |
156 | + config.stateServerKey = params.StateServerKey |
157 | + config.statePort = params.StatePort |
158 | + config.apiPort = params.APIPort |
159 | + return config, nil |
160 | } |
161 | |
162 | // Dir returns the agent-specific data directory. |
163 | @@ -206,25 +178,20 @@ |
164 | // entity from the given data directory. |
165 | func ReadConf(dataDir, tag string) (Config, error) { |
166 | dir := Dir(dataDir, tag) |
167 | - data, err := ioutil.ReadFile(path.Join(dir, "agent.conf")) |
168 | + config, err := currentFormatter.read(dir) |
169 | if err != nil { |
170 | return nil, err |
171 | } |
172 | - var c conf |
173 | - if err := goyaml.Unmarshal(data, &c); err != nil { |
174 | - return nil, err |
175 | - } |
176 | - c.dataDir = dataDir |
177 | - if err := c.check(); err != nil { |
178 | - return nil, err |
179 | - } |
180 | - if c.StateInfo != nil { |
181 | - c.StateInfo.Tag = tag |
182 | - } |
183 | - if c.APIInfo != nil { |
184 | - c.APIInfo.Tag = tag |
185 | - } |
186 | - return &c, nil |
187 | + config.dataDir = dataDir |
188 | + if err := config.check(); err != nil { |
189 | + return nil, err |
190 | + } |
191 | + return config, nil |
192 | + // Read the format for the dir. |
193 | + // Create a formatter for the format |
194 | + // Read in the config |
195 | + // If the format isn't the current format, call the upgrade function, and |
196 | + // write out using the new formatter. |
197 | } |
198 | |
199 | func requiredError(what string) error { |
200 | @@ -232,53 +199,45 @@ |
201 | } |
202 | |
203 | // File returns the path of the given file in the agent's directory. |
204 | -func (c *conf) File(name string) string { |
205 | +func (c *configInternal) File(name string) string { |
206 | return path.Join(c.Dir(), name) |
207 | } |
208 | |
209 | -func (c *conf) confFile() string { |
210 | - return c.File("agent.conf") |
211 | -} |
212 | - |
213 | -func (c *conf) DataDir() string { |
214 | +func (c *configInternal) DataDir() string { |
215 | return c.dataDir |
216 | } |
217 | |
218 | -func (c *conf) Nonce() string { |
219 | - return c.MachineNonce |
220 | +func (c *configInternal) Nonce() string { |
221 | + return c.nonce |
222 | } |
223 | |
224 | -func (c *conf) APIServerDetails() (port int, cert, key []byte) { |
225 | - return c.APIPort, c.StateServerCert, c.StateServerKey |
226 | +func (c *configInternal) APIServerDetails() (port int, cert, key []byte) { |
227 | + return c.apiPort, c.stateServerCert, c.stateServerKey |
228 | } |
229 | |
230 | // Tag returns the tag of the entity on whose behalf the state connection will |
231 | // be made. |
232 | -func (c *conf) Tag() string { |
233 | - if c.StateInfo != nil { |
234 | - return c.StateInfo.Tag |
235 | - } |
236 | - return c.APIInfo.Tag |
237 | +func (c *configInternal) Tag() string { |
238 | + return c.tag |
239 | } |
240 | |
241 | // Dir returns the agent's directory. |
242 | -func (c *conf) Dir() string { |
243 | - return Dir(c.dataDir, c.Tag()) |
244 | +func (c *configInternal) Dir() string { |
245 | + return Dir(c.dataDir, c.tag) |
246 | } |
247 | |
248 | // Check checks that the configuration has all the required elements. |
249 | -func (c *conf) check() error { |
250 | - if c.StateInfo == nil && c.APIInfo == nil { |
251 | +func (c *configInternal) check() error { |
252 | + if c.stateDetails == nil && c.apiDetails == nil { |
253 | return requiredError("state or API addresses") |
254 | } |
255 | - if c.StateInfo != nil { |
256 | - if err := checkAddrs(c.StateInfo.Addrs, "state server address"); err != nil { |
257 | + if c.stateDetails != nil { |
258 | + if err := checkAddrs(c.stateDetails.addresses, "state server address"); err != nil { |
259 | return err |
260 | } |
261 | } |
262 | - // TODO(rog) make APIInfo mandatory |
263 | - if c.APIInfo != nil { |
264 | - if err := checkAddrs(c.APIInfo.Addrs, "API server address"); err != nil { |
265 | + if c.apiDetails != nil { |
266 | + if err := checkAddrs(c.apiDetails.addresses, "API server address"); err != nil { |
267 | return err |
268 | } |
269 | } |
270 | @@ -299,17 +258,17 @@ |
271 | return nil |
272 | } |
273 | |
274 | -func (c *conf) PasswordHash() string { |
275 | +func (c *configInternal) PasswordHash() string { |
276 | var password string |
277 | - if c.StateInfo == nil { |
278 | - password = c.APIInfo.Password |
279 | + if c.stateDetails == nil { |
280 | + password = c.apiDetails.password |
281 | } else { |
282 | - password = c.StateInfo.Password |
283 | + password = c.stateDetails.password |
284 | } |
285 | return utils.PasswordHash(password) |
286 | } |
287 | |
288 | -func (c *conf) GenerateNewPassword() (string, error) { |
289 | +func (c *configInternal) GenerateNewPassword() (string, error) { |
290 | newPassword, err := utils.RandomPassword() |
291 | if err != nil { |
292 | return "", err |
293 | @@ -318,15 +277,15 @@ |
294 | // to write the configuration file, the configuration will |
295 | // still be valid. |
296 | other := *c |
297 | - if c.StateInfo != nil { |
298 | - stateInfo := *c.StateInfo |
299 | - stateInfo.Password = newPassword |
300 | - other.StateInfo = &stateInfo |
301 | + if c.stateDetails != nil { |
302 | + stateDetails := *c.stateDetails |
303 | + stateDetails.password = newPassword |
304 | + other.stateDetails = &stateDetails |
305 | } |
306 | - if c.APIInfo != nil { |
307 | - apiInfo := *c.APIInfo |
308 | - apiInfo.Password = newPassword |
309 | - other.APIInfo = &apiInfo |
310 | + if c.apiDetails != nil { |
311 | + apiDetails := *c.apiDetails |
312 | + apiDetails.password = newPassword |
313 | + other.apiDetails = &apiDetails |
314 | } |
315 | |
316 | if err := other.Write(); err != nil { |
317 | @@ -337,47 +296,15 @@ |
318 | } |
319 | |
320 | // Write writes the agent configuration. |
321 | -func (c *conf) Write() error { |
322 | - if err := c.check(); err != nil { |
323 | - return err |
324 | - } |
325 | - data, err := goyaml.Marshal(c) |
326 | - if err != nil { |
327 | - return err |
328 | - } |
329 | - if err := os.MkdirAll(c.Dir(), 0755); err != nil { |
330 | - return err |
331 | - } |
332 | - f := c.File("agent.conf-new") |
333 | - if err := ioutil.WriteFile(f, data, 0600); err != nil { |
334 | - return err |
335 | - } |
336 | - if err := os.Rename(f, c.confFile()); err != nil { |
337 | - return err |
338 | - } |
339 | - return nil |
340 | +func (c *configInternal) Write() error { |
341 | + return currentFormatter.write(c) |
342 | } |
343 | |
344 | // WriteCommands returns shell commands to write the agent |
345 | // configuration. It returns an error if the configuration does not |
346 | // have all the right elements. |
347 | -func (c *conf) WriteCommands() ([]string, error) { |
348 | - if err := c.check(); err != nil { |
349 | - return nil, err |
350 | - } |
351 | - data, err := goyaml.Marshal(c) |
352 | - if err != nil { |
353 | - return nil, err |
354 | - } |
355 | - var cmds []string |
356 | - addCmd := func(f string, a ...interface{}) { |
357 | - cmds = append(cmds, fmt.Sprintf(f, a...)) |
358 | - } |
359 | - f := utils.ShQuote(c.confFile()) |
360 | - addCmd("mkdir -p %s", utils.ShQuote(c.Dir())) |
361 | - addCmd("install -m %o /dev/null %s", 0600, f) |
362 | - addCmd(`printf '%%s\n' %s > %s`, utils.ShQuote(string(data)), f) |
363 | - return cmds, nil |
364 | +func (c *configInternal) WriteCommands() ([]string, error) { |
365 | + return currentFormatter.writeCommands(c) |
366 | } |
367 | |
368 | // OpenAPI tries to open the state using the given Conf. If it |
369 | @@ -385,9 +312,14 @@ |
370 | // to the state should be changed accordingly - the caller should write the |
371 | // configuration with StateInfo.Password set to newPassword, then |
372 | // set the entity's password accordingly. |
373 | -func (c *conf) OpenAPI(dialOpts api.DialOpts) (st *api.State, newPassword string, err error) { |
374 | - info := *c.APIInfo |
375 | - info.Nonce = c.MachineNonce |
376 | +func (c *configInternal) OpenAPI(dialOpts api.DialOpts) (st *api.State, newPassword string, err error) { |
377 | + info := api.Info{ |
378 | + Addrs: c.apiDetails.addresses, |
379 | + Password: c.apiDetails.password, |
380 | + CACert: c.caCert, |
381 | + Tag: c.tag, |
382 | + Nonce: c.nonce, |
383 | + } |
384 | if info.Password != "" { |
385 | st, err := api.Open(&info, dialOpts) |
386 | if err == nil { |
387 | @@ -401,7 +333,7 @@ |
388 | // password but before changing it, so we'll try again |
389 | // with the old password. |
390 | } |
391 | - info.Password = c.OldPassword |
392 | + info.Password = c.oldPassword |
393 | st, err = api.Open(&info, dialOpts) |
394 | if err != nil { |
395 | return nil, "", err |
396 | @@ -418,8 +350,13 @@ |
397 | } |
398 | |
399 | // OpenState tries to open the state using the given Conf. |
400 | -func (c *conf) OpenState() (*state.State, error) { |
401 | - info := *c.StateInfo |
402 | +func (c *configInternal) OpenState() (*state.State, error) { |
403 | + info := state.Info{ |
404 | + Addrs: c.stateDetails.addresses, |
405 | + Password: c.stateDetails.password, |
406 | + CACert: c.caCert, |
407 | + Tag: c.tag, |
408 | + } |
409 | if info.Password != "" { |
410 | st, err := state.Open(&info, state.DefaultDialOpts()) |
411 | if err == nil { |
412 | @@ -431,14 +368,16 @@ |
413 | return nil, err |
414 | } |
415 | } |
416 | - info.Password = c.OldPassword |
417 | + info.Password = c.oldPassword |
418 | return state.Open(&info, state.DefaultDialOpts()) |
419 | } |
420 | |
421 | func InitialStateConfiguration(agentConfig Config, cfg *config.Config, timeout state.DialOpts) (*state.State, error) { |
422 | - c := agentConfig.(*conf) |
423 | - info := *c.StateInfo |
424 | - info.Tag = "" |
425 | + c := agentConfig.(*configInternal) |
426 | + info := state.Info{ |
427 | + Addrs: c.stateDetails.addresses, |
428 | + CACert: c.caCert, |
429 | + } |
430 | logger.Debugf("initializing address %v", info.Addrs) |
431 | st, err := state.Initialize(&info, cfg, timeout) |
432 | if err != nil { |
433 | @@ -451,7 +390,7 @@ |
434 | } |
435 | logger.Debugf("state initialized") |
436 | |
437 | - if err := bootstrapUsers(st, cfg, c.OldPassword); err != nil { |
438 | + if err := bootstrapUsers(st, cfg, c.oldPassword); err != nil { |
439 | st.Close() |
440 | return nil, err |
441 | } |
442 | |
443 | === added file 'agent/format-1.12.go' |
444 | --- agent/format-1.12.go 1970-01-01 00:00:00 +0000 |
445 | +++ agent/format-1.12.go 2013-09-02 21:47:29 +0000 |
446 | @@ -0,0 +1,175 @@ |
447 | +// Copyright 2013 Canonical Ltd. |
448 | +// Licensed under the AGPLv3, see LICENCE file for details. |
449 | + |
450 | +package agent |
451 | + |
452 | +import ( |
453 | + "fmt" |
454 | + "io/ioutil" |
455 | + "os" |
456 | + "path" |
457 | + |
458 | + "launchpad.net/goyaml" |
459 | + |
460 | + "launchpad.net/juju-core/state" |
461 | + "launchpad.net/juju-core/state/api" |
462 | + "launchpad.net/juju-core/utils" |
463 | +) |
464 | + |
465 | +const format_1_12 = "format 1.12" |
466 | + |
467 | +// formatter_1_12 is the formatter for the 1.12 format. |
468 | +type formatter_1_12 struct { |
469 | +} |
470 | + |
471 | +// format_1_12Serialization holds information stored in the agent.conf file. |
472 | +type format_1_12Serialization struct { |
473 | + // StateServerCert and StateServerKey hold the state server |
474 | + // certificate and private key in PEM format. |
475 | + StateServerCert []byte `yaml:",omitempty"` |
476 | + StateServerKey []byte `yaml:",omitempty"` |
477 | + |
478 | + StatePort int `yaml:",omitempty"` |
479 | + APIPort int `yaml:",omitempty"` |
480 | + |
481 | + // OldPassword specifies a password that should be |
482 | + // used to connect to the state if StateInfo.Password |
483 | + // is blank or invalid. |
484 | + OldPassword string |
485 | + |
486 | + // MachineNonce is set at provisioning/bootstrap time and used to |
487 | + // ensure the agent is running on the correct instance. |
488 | + MachineNonce string |
489 | + |
490 | + // StateInfo specifies how the agent should connect to the |
491 | + // state. The password may be empty if an old password is |
492 | + // specified, or when bootstrapping. |
493 | + StateInfo *state.Info `yaml:",omitempty"` |
494 | + |
495 | + // OldAPIPassword specifies a password that should |
496 | + // be used to connect to the API if APIInfo.Password |
497 | + // is blank or invalid. |
498 | + OldAPIPassword string |
499 | + |
500 | + // APIInfo specifies how the agent should connect to the |
501 | + // state through the API. |
502 | + APIInfo *api.Info `yaml:",omitempty"` |
503 | +} |
504 | + |
505 | +// Ensure that the formatter_1_12 struct implements the formatter interface. |
506 | +var _ formatter = (*formatter_1_12)(nil) |
507 | + |
508 | +func (*formatter_1_12) configFile(dirName string) string { |
509 | + return path.Join(dirName, "agent.conf") |
510 | +} |
511 | + |
512 | +func (formatter *formatter_1_12) read(dirName string) (*configInternal, error) { |
513 | + data, err := ioutil.ReadFile(formatter.configFile(dirName)) |
514 | + if err != nil { |
515 | + return nil, err |
516 | + } |
517 | + var conf format_1_12Serialization |
518 | + if err := goyaml.Unmarshal(data, &conf); err != nil { |
519 | + return nil, err |
520 | + } |
521 | + |
522 | + var stateDetails *connectionDetails |
523 | + var caCert []byte |
524 | + var tag string |
525 | + if conf.StateInfo != nil { |
526 | + stateDetails = &connectionDetails{ |
527 | + conf.StateInfo.Addrs, |
528 | + conf.StateInfo.Password, |
529 | + } |
530 | + tag = conf.StateInfo.Tag |
531 | + caCert = conf.StateInfo.CACert |
532 | + } |
533 | + var apiDetails *connectionDetails |
534 | + if conf.APIInfo != nil { |
535 | + apiDetails = &connectionDetails{ |
536 | + conf.APIInfo.Addrs, |
537 | + conf.APIInfo.Password, |
538 | + } |
539 | + tag = conf.APIInfo.Tag |
540 | + caCert = conf.APIInfo.CACert |
541 | + } |
542 | + return &configInternal{ |
543 | + tag: tag, |
544 | + nonce: conf.MachineNonce, |
545 | + caCert: caCert, |
546 | + stateDetails: stateDetails, |
547 | + apiDetails: apiDetails, |
548 | + oldPassword: conf.OldPassword, |
549 | + stateServerCert: conf.StateServerCert, |
550 | + stateServerKey: conf.StateServerKey, |
551 | + statePort: conf.StatePort, |
552 | + apiPort: conf.APIPort, |
553 | + }, nil |
554 | +} |
555 | + |
556 | +func (formatter *formatter_1_12) makeAgentConf(config *configInternal) *format_1_12Serialization { |
557 | + format := &format_1_12Serialization{ |
558 | + StateServerCert: config.stateServerCert, |
559 | + StateServerKey: config.stateServerKey, |
560 | + StatePort: config.statePort, |
561 | + APIPort: config.apiPort, |
562 | + OldPassword: config.oldPassword, |
563 | + MachineNonce: config.nonce, |
564 | + } |
565 | + if config.stateDetails != nil { |
566 | + // It is fine that we are copying the slices for the addresses. |
567 | + format.StateInfo = &state.Info{ |
568 | + Addrs: config.stateDetails.addresses, |
569 | + Password: config.stateDetails.password, |
570 | + Tag: config.tag, |
571 | + CACert: config.caCert, |
572 | + } |
573 | + } |
574 | + if config.apiDetails != nil { |
575 | + format.APIInfo = &api.Info{ |
576 | + Addrs: config.apiDetails.addresses, |
577 | + Password: config.apiDetails.password, |
578 | + Tag: config.tag, |
579 | + CACert: config.caCert, |
580 | + } |
581 | + } |
582 | + return format |
583 | +} |
584 | + |
585 | +func (formatter *formatter_1_12) write(config *configInternal) error { |
586 | + dirName := config.Dir() |
587 | + conf := formatter.makeAgentConf(config) |
588 | + data, err := goyaml.Marshal(conf) |
589 | + if err != nil { |
590 | + return err |
591 | + } |
592 | + if err := os.MkdirAll(dirName, 0755); err != nil { |
593 | + return err |
594 | + } |
595 | + newFile := path.Join(dirName, "agent.conf-new") |
596 | + if err := ioutil.WriteFile(newFile, data, 0600); err != nil { |
597 | + return err |
598 | + } |
599 | + if err := os.Rename(newFile, formatter.configFile(dirName)); err != nil { |
600 | + return err |
601 | + } |
602 | + return nil |
603 | +} |
604 | + |
605 | +func (formatter *formatter_1_12) writeCommands(config *configInternal) ([]string, error) { |
606 | + dirName := config.Dir() |
607 | + conf := formatter.makeAgentConf(config) |
608 | + data, err := goyaml.Marshal(conf) |
609 | + if err != nil { |
610 | + return nil, err |
611 | + } |
612 | + var commands []string |
613 | + addCommand := func(f string, a ...interface{}) { |
614 | + commands = append(commands, fmt.Sprintf(f, a...)) |
615 | + } |
616 | + filename := utils.ShQuote(formatter.configFile(dirName)) |
617 | + addCommand("mkdir -p %s", utils.ShQuote(dirName)) |
618 | + addCommand("install -m %o /dev/null %s", 0600, filename) |
619 | + addCommand(`printf '%%s\n' %s > %s`, utils.ShQuote(string(data)), filename) |
620 | + return commands, nil |
621 | +} |
622 | |
623 | === added file 'agent/format-1.12_whitebox_test.go' |
624 | --- agent/format-1.12_whitebox_test.go 1970-01-01 00:00:00 +0000 |
625 | +++ agent/format-1.12_whitebox_test.go 2013-09-02 21:47:29 +0000 |
626 | @@ -0,0 +1,96 @@ |
627 | +// Copyright 2013 Canonical Ltd. |
628 | +// Licensed under the AGPLv3, see LICENCE file for details. |
629 | + |
630 | +package agent |
631 | + |
632 | +import ( |
633 | + "os" |
634 | + "path" |
635 | + |
636 | + gc "launchpad.net/gocheck" |
637 | + |
638 | + "launchpad.net/juju-core/testing" |
639 | + jc "launchpad.net/juju-core/testing/checkers" |
640 | +) |
641 | + |
642 | +type format_1_12Suite struct { |
643 | + testing.LoggingSuite |
644 | + formatter formatter_1_12 |
645 | +} |
646 | + |
647 | +var _ = gc.Suite(&format_1_12Suite{}) |
648 | + |
649 | +var agentParams = AgentConfigParams{ |
650 | + Tag: "omg", |
651 | + Password: "sekrit", |
652 | + CACert: []byte("ca cert"), |
653 | + StateAddresses: []string{"localhost:1234"}, |
654 | + APIAddresses: []string{"localhost:1235"}, |
655 | + Nonce: "a nonce", |
656 | +} |
657 | + |
658 | +func (s *format_1_12Suite) newConfig(c *gc.C) *configInternal { |
659 | + params := agentParams |
660 | + params.DataDir = c.MkDir() |
661 | + config, err := newConfig(params) |
662 | + c.Assert(err, gc.IsNil) |
663 | + return config |
664 | +} |
665 | + |
666 | +func (s *format_1_12Suite) TestWriteAgentConfig(c *gc.C) { |
667 | + config := s.newConfig(c) |
668 | + err := s.formatter.write(config) |
669 | + c.Assert(err, gc.IsNil) |
670 | + |
671 | + expectedLocation := path.Join(config.Dir(), "agent.conf") |
672 | + fileInfo, err := os.Stat(expectedLocation) |
673 | + c.Assert(err, gc.IsNil) |
674 | + c.Assert(fileInfo.Mode().IsRegular(), jc.IsTrue) |
675 | + c.Assert(fileInfo.Mode().Perm(), gc.Equals, os.FileMode(0600)) |
676 | + c.Assert(fileInfo.Size(), jc.GreaterThan, 0) |
677 | +} |
678 | + |
679 | +func (s *format_1_12Suite) assertWriteAndRead(c *gc.C, config *configInternal) { |
680 | + err := s.formatter.write(config) |
681 | + c.Assert(err, gc.IsNil) |
682 | + // The readConfig is missing the dataDir initially. |
683 | + readConfig, err := s.formatter.read(config.Dir()) |
684 | + c.Assert(err, gc.IsNil) |
685 | + c.Assert(readConfig.dataDir, gc.Equals, "") |
686 | + // This is put in by the ReadConf method that we are avoiding using |
687 | + // becuase it will have side-effects soon around migrating configs. |
688 | + readConfig.dataDir = config.dataDir |
689 | + c.Assert(readConfig, gc.DeepEquals, config) |
690 | +} |
691 | + |
692 | +func (s *format_1_12Suite) TestRead(c *gc.C) { |
693 | + config := s.newConfig(c) |
694 | + s.assertWriteAndRead(c, config) |
695 | +} |
696 | + |
697 | +func (s *format_1_12Suite) TestWriteCommands(c *gc.C) { |
698 | + config := s.newConfig(c) |
699 | + commands, err := s.formatter.writeCommands(config) |
700 | + c.Assert(err, gc.IsNil) |
701 | + c.Assert(commands, gc.HasLen, 3) |
702 | + c.Assert(commands[0], gc.Matches, `mkdir -p '\S+/agents/omg'`) |
703 | + c.Assert(commands[1], gc.Matches, `install -m 600 /dev/null '\S+/agents/omg/agent.conf'`) |
704 | + c.Assert(commands[2], gc.Matches, `printf '%s\\n' '(.|\n)*' > '\S+/agents/omg/agent.conf'`) |
705 | +} |
706 | + |
707 | +func (s *format_1_12Suite) TestReadWriteStateConfig(c *gc.C) { |
708 | + stateParams := StateMachineConfigParams{ |
709 | + AgentConfigParams: agentParams, |
710 | + StateServerCert: []byte("some special cert"), |
711 | + StateServerKey: []byte("a special key"), |
712 | + StatePort: 12345, |
713 | + APIPort: 23456, |
714 | + } |
715 | + stateParams.DataDir = c.MkDir() |
716 | + configInterface, err := NewStateMachineConfig(stateParams) |
717 | + c.Assert(err, gc.IsNil) |
718 | + config, ok := configInterface.(*configInternal) |
719 | + c.Assert(ok, jc.IsTrue) |
720 | + |
721 | + s.assertWriteAndRead(c, config) |
722 | +} |
723 | |
724 | === added file 'agent/format.go' |
725 | --- agent/format.go 1970-01-01 00:00:00 +0000 |
726 | +++ agent/format.go 2013-09-02 21:47:29 +0000 |
727 | @@ -0,0 +1,51 @@ |
728 | +// Copyright 2013 Canonical Ltd. |
729 | +// Licensed under the AGPLv3, see LICENCE file for details. |
730 | + |
731 | +package agent |
732 | + |
733 | +import ( |
734 | + "fmt" |
735 | +) |
736 | + |
737 | +// The format file in the agent config directory is used to identify the |
738 | +// method of serialization. This is used by individual format readers and |
739 | +// writers to be able to translate from the file format to the in-memory |
740 | +// structure. |
741 | +// |
742 | +// Juju only supports upgrading from single steps, so Juju only needs to know |
743 | +// about the current format and the format of the previous stable release. |
744 | +// For convenience, the format name includes the version number of the stable |
745 | +// release that it will be released with. Once this release has happened, the |
746 | +// format should be considered FIXED, and should no longer be modified. If |
747 | +// changes are necessary to the format, a new format should be created. |
748 | +// |
749 | +// We don't need to create new formats for each release, the version number is |
750 | +// just a convenience for us to know which stable release introduced that |
751 | +// format. |
752 | + |
753 | +const ( |
754 | + formatFilename = "format" |
755 | + currentFormat = format_1_12 |
756 | +) |
757 | + |
758 | +var currentFormatter = &formatter_1_12{} |
759 | + |
760 | +// The formatter defines the two methods needed by the formatters for |
761 | +// translating to and from the internal, format agnostic, structure. |
762 | +type formatter interface { |
763 | + read(dirName string) (*configInternal, error) |
764 | + write(config *configInternal) error |
765 | + writeCommands(config *configInternal) ([]string, error) |
766 | +} |
767 | + |
768 | +func readFormat(dirName string) (string, error) { |
769 | + return currentFormat, nil |
770 | +} |
771 | + |
772 | +func newFormatter(format string) (formatter, error) { |
773 | + switch format { |
774 | + case currentFormat: |
775 | + return currentFormatter, nil |
776 | + } |
777 | + return nil, fmt.Errorf("unknown agent config format") |
778 | +} |
779 | |
780 | === added file 'agent/format_whitebox_test.go' |
781 | --- agent/format_whitebox_test.go 1970-01-01 00:00:00 +0000 |
782 | +++ agent/format_whitebox_test.go 2013-09-02 21:47:29 +0000 |
783 | @@ -0,0 +1,32 @@ |
784 | +// Copyright 2013 Canonical Ltd. |
785 | +// Licensed under the AGPLv3, see LICENCE file for details. |
786 | + |
787 | +package agent |
788 | + |
789 | +import ( |
790 | + gc "launchpad.net/gocheck" |
791 | + |
792 | + "launchpad.net/juju-core/testing" |
793 | +) |
794 | + |
795 | +type formatSuite struct { |
796 | + testing.LoggingSuite |
797 | +} |
798 | + |
799 | +var _ = gc.Suite(&formatSuite{}) |
800 | + |
801 | +func (*formatSuite) TestReadFormat(c *gc.C) { |
802 | + format, err := readFormat("ignored") |
803 | + c.Assert(format, gc.Equals, currentFormat) |
804 | + c.Assert(err, gc.IsNil) |
805 | +} |
806 | + |
807 | +func (*formatSuite) TestNewFormatter(c *gc.C) { |
808 | + formatter, err := newFormatter(currentFormat) |
809 | + c.Assert(formatter, gc.NotNil) |
810 | + c.Assert(err, gc.IsNil) |
811 | + |
812 | + formatter, err = newFormatter("other") |
813 | + c.Assert(formatter, gc.IsNil) |
814 | + c.Assert(err, gc.ErrorMatches, "unknown agent config format") |
815 | +} |
Reviewers: mp+183074_ code.launchpad. net,
Message:
Please take a look.
Description:
Add the formatter concept to agent config.
In order to decouple the internal structure of the agent config from how
it is
stored on disk, we introduce formatters. They are responsible for
moving the
internal config to and from a serialization format.
The existing format is called "format 1.12", as that is the stable
release
that the format was introduced.
Some of the methods are purposefully trivial at this stage, and they are
flushed out in the next branch.
https:/ /code.launchpad .net/~thumper/ juju-core/ agent-config- formatters/ +merge/ 183074
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/13237050/
Affected files: 1.12.go 1.12_whitebox_ test.go whitebox_ test.go
A [revision details]
M agent/agent.go
A agent/format-
A agent/format-
A agent/format.go
A agent/format_