Merge lp:~allenap/juju-core/maas-environment-uuid-use into lp:~go-bot/juju-core/trunk
- maas-environment-uuid-use
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Ian Booth |
Approved revision: | no longer in the source branch. |
Merged at revision: | 1985 |
Proposed branch: | lp:~allenap/juju-core/maas-environment-uuid-use |
Merge into: | lp:~go-bot/juju-core/trunk |
Diff against target: |
712 lines (+186/-76) 9 files modified
provider/maas/config.go (+16/-2) provider/maas/config_test.go (+17/-3) provider/maas/environ.go (+4/-2) provider/maas/environ_test.go (+29/-34) provider/maas/environprovider.go (+21/-1) provider/maas/environprovider_test.go (+44/-3) provider/maas/instance_test.go (+15/-13) provider/maas/maas_test.go (+24/-2) provider/maas/storage_test.go (+16/-16) |
To merge this branch: | bzr merge lp:~allenap/juju-core/maas-environment-uuid-use |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Tim Penhey (community) | conditional | Approve | |
Review via email: mp+191249@code.launchpad.net |
Commit message
Use environment-uuid when interacting with MAAS
The environment-uuid is passed - as agent_name - to MAAS when acquiring
and listing nodes. This will allow multiple Juju environments to coexist
in a single MAAS user's account.
In addition, it prevents environment-uuid from being set in
environments.yaml, and returns an appropriate error message. This is
intended to stop people from inadvertently clobbering existing
environments.
It also starts cleaning up MAAS's providerSuite by dropping the environ
attribute in favour of makeEnviron(). The former was initialised to a
half-working environ that caused strange test failures.
Description of the change
Use environment-uuid when interacting with MAAS
The environment-uuid is passed - as agent_name - to MAAS when acquiring
and listing nodes. This will allow multiple Juju environments to coexist
in a single MAAS user's account.
In addition, it prevents environment-uuid from being set in
environments.yaml, and returns an appropriate error message. This is
intended to stop people from inadvertently clobbering existing
environments.
It also starts cleaning up MAAS's providerSuite by dropping the environ
attribute in favour of makeEnviron(). The former was initialised to a
half-working environ that caused strange test failures.
Gavin Panella (allenap) wrote : | # |
Gavin Panella (allenap) wrote : | # |
Please take a look.
Tim Penhey (thumper) wrote : | # |
LGTM - with one question and one concern.
What does older MAAS installs do with unexpected GET parameters? When
the new agent_name field is passed through, is this going to cause
problems with older systems? Also, I wonder where we should document
the required versions.
I foresee a problem when a new juju is running against an older MAAS,
the new agent_name is passed through, and also passed through as a
filter when listing. Is the unknown field similarly ignored by the
filter code?
https:/
File provider/
https:/
provider/
I do think that we should move this into the general environ config, but
this can happen later. For expediency, I'm happy to have this just in
MAAS for now.
https:/
File provider/
https:/
provider/
environ.
Is "agent_name" a new MAAS parameter? The current docs don't list it.
https:/
provider/
environ.
Beautiful :-)
https:/
File provider/
https:/
provider/
Didn't this bit land as r1984 on lp:juju-core?
https:/
File provider/
https:/
provider/
TestStringWitho
Thank you. There is a tendency in some juju tests to have too much
tested in one test.
Tim Penhey (thumper) wrote : | # |
As long as the "agent_name" is ignored by older MAAS versions, this should be fine.
Julian Edwards (julian-edwards) wrote : | # |
It will be ignored, by design. This is a good thing, it basically leaves the functionality unaltered on old maases.
Preview Diff
1 | === modified file 'provider/maas/config.go' |
2 | --- provider/maas/config.go 2013-08-09 04:58:34 +0000 |
3 | +++ provider/maas/config.go 2013-10-15 16:59:34 +0000 |
4 | @@ -18,8 +18,15 @@ |
5 | // maas-oauth is a colon-separated triplet of: |
6 | // consumer-key:resource-token:resource-secret |
7 | "maas-oauth": schema.String(), |
8 | -} |
9 | -var configDefaults = schema.Defaults{} |
10 | + // environment-uuid is an optional UUID to group the machines |
11 | + // acquired from MAAS, to support multiple environments per MAAS user. |
12 | + "environment-uuid": schema.String(), |
13 | +} |
14 | +var configDefaults = schema.Defaults{ |
15 | + // For backward-compatibility, environment-uuid is the empty string |
16 | + // by default. However, new environments should all use a UUID. |
17 | + "environment-uuid": "", |
18 | +} |
19 | |
20 | type maasEnvironConfig struct { |
21 | *config.Config |
22 | @@ -34,6 +41,13 @@ |
23 | return cfg.attrs["maas-oauth"].(string) |
24 | } |
25 | |
26 | +func (cfg *maasEnvironConfig) maasEnvironmentUUID() string { |
27 | + if uuid, ok := cfg.attrs["environment-uuid"].(string); ok { |
28 | + return uuid |
29 | + } |
30 | + return "" |
31 | +} |
32 | + |
33 | func (prov maasEnvironProvider) newConfig(cfg *config.Config) (*maasEnvironConfig, error) { |
34 | validCfg, err := prov.Validate(cfg, nil) |
35 | if err != nil { |
36 | |
37 | === modified file 'provider/maas/config_test.go' |
38 | --- provider/maas/config_test.go 2013-09-20 02:33:04 +0000 |
39 | +++ provider/maas/config_test.go 2013-10-15 16:59:34 +0000 |
40 | @@ -9,6 +9,7 @@ |
41 | "launchpad.net/juju-core/environs" |
42 | "launchpad.net/juju-core/testing" |
43 | "launchpad.net/juju-core/testing/testbase" |
44 | + "launchpad.net/juju-core/utils" |
45 | ) |
46 | |
47 | type configSuite struct { |
48 | @@ -43,17 +44,30 @@ |
49 | server := "http://maas.testing.invalid/maas/" |
50 | oauth := "consumer-key:resource-token:resource-secret" |
51 | future := "futurama" |
52 | + uuid, err := utils.NewUUID() |
53 | + c.Assert(err, gc.IsNil) |
54 | ecfg, err := newConfig(map[string]interface{}{ |
55 | - "maas-server": server, |
56 | - "maas-oauth": oauth, |
57 | - "future-key": future, |
58 | + "maas-server": server, |
59 | + "maas-oauth": oauth, |
60 | + "environment-uuid": uuid.String(), |
61 | + "future-key": future, |
62 | }) |
63 | c.Assert(err, gc.IsNil) |
64 | c.Check(ecfg.maasServer(), gc.Equals, server) |
65 | c.Check(ecfg.maasOAuth(), gc.DeepEquals, oauth) |
66 | + c.Check(ecfg.maasEnvironmentUUID(), gc.Equals, uuid.String()) |
67 | c.Check(ecfg.UnknownAttrs()["future-key"], gc.DeepEquals, future) |
68 | } |
69 | |
70 | +func (*configSuite) TestEnvironmentUUIDDefault(c *gc.C) { |
71 | + ecfg, err := newConfig(map[string]interface{}{ |
72 | + "maas-server": "http://maas.testing.invalid/maas/", |
73 | + "maas-oauth": "consumer-key:resource-token:resource-secret", |
74 | + }) |
75 | + c.Assert(err, gc.IsNil) |
76 | + c.Check(ecfg.maasEnvironmentUUID(), gc.Equals, "") |
77 | +} |
78 | + |
79 | func (*configSuite) TestChecksWellFormedMaasServer(c *gc.C) { |
80 | _, err := newConfig(map[string]interface{}{ |
81 | "maas-server": "This should have been a URL.", |
82 | |
83 | === modified file 'provider/maas/environ.go' |
84 | --- provider/maas/environ.go 2013-10-02 00:29:29 +0000 |
85 | +++ provider/maas/environ.go 2013-10-15 16:59:34 +0000 |
86 | @@ -170,12 +170,13 @@ |
87 | |
88 | // acquireNode allocates a node from the MAAS. |
89 | func (environ *maasEnviron) acquireNode(cons constraints.Value, possibleTools tools.List) (gomaasapi.MAASObject, *tools.Tools, error) { |
90 | - constraintsParams := convertConstraints(cons) |
91 | + acquireParams := convertConstraints(cons) |
92 | + acquireParams.Add("agent_name", environ.ecfg().maasEnvironmentUUID()) |
93 | var result gomaasapi.JSONObject |
94 | var err error |
95 | for a := shortAttempt.Start(); a.Next(); { |
96 | client := environ.getMAASClient().GetSubObject("nodes/") |
97 | - result, err = client.CallPost("acquire", constraintsParams) |
98 | + result, err = client.CallPost("acquire", acquireParams) |
99 | if err == nil { |
100 | break |
101 | } |
102 | @@ -322,6 +323,7 @@ |
103 | func (environ *maasEnviron) instances(ids []instance.Id) ([]instance.Instance, error) { |
104 | nodeListing := environ.getMAASClient().GetSubObject("nodes") |
105 | filter := getSystemIdValues(ids) |
106 | + filter.Add("agent_name", environ.ecfg().maasEnvironmentUUID()) |
107 | listNodeObjects, err := nodeListing.CallGet("list", filter) |
108 | if err != nil { |
109 | return nil, err |
110 | |
111 | === modified file 'provider/maas/environ_test.go' |
112 | --- provider/maas/environ_test.go 2013-10-01 12:03:33 +0000 |
113 | +++ provider/maas/environ_test.go 2013-10-15 16:59:34 +0000 |
114 | @@ -26,7 +26,6 @@ |
115 | "launchpad.net/juju-core/instance" |
116 | "launchpad.net/juju-core/juju/testing" |
117 | "launchpad.net/juju-core/provider/common" |
118 | - coretesting "launchpad.net/juju-core/testing" |
119 | jc "launchpad.net/juju-core/testing/checkers" |
120 | "launchpad.net/juju-core/tools" |
121 | "launchpad.net/juju-core/utils" |
122 | @@ -54,31 +53,12 @@ |
123 | return ecfg.Config |
124 | } |
125 | |
126 | -// makeEnviron creates a functional maasEnviron for a test. |
127 | -func (suite *environSuite) makeEnviron() *maasEnviron { |
128 | - attrs := coretesting.FakeConfig().Merge(coretesting.Attrs{ |
129 | - "name": suite.environ.Name(), |
130 | - "type": "maas", |
131 | - "maas-oauth": "a:b:c", |
132 | - "maas-server": suite.testMAASObject.TestServer.URL, |
133 | - }) |
134 | - cfg, err := config.New(config.NoDefaults, attrs) |
135 | - if err != nil { |
136 | - panic(err) |
137 | - } |
138 | - env, err := NewEnviron(cfg) |
139 | - if err != nil { |
140 | - panic(err) |
141 | - } |
142 | - return env |
143 | -} |
144 | - |
145 | func (suite *environSuite) setupFakeProviderStateFile(c *gc.C) { |
146 | suite.testMAASObject.TestServer.NewFile(common.StateFile, []byte("test file content")) |
147 | } |
148 | |
149 | func (suite *environSuite) setupFakeTools(c *gc.C) { |
150 | - stor := NewStorage(suite.environ) |
151 | + stor := NewStorage(suite.makeEnviron()) |
152 | envtesting.UploadFakeTools(c, stor) |
153 | } |
154 | |
155 | @@ -139,7 +119,7 @@ |
156 | resourceURI, _ := node.GetField("resource_uri") |
157 | instanceIds := []instance.Id{instance.Id(resourceURI)} |
158 | |
159 | - instances, err := suite.environ.Instances(instanceIds) |
160 | + instances, err := suite.makeEnviron().Instances(instanceIds) |
161 | |
162 | c.Check(err, gc.IsNil) |
163 | c.Check(len(instances), gc.Equals, 1) |
164 | @@ -149,7 +129,7 @@ |
165 | func (suite *environSuite) TestInstancesReturnsErrNoInstancesIfEmptyParameter(c *gc.C) { |
166 | input := `{"system_id": "test"}` |
167 | suite.testMAASObject.TestServer.NewNode(input) |
168 | - instances, err := suite.environ.Instances([]instance.Id{}) |
169 | + instances, err := suite.makeEnviron().Instances([]instance.Id{}) |
170 | |
171 | c.Check(err, gc.Equals, environs.ErrNoInstances) |
172 | c.Check(instances, gc.IsNil) |
173 | @@ -158,14 +138,14 @@ |
174 | func (suite *environSuite) TestInstancesReturnsErrNoInstancesIfNilParameter(c *gc.C) { |
175 | input := `{"system_id": "test"}` |
176 | suite.testMAASObject.TestServer.NewNode(input) |
177 | - instances, err := suite.environ.Instances(nil) |
178 | + instances, err := suite.makeEnviron().Instances(nil) |
179 | |
180 | c.Check(err, gc.Equals, environs.ErrNoInstances) |
181 | c.Check(instances, gc.IsNil) |
182 | } |
183 | |
184 | func (suite *environSuite) TestInstancesReturnsErrNoInstancesIfNoneFound(c *gc.C) { |
185 | - _, err := suite.environ.Instances([]instance.Id{"unknown"}) |
186 | + _, err := suite.makeEnviron().Instances([]instance.Id{"unknown"}) |
187 | c.Check(err, gc.Equals, environs.ErrNoInstances) |
188 | } |
189 | |
190 | @@ -174,7 +154,7 @@ |
191 | node := suite.testMAASObject.TestServer.NewNode(input) |
192 | resourceURI, _ := node.GetField("resource_uri") |
193 | |
194 | - instances, err := suite.environ.AllInstances() |
195 | + instances, err := suite.makeEnviron().AllInstances() |
196 | |
197 | c.Check(err, gc.IsNil) |
198 | c.Check(len(instances), gc.Equals, 1) |
199 | @@ -182,7 +162,7 @@ |
200 | } |
201 | |
202 | func (suite *environSuite) TestAllInstancesReturnsEmptySliceIfNoInstance(c *gc.C) { |
203 | - instances, err := suite.environ.AllInstances() |
204 | + instances, err := suite.makeEnviron().AllInstances() |
205 | |
206 | c.Check(err, gc.IsNil) |
207 | c.Check(len(instances), gc.Equals, 0) |
208 | @@ -198,7 +178,7 @@ |
209 | instanceId2 := instance.Id("unknown systemID") |
210 | instanceIds := []instance.Id{instanceId1, instanceId2} |
211 | |
212 | - instances, err := suite.environ.Instances(instanceIds) |
213 | + instances, err := suite.makeEnviron().Instances(instanceIds) |
214 | |
215 | c.Check(err, gc.Equals, environs.ErrPartialInstances) |
216 | c.Check(len(instances), gc.Equals, 1) |
217 | @@ -298,7 +278,7 @@ |
218 | } |
219 | |
220 | func (suite *environSuite) TestAcquireNode(c *gc.C) { |
221 | - stor := NewStorage(suite.environ) |
222 | + stor := NewStorage(suite.makeEnviron()) |
223 | fakeTools := envtesting.MustUploadFakeToolsVersions(stor, version.Current)[0] |
224 | env := suite.makeEnviron() |
225 | suite.testMAASObject.TestServer.NewNode(`{"system_id": "node0", "hostname": "host0"}`) |
226 | @@ -313,7 +293,7 @@ |
227 | } |
228 | |
229 | func (suite *environSuite) TestAcquireNodeTakesConstraintsIntoAccount(c *gc.C) { |
230 | - stor := NewStorage(suite.environ) |
231 | + stor := NewStorage(suite.makeEnviron()) |
232 | fakeTools := envtesting.MustUploadFakeToolsVersions(stor, version.Current)[0] |
233 | env := suite.makeEnviron() |
234 | suite.testMAASObject.TestServer.NewNode(`{"system_id": "node0", "hostname": "host0"}`) |
235 | @@ -329,6 +309,21 @@ |
236 | c.Assert(nodeRequestValues[0].Get("mem"), gc.Equals, "1024") |
237 | } |
238 | |
239 | +func (suite *environSuite) TestAcquireNodePassedEnvironmentUUID(c *gc.C) { |
240 | + stor := NewStorage(suite.makeEnviron()) |
241 | + fakeTools := envtesting.MustUploadFakeToolsVersions(stor, version.Current)[0] |
242 | + env := suite.makeEnviron() |
243 | + suite.testMAASObject.TestServer.NewNode(`{"system_id": "node0", "hostname": "host0"}`) |
244 | + |
245 | + _, _, err := env.acquireNode(constraints.Value{}, tools.List{fakeTools}) |
246 | + |
247 | + c.Check(err, gc.IsNil) |
248 | + requestValues := suite.testMAASObject.TestServer.NodeOperationRequestValues() |
249 | + nodeRequestValues, found := requestValues["node0"] |
250 | + c.Assert(found, gc.Equals, true) |
251 | + c.Assert(nodeRequestValues[0].Get("agent_name"), gc.Equals, exampleUUID) |
252 | +} |
253 | + |
254 | func (*environSuite) TestConvertConstraints(c *gc.C) { |
255 | var testValues = []struct { |
256 | constraints constraints.Value |
257 | @@ -352,13 +347,13 @@ |
258 | func (suite *environSuite) getInstance(systemId string) *maasInstance { |
259 | input := `{"system_id": "` + systemId + `"}` |
260 | node := suite.testMAASObject.TestServer.NewNode(input) |
261 | - return &maasInstance{&node, suite.environ} |
262 | + return &maasInstance{&node, suite.makeEnviron()} |
263 | } |
264 | |
265 | func (suite *environSuite) TestStopInstancesReturnsIfParameterEmpty(c *gc.C) { |
266 | suite.getInstance("test1") |
267 | |
268 | - err := suite.environ.StopInstances([]instance.Instance{}) |
269 | + err := suite.makeEnviron().StopInstances([]instance.Instance{}) |
270 | c.Check(err, gc.IsNil) |
271 | operations := suite.testMAASObject.TestServer.NodeOperations() |
272 | c.Check(operations, gc.DeepEquals, map[string][]string{}) |
273 | @@ -370,7 +365,7 @@ |
274 | suite.getInstance("test3") |
275 | instances := []instance.Instance{instance1, instance2} |
276 | |
277 | - err := suite.environ.StopInstances(instances) |
278 | + err := suite.makeEnviron().StopInstances(instances) |
279 | |
280 | c.Check(err, gc.IsNil) |
281 | operations := suite.testMAASObject.TestServer.NodeOperations() |
282 | @@ -383,7 +378,7 @@ |
283 | hostname := "test" |
284 | input := `{"system_id": "system_id", "hostname": "` + hostname + `"}` |
285 | node := suite.testMAASObject.TestServer.NewNode(input) |
286 | - testInstance := &maasInstance{&node, suite.environ} |
287 | + testInstance := &maasInstance{&node, suite.makeEnviron()} |
288 | err := common.SaveState( |
289 | env.Storage(), |
290 | &common.BootstrapState{StateInstances: []instance.Id{testInstance.Id()}}) |
291 | |
292 | === modified file 'provider/maas/environprovider.go' |
293 | --- provider/maas/environprovider.go 2013-10-11 14:05:04 +0000 |
294 | +++ provider/maas/environprovider.go 2013-10-15 16:59:34 +0000 |
295 | @@ -4,6 +4,7 @@ |
296 | package maas |
297 | |
298 | import ( |
299 | + "errors" |
300 | "os" |
301 | |
302 | "launchpad.net/loggo" |
303 | @@ -37,8 +38,27 @@ |
304 | return env, nil |
305 | } |
306 | |
307 | +var errUUIDAlreadySet = errors.New( |
308 | + "environment-uuid is already set; this should not be set by hand") |
309 | + |
310 | func (p maasEnvironProvider) Prepare(cfg *config.Config) (environs.Environ, error) { |
311 | - // TODO any attributes to prepare? |
312 | + attrs := cfg.UnknownAttrs() |
313 | + uuid, found := attrs["environment-uuid"] |
314 | + if found { |
315 | + if uuid != "" { |
316 | + return nil, errUUIDAlreadySet |
317 | + } |
318 | + } else { |
319 | + uuid, err := utils.NewUUID() |
320 | + if err != nil { |
321 | + return nil, err |
322 | + } |
323 | + attrs["environment-uuid"] = uuid.String() |
324 | + } |
325 | + cfg, err := cfg.Apply(attrs) |
326 | + if err != nil { |
327 | + return nil, err |
328 | + } |
329 | return p.Open(cfg) |
330 | } |
331 | |
332 | |
333 | === modified file 'provider/maas/environprovider_test.go' |
334 | --- provider/maas/environprovider_test.go 2013-09-25 17:04:52 +0000 |
335 | +++ provider/maas/environprovider_test.go 2013-10-15 16:59:34 +0000 |
336 | @@ -11,6 +11,7 @@ |
337 | |
338 | "launchpad.net/juju-core/environs/config" |
339 | "launchpad.net/juju-core/testing" |
340 | + "launchpad.net/juju-core/utils" |
341 | ) |
342 | |
343 | type EnvironProviderSuite struct { |
344 | @@ -31,13 +32,53 @@ |
345 | config, err := config.New(config.NoDefaults, attrs) |
346 | c.Assert(err, gc.IsNil) |
347 | |
348 | - secretAttrs, err := suite.environ.Provider().SecretAttrs(config) |
349 | + secretAttrs, err := suite.makeEnviron().Provider().SecretAttrs(config) |
350 | c.Assert(err, gc.IsNil) |
351 | |
352 | expectedAttrs := map[string]string{"maas-oauth": oauth} |
353 | c.Check(secretAttrs, gc.DeepEquals, expectedAttrs) |
354 | } |
355 | |
356 | +func (suite *EnvironProviderSuite) TestUnknownAttrsContainEnvironmentUUID(c *gc.C) { |
357 | + testJujuHome := c.MkDir() |
358 | + defer config.SetJujuHome(config.SetJujuHome(testJujuHome)) |
359 | + attrs := testing.FakeConfig().Merge(testing.Attrs{ |
360 | + "type": "maas", |
361 | + "maas-oauth": "aa:bb:cc", |
362 | + "maas-server": "http://maas.testing.invalid/maas/", |
363 | + }) |
364 | + config, err := config.New(config.NoDefaults, attrs) |
365 | + c.Assert(err, gc.IsNil) |
366 | + |
367 | + environ, err := suite.makeEnviron().Provider().Prepare(config) |
368 | + c.Assert(err, gc.IsNil) |
369 | + |
370 | + preparedConfig := environ.Config() |
371 | + unknownAttrs := preparedConfig.UnknownAttrs() |
372 | + |
373 | + uuid, ok := unknownAttrs["environment-uuid"] |
374 | + c.Assert(ok, gc.Equals, true) |
375 | + |
376 | + _, err = utils.UUIDFromString(uuid.(string)) |
377 | + c.Assert(err, gc.IsNil) |
378 | +} |
379 | + |
380 | +func (suite *EnvironProviderSuite) TestEnvironmentUUIDShouldNotBeSetByHand(c *gc.C) { |
381 | + testJujuHome := c.MkDir() |
382 | + defer config.SetJujuHome(config.SetJujuHome(testJujuHome)) |
383 | + attrs := testing.FakeConfig().Merge(testing.Attrs{ |
384 | + "type": "maas", |
385 | + "maas-oauth": "aa:bb:cc", |
386 | + "maas-server": "http://maas.testing.invalid/maas/", |
387 | + "environment-uuid": "foobar", |
388 | + }) |
389 | + config, err := config.New(config.NoDefaults, attrs) |
390 | + c.Assert(err, gc.IsNil) |
391 | + |
392 | + _, err = suite.makeEnviron().Provider().Prepare(config) |
393 | + c.Assert(err, gc.Equals, errUUIDAlreadySet) |
394 | +} |
395 | + |
396 | // create a temporary file with the given content. The file will be cleaned |
397 | // up at the end of the test calling this method. |
398 | func createTempFile(c *gc.C, content []byte) string { |
399 | @@ -65,7 +106,7 @@ |
400 | _MAASInstanceFilename = filename |
401 | defer func() { _MAASInstanceFilename = old_MAASInstanceFilename }() |
402 | |
403 | - provider := suite.environ.Provider() |
404 | + provider := suite.makeEnviron().Provider() |
405 | publicAddress, err := provider.PublicAddress() |
406 | c.Assert(err, gc.IsNil) |
407 | c.Check(publicAddress, gc.Equals, hostname) |
408 | @@ -85,7 +126,7 @@ |
409 | }) |
410 | config, err := config.New(config.NoDefaults, attrs) |
411 | c.Assert(err, gc.IsNil) |
412 | - env, err := suite.environ.Provider().Open(config) |
413 | + env, err := suite.makeEnviron().Provider().Open(config) |
414 | // When Open() fails (i.e. returns a non-nil error), it returns an |
415 | // environs.Environ interface object with a nil value and a nil |
416 | // type. |
417 | |
418 | === modified file 'provider/maas/instance_test.go' |
419 | --- provider/maas/instance_test.go 2013-10-10 11:40:54 +0000 |
420 | +++ provider/maas/instance_test.go 2013-10-15 16:59:34 +0000 |
421 | @@ -21,7 +21,7 @@ |
422 | jsonValue := `{"system_id": "system_id", "test": "test"}` |
423 | obj := s.testMAASObject.TestServer.NewNode(jsonValue) |
424 | resourceURI, _ := obj.GetField("resource_uri") |
425 | - instance := maasInstance{&obj, s.environ} |
426 | + instance := maasInstance{&obj, s.makeEnviron()} |
427 | |
428 | c.Check(string(instance.Id()), gc.Equals, resourceURI) |
429 | } |
430 | @@ -29,19 +29,21 @@ |
431 | func (s *instanceTest) TestString(c *gc.C) { |
432 | jsonValue := `{"hostname": "thethingintheplace", "system_id": "system_id", "test": "test"}` |
433 | obj := s.testMAASObject.TestServer.NewNode(jsonValue) |
434 | - instance := &maasInstance{&obj, s.environ} |
435 | + instance := &maasInstance{&obj, s.makeEnviron()} |
436 | hostname, err := instance.DNSName() |
437 | c.Assert(err, gc.IsNil) |
438 | expected := hostname + ":" + string(instance.Id()) |
439 | c.Assert(fmt.Sprint(instance), gc.Equals, expected) |
440 | +} |
441 | |
442 | +func (s *instanceTest) TestStringWithoutHostname(c *gc.C) { |
443 | // For good measure, test what happens if we don't have a hostname. |
444 | - jsonValue = `{"system_id": "system_id", "test": "test"}` |
445 | - obj = s.testMAASObject.TestServer.NewNode(jsonValue) |
446 | - instance = &maasInstance{&obj, s.environ} |
447 | - hostname, err = instance.DNSName() |
448 | + jsonValue := `{"system_id": "system_id", "test": "test"}` |
449 | + obj := s.testMAASObject.TestServer.NewNode(jsonValue) |
450 | + instance := &maasInstance{&obj, s.makeEnviron()} |
451 | + _, err := instance.DNSName() |
452 | c.Assert(err, gc.NotNil) |
453 | - expected = fmt.Sprintf("<DNSName failed: %q>", err) + ":" + string(instance.Id()) |
454 | + expected := fmt.Sprintf("<DNSName failed: %q>", err) + ":" + string(instance.Id()) |
455 | c.Assert(fmt.Sprint(instance), gc.Equals, expected) |
456 | } |
457 | |
458 | @@ -49,7 +51,7 @@ |
459 | jsonValue := `{"system_id": "system_id", "test": "test"}` |
460 | obj := s.testMAASObject.TestServer.NewNode(jsonValue) |
461 | s.testMAASObject.TestServer.ChangeNode("system_id", "test2", "test2") |
462 | - instance := maasInstance{&obj, s.environ} |
463 | + instance := maasInstance{&obj, s.makeEnviron()} |
464 | |
465 | err := instance.refreshInstance() |
466 | |
467 | @@ -62,7 +64,7 @@ |
468 | func (s *instanceTest) TestDNSName(c *gc.C) { |
469 | jsonValue := `{"hostname": "DNS name", "system_id": "system_id"}` |
470 | obj := s.testMAASObject.TestServer.NewNode(jsonValue) |
471 | - instance := maasInstance{&obj, s.environ} |
472 | + instance := maasInstance{&obj, s.makeEnviron()} |
473 | |
474 | dnsName, err := instance.DNSName() |
475 | |
476 | @@ -83,7 +85,7 @@ |
477 | "ip_addresses": [ "1.2.3.4", "fe80::d806:dbff:fe23:1199" ] |
478 | }` |
479 | obj := s.testMAASObject.TestServer.NewNode(jsonValue) |
480 | - inst := maasInstance{&obj, s.environ} |
481 | + inst := maasInstance{&obj, s.makeEnviron()} |
482 | |
483 | expected := []instance.Address{ |
484 | {Value: "testing.invalid", Type: instance.HostName, NetworkScope: instance.NetworkPublic}, |
485 | @@ -105,7 +107,7 @@ |
486 | "system_id": "system_id" |
487 | }` |
488 | obj := s.testMAASObject.TestServer.NewNode(jsonValue) |
489 | - inst := maasInstance{&obj, s.environ} |
490 | + inst := maasInstance{&obj, s.makeEnviron()} |
491 | |
492 | addr, err := inst.Addresses() |
493 | c.Assert(err, gc.IsNil) |
494 | @@ -121,7 +123,7 @@ |
495 | "ip_addresses": "incompatible" |
496 | }` |
497 | obj := s.testMAASObject.TestServer.NewNode(jsonValue) |
498 | - inst := maasInstance{&obj, s.environ} |
499 | + inst := maasInstance{&obj, s.makeEnviron()} |
500 | |
501 | _, err := inst.Addresses() |
502 | c.Assert(err, gc.NotNil) |
503 | @@ -134,7 +136,7 @@ |
504 | "ip_addresses": [42] |
505 | }` |
506 | obj := s.testMAASObject.TestServer.NewNode(jsonValue) |
507 | - inst := maasInstance{&obj, s.environ} |
508 | + inst := maasInstance{&obj, s.makeEnviron()} |
509 | |
510 | _, err := inst.Addresses() |
511 | c.Assert(err, gc.NotNil) |
512 | |
513 | === modified file 'provider/maas/maas_test.go' |
514 | --- provider/maas/maas_test.go 2013-09-20 02:53:59 +0000 |
515 | +++ provider/maas/maas_test.go 2013-10-15 16:59:34 +0000 |
516 | @@ -9,7 +9,9 @@ |
517 | gc "launchpad.net/gocheck" |
518 | "launchpad.net/gomaasapi" |
519 | |
520 | + "launchpad.net/juju-core/environs/config" |
521 | envtesting "launchpad.net/juju-core/environs/testing" |
522 | + coretesting "launchpad.net/juju-core/testing" |
523 | "launchpad.net/juju-core/testing/testbase" |
524 | ) |
525 | |
526 | @@ -20,7 +22,6 @@ |
527 | type providerSuite struct { |
528 | testbase.LoggingSuite |
529 | envtesting.ToolsFixture |
530 | - environ *maasEnviron |
531 | testMAASObject *gomaasapi.TestMAASObject |
532 | restoreTimeouts func() |
533 | } |
534 | @@ -32,7 +33,6 @@ |
535 | s.LoggingSuite.SetUpSuite(c) |
536 | TestMAASObject := gomaasapi.NewTestMAAS("1.0") |
537 | s.testMAASObject = TestMAASObject |
538 | - s.environ = &maasEnviron{name: "test env", maasClientUnlocked: &TestMAASObject.MAASObject} |
539 | } |
540 | |
541 | func (s *providerSuite) SetUpTest(c *gc.C) { |
542 | @@ -51,3 +51,25 @@ |
543 | s.restoreTimeouts() |
544 | s.LoggingSuite.TearDownSuite(c) |
545 | } |
546 | + |
547 | +const exampleUUID = "dfb69555-0bc4-4d1f-85f2-4ee390974984" |
548 | + |
549 | +// makeEnviron creates a functional maasEnviron for a test. |
550 | +func (suite *providerSuite) makeEnviron() *maasEnviron { |
551 | + attrs := coretesting.FakeConfig().Merge(coretesting.Attrs{ |
552 | + "name": "test env", |
553 | + "type": "maas", |
554 | + "maas-oauth": "a:b:c", |
555 | + "maas-server": suite.testMAASObject.TestServer.URL, |
556 | + "environment-uuid": exampleUUID, |
557 | + }) |
558 | + cfg, err := config.New(config.NoDefaults, attrs) |
559 | + if err != nil { |
560 | + panic(err) |
561 | + } |
562 | + env, err := NewEnviron(cfg) |
563 | + if err != nil { |
564 | + panic(err) |
565 | + } |
566 | + return env |
567 | +} |
568 | |
569 | === modified file 'provider/maas/storage_test.go' |
570 | --- provider/maas/storage_test.go 2013-09-18 22:54:32 +0000 |
571 | +++ provider/maas/storage_test.go 2013-10-15 16:59:34 +0000 |
572 | @@ -144,20 +144,20 @@ |
573 | |
574 | func (s *storageSuite) TestGetReturnsNotFoundErrorIfNotFound(c *gc.C) { |
575 | const filename = "lost-data" |
576 | - stor := NewStorage(s.environ) |
577 | + stor := NewStorage(s.makeEnviron()) |
578 | _, err := storage.Get(stor, filename) |
579 | c.Assert(err, jc.Satisfies, errors.IsNotFoundError) |
580 | } |
581 | |
582 | func (s *storageSuite) TestListReturnsEmptyIfNoFilesStored(c *gc.C) { |
583 | - stor := NewStorage(s.environ) |
584 | + stor := NewStorage(s.makeEnviron()) |
585 | listing, err := storage.List(stor, "") |
586 | c.Assert(err, gc.IsNil) |
587 | c.Check(listing, gc.DeepEquals, []string{}) |
588 | } |
589 | |
590 | func (s *storageSuite) TestListReturnsAllFilesIfPrefixEmpty(c *gc.C) { |
591 | - stor := NewStorage(s.environ) |
592 | + stor := NewStorage(s.makeEnviron()) |
593 | files := []string{"1a", "2b", "3c"} |
594 | for _, name := range files { |
595 | s.fakeStoredFile(stor, name) |
596 | @@ -169,7 +169,7 @@ |
597 | } |
598 | |
599 | func (s *storageSuite) TestListSortsResults(c *gc.C) { |
600 | - stor := NewStorage(s.environ) |
601 | + stor := NewStorage(s.makeEnviron()) |
602 | files := []string{"4d", "1a", "3c", "2b"} |
603 | for _, name := range files { |
604 | s.fakeStoredFile(stor, name) |
605 | @@ -181,7 +181,7 @@ |
606 | } |
607 | |
608 | func (s *storageSuite) TestListReturnsNoFilesIfNoFilesMatchPrefix(c *gc.C) { |
609 | - stor := NewStorage(s.environ) |
610 | + stor := NewStorage(s.makeEnviron()) |
611 | s.fakeStoredFile(stor, "foo") |
612 | |
613 | listing, err := storage.List(stor, "bar") |
614 | @@ -190,7 +190,7 @@ |
615 | } |
616 | |
617 | func (s *storageSuite) TestListReturnsOnlyFilesWithMatchingPrefix(c *gc.C) { |
618 | - stor := NewStorage(s.environ) |
619 | + stor := NewStorage(s.makeEnviron()) |
620 | s.fakeStoredFile(stor, "abc") |
621 | s.fakeStoredFile(stor, "xyz") |
622 | |
623 | @@ -200,7 +200,7 @@ |
624 | } |
625 | |
626 | func (s *storageSuite) TestListMatchesPrefixOnly(c *gc.C) { |
627 | - stor := NewStorage(s.environ) |
628 | + stor := NewStorage(s.makeEnviron()) |
629 | s.fakeStoredFile(stor, "abc") |
630 | s.fakeStoredFile(stor, "xabc") |
631 | |
632 | @@ -210,7 +210,7 @@ |
633 | } |
634 | |
635 | func (s *storageSuite) TestListOperatesOnFlatNamespace(c *gc.C) { |
636 | - stor := NewStorage(s.environ) |
637 | + stor := NewStorage(s.makeEnviron()) |
638 | s.fakeStoredFile(stor, "a/b/c/d") |
639 | |
640 | listing, err := storage.List(stor, "a/b") |
641 | @@ -233,7 +233,7 @@ |
642 | |
643 | func (s *storageSuite) TestURLReturnsURLCorrespondingToFile(c *gc.C) { |
644 | const filename = "my-file.txt" |
645 | - stor := NewStorage(s.environ).(*maasStorage) |
646 | + stor := NewStorage(s.makeEnviron()).(*maasStorage) |
647 | file := s.fakeStoredFile(stor, filename) |
648 | // The file contains an anon_resource_uri, which lacks a network part |
649 | // (but will probably contain a query part). anonURL will be the |
650 | @@ -256,7 +256,7 @@ |
651 | const filename = "broken-toaster.jpg" |
652 | contents := []byte("Contents here") |
653 | length := int64(len(contents)) |
654 | - stor := NewStorage(s.environ) |
655 | + stor := NewStorage(s.makeEnviron()) |
656 | |
657 | err := stor.Put(filename, bytes.NewReader(contents), length) |
658 | |
659 | @@ -271,7 +271,7 @@ |
660 | |
661 | func (s *storageSuite) TestPutOverwritesFile(c *gc.C) { |
662 | const filename = "foo.bar" |
663 | - stor := NewStorage(s.environ) |
664 | + stor := NewStorage(s.makeEnviron()) |
665 | s.fakeStoredFile(stor, filename) |
666 | newContents := []byte("Overwritten") |
667 | |
668 | @@ -292,7 +292,7 @@ |
669 | const filename = "xyzzyz.2.xls" |
670 | const length = 5 |
671 | contents := []byte("abcdefghijklmnopqrstuvwxyz") |
672 | - stor := NewStorage(s.environ) |
673 | + stor := NewStorage(s.makeEnviron()) |
674 | |
675 | err := stor.Put(filename, bytes.NewReader(contents), length) |
676 | c.Assert(err, gc.IsNil) |
677 | @@ -310,7 +310,7 @@ |
678 | const filename = "a-file-which-is-mine" |
679 | oldContents := []byte("abcdefghijklmnopqrstuvwxyz") |
680 | newContents := []byte("xyz") |
681 | - stor := NewStorage(s.environ) |
682 | + stor := NewStorage(s.makeEnviron()) |
683 | err := stor.Put(filename, bytes.NewReader(oldContents), int64(len(oldContents))) |
684 | c.Assert(err, gc.IsNil) |
685 | |
686 | @@ -329,7 +329,7 @@ |
687 | |
688 | func (s *storageSuite) TestRemoveDeletesFile(c *gc.C) { |
689 | const filename = "doomed.txt" |
690 | - stor := NewStorage(s.environ) |
691 | + stor := NewStorage(s.makeEnviron()) |
692 | s.fakeStoredFile(stor, filename) |
693 | |
694 | err := stor.Remove(filename) |
695 | @@ -345,7 +345,7 @@ |
696 | |
697 | func (s *storageSuite) TestRemoveIsIdempotent(c *gc.C) { |
698 | const filename = "half-a-file" |
699 | - stor := NewStorage(s.environ) |
700 | + stor := NewStorage(s.makeEnviron()) |
701 | s.fakeStoredFile(stor, filename) |
702 | |
703 | err := stor.Remove(filename) |
704 | @@ -358,7 +358,7 @@ |
705 | func (s *storageSuite) TestNamesMayHaveSlashes(c *gc.C) { |
706 | const filename = "name/with/slashes" |
707 | content := []byte("File contents") |
708 | - stor := NewStorage(s.environ) |
709 | + stor := NewStorage(s.makeEnviron()) |
710 | |
711 | err := stor.Put(filename, bytes.NewReader(content), int64(len(content))) |
712 | c.Assert(err, gc.IsNil) |
Reviewers: mp+191249_ code.launchpad. net,
Message:
Please take a look.
Description:
Use environment-uuid when interacting with MAAS
The environment-uuid is passed - as agent_name - to MAAS when acquiring
and listing nodes. This will allow multiple Juju environments to coexist
in a single MAAS user's account.
In addition, it prevents environment-uuid from being set in
environments.yaml, and returns an appropriate error message. This is
intended to stop people from inadvertently clobbering existing
environments.
It also starts cleaning up MAAS's providerSuite by dropping the environ
attribute in favour of makeEnviron(). The former was initialised to a
half-working environ that caused strange test failures.
https:/ /code.launchpad .net/~allenap/ juju-core/ maas-environmen t-uuid- use/+merge/ 191249
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/14644045/
Affected files (+188, -76 lines): maas/config. go maas/config_ test.go maas/environ. go maas/environ_ test.go maas/environpro vider.go maas/environpro vider_test. go maas/instance_ test.go maas/maas_ test.go maas/storage_ test.go
A [revision details]
M provider/
M provider/
M provider/
M provider/
M provider/
M provider/
M provider/
M provider/
M provider/