juju-core

Merge lp:~axwalk/juju-core/lp1237709-maasname into lp:~go-bot/juju-core/trunk

  1. lp1237709-maasname
  2. Merge into trunk
Proposed by Andrew Wilkins
Status: Merged
Approved by: Andrew Wilkins
Approved revision: no longer in the source branch.
Merged at revision: 2668
Proposed branch: lp:~axwalk/juju-core/lp1237709-maasname
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 109 lines (+41/-15)
2 files modified
provider/maas/environ.go (+7/-6)
provider/maas/environ_whitebox_test.go (+34/-9)
To merge this branch: bzr merge lp:~axwalk/juju-core/lp1237709-maasname
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+216844@code.launchpad.net

Commit message

provider/maas: support maas-name directives

We treat all MAAS placement directives as
maas-name, and pass it on as a constraint
to the acquire-node API.

Partially fixes lp:1237709

https://codereview.appspot.com/90470044/

Description of the change

provider/maas: support maas-name directives

We treat all MAAS placement directives as
maas-name, and pass it on as a constraint
to the acquire-node API.

Partially fixes lp:1237709

https://codereview.appspot.com/90470044/

To post a comment you must log in.
Revision history for this message
Andrew Wilkins (axwalk) wrote :
Download full text (5.4 KiB)

Reviewers: mp+216844_code.launchpad.net,

Message:
Please take a look.

Description:
provider/maas: support maas-name directives

We treat all MAAS placement directives as
maas-name, and pass it on as a constraint
to the acquire-node API.

Partially fixes lp:1237709

https://code.launchpad.net/~axwalk/juju-core/lp1237709-maasname/+merge/216844

(do not edit description out of merge proposal)

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

Affected files (+43, -15 lines):
   A [revision details]
   M provider/maas/environ.go
   M provider/maas/environ_whitebox_test.go

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

Index: provider/maas/environ.go
=== modified file 'provider/maas/environ.go'
--- provider/maas/environ.go 2014-04-22 09:23:39 +0000
+++ provider/maas/environ.go 2014-04-23 09:33:46 +0000
@@ -171,11 +171,7 @@
  }

  func (env *maasEnviron) PrecheckInstance(series string, cons
constraints.Value, placement string) error {
- // TODO(axw) 2014-04-22 #1237709
- // Handle maas-name placement directive.
- if placement != "" {
- return fmt.Errorf("unknown placement directive: %s", placement)
- }
+ // We treat all placement directives as maas-name.
   return nil
  }

@@ -279,10 +275,13 @@
  }

  // acquireNode allocates a node from the MAAS.
-func (environ *maasEnviron) acquireNode(cons constraints.Value,
includeNetworks, excludeNetworks []string, possibleTools tools.List)
(gomaasapi.MAASObject, *tools.Tools, error) {
+func (environ *maasEnviron) acquireNode(nodeName string, cons
constraints.Value, includeNetworks, excludeNetworks []string, possibleTools
tools.List) (gomaasapi.MAASObject, *tools.Tools, error) {
   acquireParams := convertConstraints(cons)
   addNetworks(acquireParams, includeNetworks, excludeNetworks)
   acquireParams.Add("agent_name", environ.ecfg().maasAgentName())
+ if nodeName != "" {
+ acquireParams.Add("name", nodeName)
+ }
   var result gomaasapi.JSONObject
   var err error
   for a := shortAttempt.Start(); a.Next(); {
@@ -414,7 +413,9 @@
  ) {
   var inst *maasInstance
   var err error
+ nodeName := args.Placement
   node, tools, err := environ.acquireNode(
+ nodeName,
    args.Constraints,
    args.MachineConfig.IncludeNetworks,
    args.MachineConfig.ExcludeNetworks,

Index: provider/maas/environ_whitebox_test.go
=== modified file 'provider/maas/environ_whitebox_test.go'
--- provider/maas/environ_whitebox_test.go 2014-04-21 11:07:50 +0000
+++ provider/maas/environ_whitebox_test.go 2014-04-23 09:33:46 +0000
@@ -270,13 +270,38 @@
   env := suite.makeEnviron()

suite.testMAASObject.TestServer.NewNode(`{"system_id": "node0", "hostname": "host0"}`)

- _, _, err := env.acquireNode(constraints.Value{}, nil, nil,
tools.List{fakeTools})
-
- c.Check(err, gc.IsNil)
- operations := suite.testMAASObject.TestServer.NodeOperations()
- actions, found := operations["node0"]
- c.Assert(found, gc.Equals, true)
- c.Check(actions, gc.DeepEqual...

Read more...

Revision history for this message
William Reade (fwereade) wrote :

LGTM.

https://codereview.appspot.com/90470044/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'provider/maas/environ.go'
--- provider/maas/environ.go 2014-04-22 09:23:39 +0000
+++ provider/maas/environ.go 2014-04-23 09:36:22 +0000
@@ -171,11 +171,7 @@
171}171}
172172
173func (env *maasEnviron) PrecheckInstance(series string, cons constraints.Value, placement string) error {173func (env *maasEnviron) PrecheckInstance(series string, cons constraints.Value, placement string) error {
174 // TODO(axw) 2014-04-22 #1237709174 // We treat all placement directives as maas-name.
175 // Handle maas-name placement directive.
176 if placement != "" {
177 return fmt.Errorf("unknown placement directive: %s", placement)
178 }
179 return nil175 return nil
180}176}
181177
@@ -279,10 +275,13 @@
279}275}
280276
281// acquireNode allocates a node from the MAAS.277// acquireNode allocates a node from the MAAS.
282func (environ *maasEnviron) acquireNode(cons constraints.Value, includeNetworks, excludeNetworks []string, possibleTools tools.List) (gomaasapi.MAASObject, *tools.Tools, error) {278func (environ *maasEnviron) acquireNode(nodeName string, cons constraints.Value, includeNetworks, excludeNetworks []string, possibleTools tools.List) (gomaasapi.MAASObject, *tools.Tools, error) {
283 acquireParams := convertConstraints(cons)279 acquireParams := convertConstraints(cons)
284 addNetworks(acquireParams, includeNetworks, excludeNetworks)280 addNetworks(acquireParams, includeNetworks, excludeNetworks)
285 acquireParams.Add("agent_name", environ.ecfg().maasAgentName())281 acquireParams.Add("agent_name", environ.ecfg().maasAgentName())
282 if nodeName != "" {
283 acquireParams.Add("name", nodeName)
284 }
286 var result gomaasapi.JSONObject285 var result gomaasapi.JSONObject
287 var err error286 var err error
288 for a := shortAttempt.Start(); a.Next(); {287 for a := shortAttempt.Start(); a.Next(); {
@@ -414,7 +413,9 @@
414) {413) {
415 var inst *maasInstance414 var inst *maasInstance
416 var err error415 var err error
416 nodeName := args.Placement
417 node, tools, err := environ.acquireNode(417 node, tools, err := environ.acquireNode(
418 nodeName,
418 args.Constraints,419 args.Constraints,
419 args.MachineConfig.IncludeNetworks,420 args.MachineConfig.IncludeNetworks,
420 args.MachineConfig.ExcludeNetworks,421 args.MachineConfig.ExcludeNetworks,
421422
=== modified file 'provider/maas/environ_whitebox_test.go'
--- provider/maas/environ_whitebox_test.go 2014-04-21 11:07:50 +0000
+++ provider/maas/environ_whitebox_test.go 2014-04-23 09:36:22 +0000
@@ -270,13 +270,38 @@
270 env := suite.makeEnviron()270 env := suite.makeEnviron()
271 suite.testMAASObject.TestServer.NewNode(`{"system_id": "node0", "hostname": "host0"}`)271 suite.testMAASObject.TestServer.NewNode(`{"system_id": "node0", "hostname": "host0"}`)
272272
273 _, _, err := env.acquireNode(constraints.Value{}, nil, nil, tools.List{fakeTools})273 _, _, err := env.acquireNode("", constraints.Value{}, nil, nil, tools.List{fakeTools})
274274
275 c.Check(err, gc.IsNil)275 c.Check(err, gc.IsNil)
276 operations := suite.testMAASObject.TestServer.NodeOperations()276 operations := suite.testMAASObject.TestServer.NodeOperations()
277 actions, found := operations["node0"]277 actions, found := operations["node0"]
278 c.Assert(found, gc.Equals, true)278 c.Assert(found, gc.Equals, true)
279 c.Check(actions, gc.DeepEquals, []string{"acquire"})279 c.Check(actions, gc.DeepEquals, []string{"acquire"})
280
281 // no "name" parameter should have been passed through
282 values := suite.testMAASObject.TestServer.NodeOperationRequestValues()["node0"][0]
283 _, found = values["name"]
284 c.Assert(found, jc.IsFalse)
285}
286
287func (suite *environSuite) TestAcquireNodeByName(c *gc.C) {
288 stor := NewStorage(suite.makeEnviron())
289 fakeTools := envtesting.MustUploadFakeToolsVersions(stor, version.Current)[0]
290 env := suite.makeEnviron()
291 suite.testMAASObject.TestServer.NewNode(`{"system_id": "node0", "hostname": "host0"}`)
292
293 _, _, err := env.acquireNode("host0", constraints.Value{}, nil, nil, tools.List{fakeTools})
294
295 c.Check(err, gc.IsNil)
296 operations := suite.testMAASObject.TestServer.NodeOperations()
297 actions, found := operations["node0"]
298 c.Assert(found, gc.Equals, true)
299 c.Check(actions, gc.DeepEquals, []string{"acquire"})
300
301 // no "name" parameter should have been passed through
302 values := suite.testMAASObject.TestServer.NodeOperationRequestValues()["node0"][0]
303 nodeName := values.Get("name")
304 c.Assert(nodeName, gc.Equals, "host0")
280}305}
281306
282func (suite *environSuite) TestAcquireNodeTakesConstraintsIntoAccount(c *gc.C) {307func (suite *environSuite) TestAcquireNodeTakesConstraintsIntoAccount(c *gc.C) {
@@ -286,7 +311,7 @@
286 suite.testMAASObject.TestServer.NewNode(`{"system_id": "node0", "hostname": "host0"}`)311 suite.testMAASObject.TestServer.NewNode(`{"system_id": "node0", "hostname": "host0"}`)
287 constraints := constraints.Value{Arch: stringp("arm"), Mem: uint64p(1024)}312 constraints := constraints.Value{Arch: stringp("arm"), Mem: uint64p(1024)}
288313
289 _, _, err := env.acquireNode(constraints, nil, nil, tools.List{fakeTools})314 _, _, err := env.acquireNode("", constraints, nil, nil, tools.List{fakeTools})
290315
291 c.Check(err, gc.IsNil)316 c.Check(err, gc.IsNil)
292 requestValues := suite.testMAASObject.TestServer.NodeOperationRequestValues()317 requestValues := suite.testMAASObject.TestServer.NodeOperationRequestValues()
@@ -302,7 +327,7 @@
302 env := suite.makeEnviron()327 env := suite.makeEnviron()
303 suite.testMAASObject.TestServer.NewNode(`{"system_id": "node0", "hostname": "host0"}`)328 suite.testMAASObject.TestServer.NewNode(`{"system_id": "node0", "hostname": "host0"}`)
304329
305 _, _, err := env.acquireNode(constraints.Value{}, nil, nil, tools.List{fakeTools})330 _, _, err := env.acquireNode("", constraints.Value{}, nil, nil, tools.List{fakeTools})
306331
307 c.Check(err, gc.IsNil)332 c.Check(err, gc.IsNil)
308 requestValues := suite.testMAASObject.TestServer.NodeOperationRequestValues()333 requestValues := suite.testMAASObject.TestServer.NodeOperationRequestValues()

Subscribers

People subscribed via source and target branches

to status/vote changes: