launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/gomaasapi/acquire into lp:gomaasapi.

Commit message:
Support node "acquire" operation.  Also, make unknown operations Bad Request errors instead of Not Found.

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~jtv/gomaasapi/acquire/+merge/152951

This is needed for us to be able to test StartInstance in the juju provider.

I had wanted to avoid mirroring the MAAS API in gomaasapi precisely so we could avoid this kind of cross-project update, but we needed the test service.  It's tempting to re-implement the whole MAAS API in the test service, but obviously that won't do!


Jeroen
-- 
https://code.launchpad.net/~jtv/gomaasapi/acquire/+merge/152951
Your team MAAS Maintainers is requested to review the proposed merge of lp:~jtv/gomaasapi/acquire into lp:gomaasapi.

=== modified file 'testservice.go'
--- testservice.go	2013-03-08 10:57:42 +0000
+++ testservice.go	2013-03-12 16:08:41 +0000
@@ -56,6 +56,7 @@
 	serveMux       *http.ServeMux
 	client         Client
 	nodes          map[string]MAASObject
+	ownedNodes     map[string]bool
 	nodeOperations map[string][]string
 	files          map[string]MAASObject
 	version        string
@@ -75,6 +76,7 @@
 // (nodes, recorded operations, etc.).
 func (server *TestServer) Clear() {
 	server.nodes = make(map[string]MAASObject)
+	server.ownedNodes = make(map[string]bool)
 	server.nodeOperations = make(map[string][]string)
 	server.files = make(map[string]MAASObject)
 }
@@ -114,12 +116,18 @@
 	return obj
 }
 
-// Returns a map associating all the nodes' system ids with the nodes'
+// Nodes returns a map associating all the nodes' system ids with the nodes'
 // objects.
 func (server *TestServer) Nodes() map[string]MAASObject {
 	return server.nodes
 }
 
+// OwnedNodes returns a map whose keys represent the nodes that are currently
+// allocated.
+func (server *TestServer) OwnedNodes() map[string]bool {
+	return server.ownedNodes
+}
+
 // NewFile creates a file in the test MAAS server.
 func (server *TestServer) NewFile(filename string, filecontent []byte) MAASObject {
 	attrs := make(map[string]interface{})
@@ -205,9 +213,8 @@
 	nodeURLMatch := nodeURLRE.FindStringSubmatch(r.URL.Path)
 	nodeListingURL := getNodeListingURL(server.version)
 	switch {
-	case r.Method == "GET" && op == "list" && r.URL.Path == nodeListingURL:
-		// Node listing operation.
-		nodeListingHandler(server, w, r)
+	case r.URL.Path == nodeListingURL:
+		nodesTopLevelHandler(server, w, r, op)
 	case nodeURLMatch != nil:
 		// Request for a single node.
 		nodeHandler(server, w, r, nodeURLMatch[1], op)
@@ -240,6 +247,10 @@
 			// Record operation on node.
 			server.addNodeOperation(systemId, operation)
 
+			if operation == "release" {
+				delete(server.OwnedNodes(), systemId)
+			}
+
 			w.WriteHeader(http.StatusOK)
 			fmt.Fprint(w, marshalNode(node))
 			return
@@ -282,6 +293,47 @@
 	fmt.Fprint(w, string(res))
 }
 
+// findFreeNode looks for a node that is currently available.
+func findFreeNode(server *TestServer) *MAASObject {
+	for systemID, node := range server.Nodes() {
+		_, present := server.OwnedNodes()[systemID]
+		if !present {
+			return &node
+		}
+	}
+	return nil
+}
+
+// nodesAcquireHandler simulates acquiring a node.
+func nodesAcquireHandler(server *TestServer, w http.ResponseWriter, r *http.Request) {
+	node := findFreeNode(server)
+	if node == nil {
+		w.WriteHeader(http.StatusConflict)
+	} else {
+		systemID, err := node.GetField("system_id")
+		checkError(err)
+		server.OwnedNodes()[systemID] = true
+		res, err := json.Marshal(node)
+		checkError(err)
+		w.WriteHeader(http.StatusOK)
+		fmt.Fprint(w, string(res))
+	}
+}
+
+// nodesTopLevelHandler handles a request for /api/<version>/nodes/
+// (with no node id following as part of the path).
+func nodesTopLevelHandler(server *TestServer, w http.ResponseWriter, r *http.Request, op string) {
+	switch {
+	case r.Method == "GET" && op == "list":
+		// Node listing operation.
+		nodeListingHandler(server, w, r)
+	case r.Method == "POST" && op == "acquire":
+		nodesAcquireHandler(server, w, r)
+	default:
+		w.WriteHeader(http.StatusBadRequest)
+	}
+}
+
 // filesHandler handles requests for '/api/<version>/files/*'.
 func filesHandler(server *TestServer, w http.ResponseWriter, r *http.Request) {
 	values, err := url.ParseQuery(r.URL.RawQuery)

=== modified file 'testservice_test.go'
--- testservice_test.go	2013-03-08 10:56:45 +0000
+++ testservice_test.go	2013-03-12 16:08:41 +0000
@@ -47,6 +47,15 @@
 	c.Check(getNodeURI("version", "test"), Equals, "/api/version/nodes/test/")
 }
 
+func (suite *TestServerSuite) TestInvalidOperationOnNodesIsBadRequest(c *C) {
+	badURL := getNodeListingURL(suite.server.version) + "?op=procrastinate"
+
+	response, err := http.Get(suite.server.Server.URL + badURL)
+	c.Assert(err, IsNil)
+
+	c.Check(response.StatusCode, Equals, http.StatusBadRequest)
+}
+
 func (suite *TestServerSuite) TestHandlesNodeListingUnknownPath(c *C) {
 	invalidPath := fmt.Sprintf("/api/%s/nodes/invalid/path/", suite.server.version)
 	resp, err := http.Get(suite.server.Server.URL + invalidPath)
@@ -468,3 +477,56 @@
 	c.Assert(err, IsNil)
 	c.Check(field, Equals, base64.StdEncoding.EncodeToString([]byte(fileContent)))
 }
+
+func (suite *TestMAASObjectSuite) TestAcquireNodeGrabsAvailableNode(c *C) {
+	input := `{"system_id": "nodeid"}`
+	suite.TestMAASObject.TestServer.NewNode(input)
+	nodesObj := suite.TestMAASObject.GetSubObject("nodes/")
+
+	jsonResponse, err := nodesObj.CallPost("acquire", nil)
+	c.Assert(err, IsNil)
+
+	acquiredNode, err := jsonResponse.GetMAASObject()
+	c.Assert(err, IsNil)
+	systemID, err := acquiredNode.GetField("system_id")
+	c.Assert(err, IsNil)
+	c.Check(systemID, Equals, "nodeid")
+	_, owned := suite.TestMAASObject.TestServer.OwnedNodes()[systemID]
+	c.Check(owned, Equals, true)
+}
+
+func (suite *TestMAASObjectSuite) TestAcquireNodeNeedsANode(c *C) {
+	nodesObj := suite.TestMAASObject.GetSubObject("nodes/")
+	_, err := nodesObj.CallPost("acquire", nil)
+	c.Check(err.(ServerError).StatusCode, Equals, http.StatusConflict)
+}
+
+func (suite *TestMAASObjectSuite) TestAcquireNodeIgnoresOwnedNodes(c *C) {
+	input := `{"system_id": "nodeid"}`
+	suite.TestMAASObject.TestServer.NewNode(input)
+	nodesObj := suite.TestMAASObject.GetSubObject("nodes/")
+	// Ensure that the one node in the MAAS is not available.
+	_, err := nodesObj.CallPost("acquire", nil)
+	c.Assert(err, IsNil)
+
+	_, err = nodesObj.CallPost("acquire", nil)
+	c.Check(err.(ServerError).StatusCode, Equals, http.StatusConflict)
+}
+
+func (suite *TestMAASObjectSuite) TestReleaseNodeReleasesAcquiredNode(c *C) {
+	input := `{"system_id": "nodeid"}`
+	suite.TestMAASObject.TestServer.NewNode(input)
+	nodesObj := suite.TestMAASObject.GetSubObject("nodes/")
+	jsonResponse, err := nodesObj.CallPost("acquire", nil)
+	c.Assert(err, IsNil)
+	acquiredNode, err := jsonResponse.GetMAASObject()
+	c.Assert(err, IsNil)
+	systemID, err := acquiredNode.GetField("system_id")
+	c.Assert(err, IsNil)
+	nodeObj := nodesObj.GetSubObject(systemID)
+
+	_, err = nodeObj.CallPost("release", nil)
+	c.Assert(err, IsNil)
+	_, owned := suite.TestMAASObject.TestServer.OwnedNodes()[systemID]
+	c.Check(owned, Equals, false)
+}