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
1=== modified file 'provider/maas/environ.go'
2--- provider/maas/environ.go 2014-04-22 09:23:39 +0000
3+++ provider/maas/environ.go 2014-04-23 09:36:22 +0000
4@@ -171,11 +171,7 @@
5 }
6
7 func (env *maasEnviron) PrecheckInstance(series string, cons constraints.Value, placement string) error {
8- // TODO(axw) 2014-04-22 #1237709
9- // Handle maas-name placement directive.
10- if placement != "" {
11- return fmt.Errorf("unknown placement directive: %s", placement)
12- }
13+ // We treat all placement directives as maas-name.
14 return nil
15 }
16
17@@ -279,10 +275,13 @@
18 }
19
20 // acquireNode allocates a node from the MAAS.
21-func (environ *maasEnviron) acquireNode(cons constraints.Value, includeNetworks, excludeNetworks []string, possibleTools tools.List) (gomaasapi.MAASObject, *tools.Tools, error) {
22+func (environ *maasEnviron) acquireNode(nodeName string, cons constraints.Value, includeNetworks, excludeNetworks []string, possibleTools tools.List) (gomaasapi.MAASObject, *tools.Tools, error) {
23 acquireParams := convertConstraints(cons)
24 addNetworks(acquireParams, includeNetworks, excludeNetworks)
25 acquireParams.Add("agent_name", environ.ecfg().maasAgentName())
26+ if nodeName != "" {
27+ acquireParams.Add("name", nodeName)
28+ }
29 var result gomaasapi.JSONObject
30 var err error
31 for a := shortAttempt.Start(); a.Next(); {
32@@ -414,7 +413,9 @@
33 ) {
34 var inst *maasInstance
35 var err error
36+ nodeName := args.Placement
37 node, tools, err := environ.acquireNode(
38+ nodeName,
39 args.Constraints,
40 args.MachineConfig.IncludeNetworks,
41 args.MachineConfig.ExcludeNetworks,
42
43=== modified file 'provider/maas/environ_whitebox_test.go'
44--- provider/maas/environ_whitebox_test.go 2014-04-21 11:07:50 +0000
45+++ provider/maas/environ_whitebox_test.go 2014-04-23 09:36:22 +0000
46@@ -270,13 +270,38 @@
47 env := suite.makeEnviron()
48 suite.testMAASObject.TestServer.NewNode(`{"system_id": "node0", "hostname": "host0"}`)
49
50- _, _, err := env.acquireNode(constraints.Value{}, nil, nil, tools.List{fakeTools})
51-
52- c.Check(err, gc.IsNil)
53- operations := suite.testMAASObject.TestServer.NodeOperations()
54- actions, found := operations["node0"]
55- c.Assert(found, gc.Equals, true)
56- c.Check(actions, gc.DeepEquals, []string{"acquire"})
57+ _, _, err := env.acquireNode("", constraints.Value{}, nil, nil, tools.List{fakeTools})
58+
59+ c.Check(err, gc.IsNil)
60+ operations := suite.testMAASObject.TestServer.NodeOperations()
61+ actions, found := operations["node0"]
62+ c.Assert(found, gc.Equals, true)
63+ c.Check(actions, gc.DeepEquals, []string{"acquire"})
64+
65+ // no "name" parameter should have been passed through
66+ values := suite.testMAASObject.TestServer.NodeOperationRequestValues()["node0"][0]
67+ _, found = values["name"]
68+ c.Assert(found, jc.IsFalse)
69+}
70+
71+func (suite *environSuite) TestAcquireNodeByName(c *gc.C) {
72+ stor := NewStorage(suite.makeEnviron())
73+ fakeTools := envtesting.MustUploadFakeToolsVersions(stor, version.Current)[0]
74+ env := suite.makeEnviron()
75+ suite.testMAASObject.TestServer.NewNode(`{"system_id": "node0", "hostname": "host0"}`)
76+
77+ _, _, err := env.acquireNode("host0", constraints.Value{}, nil, nil, tools.List{fakeTools})
78+
79+ c.Check(err, gc.IsNil)
80+ operations := suite.testMAASObject.TestServer.NodeOperations()
81+ actions, found := operations["node0"]
82+ c.Assert(found, gc.Equals, true)
83+ c.Check(actions, gc.DeepEquals, []string{"acquire"})
84+
85+ // no "name" parameter should have been passed through
86+ values := suite.testMAASObject.TestServer.NodeOperationRequestValues()["node0"][0]
87+ nodeName := values.Get("name")
88+ c.Assert(nodeName, gc.Equals, "host0")
89 }
90
91 func (suite *environSuite) TestAcquireNodeTakesConstraintsIntoAccount(c *gc.C) {
92@@ -286,7 +311,7 @@
93 suite.testMAASObject.TestServer.NewNode(`{"system_id": "node0", "hostname": "host0"}`)
94 constraints := constraints.Value{Arch: stringp("arm"), Mem: uint64p(1024)}
95
96- _, _, err := env.acquireNode(constraints, nil, nil, tools.List{fakeTools})
97+ _, _, err := env.acquireNode("", constraints, nil, nil, tools.List{fakeTools})
98
99 c.Check(err, gc.IsNil)
100 requestValues := suite.testMAASObject.TestServer.NodeOperationRequestValues()
101@@ -302,7 +327,7 @@
102 env := suite.makeEnviron()
103 suite.testMAASObject.TestServer.NewNode(`{"system_id": "node0", "hostname": "host0"}`)
104
105- _, _, err := env.acquireNode(constraints.Value{}, nil, nil, tools.List{fakeTools})
106+ _, _, err := env.acquireNode("", constraints.Value{}, nil, nil, tools.List{fakeTools})
107
108 c.Check(err, gc.IsNil)
109 requestValues := suite.testMAASObject.TestServer.NodeOperationRequestValues()

Subscribers

People subscribed via source and target branches

to status/vote changes: