From d5a103d01b4104ce93f6c068dece2fdc0d186d39 Mon Sep 17 00:00:00 2001 From: Tim Penhey Date: Mon, 4 Apr 2016 12:41:29 +1200 Subject: [PATCH 1/6] Create a device. --- controller.go | 33 +++++++++++++++++++++++++++++++++ device.go | 26 ++++++++++++++++++++++++-- interface.go | 1 + 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/controller.go b/controller.go index e926345..9c0f8cf 100644 --- a/controller.go +++ b/controller.go @@ -194,6 +194,39 @@ func (c *controller) Devices(args DevicesArgs) ([]Device, error) { return result, nil } +// CreateDeviceArgs is a argument struct for passing information into CreateDevice. +type CreateDeviceArgs struct { + Hostname string + MACAddresses []string + Domain string + Parent string +} + +// Devices implements Controller. +func (c *controller) CreateDevice(args CreateDeviceArgs) (Device, error) { + params := NewURLParams() + params.MaybeAdd("hostname", args.Hostname) + params.MaybeAdd("domain", args.Domain) + params.MaybeAddMany("mac_addresses", args.MACAddresses) + params.MaybeAdd("parent", args.Parent) + result, err := c.post("devices", "create", params.Values) + if err != nil { + if svrErr, ok := errors.Cause(err).(ServerError); ok { + if svrErr.StatusCode == http.StatusConflict { + return nil, errors.Wrap(err, NewNoMatchError(svrErr.BodyMessage)) + } + } + // Translate http errors. + return nil, NewUnexpectedError(err) + } + + device, err := readDevice(c.apiVersion, result) + if err != nil { + return nil, errors.Trace(err) + } + return device, nil +} + // MachinesArgs is a argument struct for selecting Machines. // Only machines that match the specified criteria are returned. type MachinesArgs struct { diff --git a/device.go b/device.go index 2a2b488..b807150 100644 --- a/device.go +++ b/device.go @@ -45,14 +45,37 @@ func (d *device) Zone() Zone { return d.zone } +func readDevice(controllerVersion version.Number, source interface{}) (*device, error) { + readFunc, err := getDeviceDeserializationFunc(controllerVersion) + if err != nil { + return nil, errors.Trace(err) + } + + checker := schema.StringMap(schema.Any()) + coerced, err := checker.Coerce(source, nil) + if err != nil { + return nil, WrapWithDeserializationError(err, "device base schema check failed") + } + valid := coerced.(map[string]interface{}) + return readFunc(valid) +} + func readDevices(controllerVersion version.Number, source interface{}) ([]*device, error) { + readFunc, err := getDeviceDeserializationFunc(controllerVersion) + if err != nil { + return nil, errors.Trace(err) + } + checker := schema.List(schema.StringMap(schema.Any())) coerced, err := checker.Coerce(source, nil) if err != nil { return nil, WrapWithDeserializationError(err, "device base schema check failed") } valid := coerced.([]interface{}) + return readDeviceList(valid, readFunc) +} +func getDeviceDeserializationFunc(controllerVersion version.Number) (deviceDeserializationFunc, error) { var deserialisationVersion version.Number for v := range deviceDeserializationFuncs { if v.Compare(deserialisationVersion) > 0 && v.Compare(controllerVersion) <= 0 { @@ -62,8 +85,7 @@ func readDevices(controllerVersion version.Number, source interface{}) ([]*devic if deserialisationVersion == version.Zero { return nil, NewUnsupportedVersionError("no device read func for version %s", controllerVersion) } - readFunc := deviceDeserializationFuncs[deserialisationVersion] - return readDeviceList(valid, readFunc) + return deviceDeserializationFuncs[deserialisationVersion], nil } // readDeviceList expects the values of the sourceList to be string maps. diff --git a/interface.go b/interface.go index 98cba34..722ff15 100644 --- a/interface.go +++ b/interface.go @@ -43,6 +43,7 @@ type Controller interface { // Devices returns a list of devices that match the params. Devices(DevicesArgs) ([]Device, error) + CreateDevice(CreateDeviceArgs) (Device, error) } // Fabric represents a set of interconnected VLANs that are capable of mutual From 9c6a17efa6d008a22c2d3834beb488877b877bb3 Mon Sep 17 00:00:00 2001 From: Tim Penhey Date: Mon, 4 Apr 2016 12:48:45 +1200 Subject: [PATCH 2/6] Tests around create device. --- controller_test.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/controller_test.go b/controller_test.go index 08bc403..da587c4 100644 --- a/controller_test.go +++ b/controller_test.go @@ -107,6 +107,32 @@ func (s *controllerSuite) TestDevicessArgs(c *gc.C) { c.Assert(request.URL.Query(), gc.HasLen, 6) } +func (s *controllerSuite) TestCreateDevice(c *gc.C) { + s.server.AddPostResponse("/api/2.0/devices/?op=create", http.StatusOK, deviceResponse) + controller := s.getController(c) + device, err := controller.CreateDevice(CreateDeviceArgs{}) + c.Assert(err, jc.ErrorIsNil) + c.Assert(device.SystemID(), gc.Equals, "4y3ha8") +} + +func (s *controllerSuite) TestCreateDeviceArgs(c *gc.C) { + s.server.AddPostResponse("/api/2.0/devices/?op=create", http.StatusOK, deviceResponse) + controller := s.getController(c) + // Create an arg structure that sets all the values. + args := CreateDeviceArgs{ + Hostname: "foobar", + MACAddresses: []string{"an-address"}, + Domain: "a domain", + Parent: "parent", + } + _, err := controller.CreateDevice(args) + c.Assert(err, jc.ErrorIsNil) + + request := s.server.LastRequest() + // There should be one entry in the form values for each of the args. + c.Assert(request.PostForm, gc.HasLen, 4) +} + func (s *controllerSuite) TestFabrics(c *gc.C) { controller := s.getController(c) fabrics, err := controller.Fabrics() From d0cde9972ed59b14718858ed2d8277b913df7a12 Mon Sep 17 00:00:00 2001 From: Tim Penhey Date: Mon, 4 Apr 2016 13:05:51 +1200 Subject: [PATCH 3/6] Test for known errors. --- controller.go | 8 ++++++-- controller_test.go | 21 ++++++++++++++++++++- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/controller.go b/controller.go index 9c0f8cf..4257741 100644 --- a/controller.go +++ b/controller.go @@ -204,6 +204,10 @@ type CreateDeviceArgs struct { // Devices implements Controller. func (c *controller) CreateDevice(args CreateDeviceArgs) (Device, error) { + // There must be at least one mac address. + if len(args.MACAddresses) == 0 { + return nil, NewBadRequestError("at least one MAC address must be specified") + } params := NewURLParams() params.MaybeAdd("hostname", args.Hostname) params.MaybeAdd("domain", args.Domain) @@ -212,8 +216,8 @@ func (c *controller) CreateDevice(args CreateDeviceArgs) (Device, error) { result, err := c.post("devices", "create", params.Values) if err != nil { if svrErr, ok := errors.Cause(err).(ServerError); ok { - if svrErr.StatusCode == http.StatusConflict { - return nil, errors.Wrap(err, NewNoMatchError(svrErr.BodyMessage)) + if svrErr.StatusCode == http.StatusBadRequest { + return nil, errors.Wrap(err, NewBadRequestError(svrErr.BodyMessage)) } } // Translate http errors. diff --git a/controller_test.go b/controller_test.go index da587c4..80e087e 100644 --- a/controller_test.go +++ b/controller_test.go @@ -110,11 +110,30 @@ func (s *controllerSuite) TestDevicessArgs(c *gc.C) { func (s *controllerSuite) TestCreateDevice(c *gc.C) { s.server.AddPostResponse("/api/2.0/devices/?op=create", http.StatusOK, deviceResponse) controller := s.getController(c) - device, err := controller.CreateDevice(CreateDeviceArgs{}) + device, err := controller.CreateDevice(CreateDeviceArgs{ + MACAddresses: []string{"a-mac-address"}, + }) c.Assert(err, jc.ErrorIsNil) c.Assert(device.SystemID(), gc.Equals, "4y3ha8") } +func (s *controllerSuite) TestCreateDeviceMissingAddress(c *gc.C) { + controller := s.getController(c) + _, err := controller.CreateDevice(CreateDeviceArgs{}) + c.Assert(err, jc.Satisfies, IsBadRequestError) + c.Assert(err.Error(), gc.Equals, "at least one MAC address must be specified") +} + +func (s *controllerSuite) TestCreateDeviceBadRequest(c *gc.C) { + s.server.AddPostResponse("/api/2.0/devices/?op=create", http.StatusBadRequest, "some error") + controller := s.getController(c) + _, err := controller.CreateDevice(CreateDeviceArgs{ + MACAddresses: []string{"a-mac-address"}, + }) + c.Assert(err, jc.Satisfies, IsBadRequestError) + c.Assert(err.Error(), gc.Equals, "some error") +} + func (s *controllerSuite) TestCreateDeviceArgs(c *gc.C) { s.server.AddPostResponse("/api/2.0/devices/?op=create", http.StatusOK, deviceResponse) controller := s.getController(c) From 224ea803d5aa23732bee847b3b9acf9dda69be90 Mon Sep 17 00:00:00 2001 From: Tim Penhey Date: Mon, 4 Apr 2016 15:33:11 +1200 Subject: [PATCH 4/6] Delete devices. --- controller.go | 16 ++++++++++++++++ controller_test.go | 21 +++++++++++++++++++++ device.go | 21 +++++++++++++++++++++ device_test.go | 46 +++++++++++++++++++++++++++++++++++++++++++++- interface.go | 2 ++ testing.go | 31 +++++++++++++++++++++++-------- 6 files changed, 128 insertions(+), 9 deletions(-) diff --git a/controller.go b/controller.go index 4257741..a6d5506 100644 --- a/controller.go +++ b/controller.go @@ -189,6 +189,7 @@ func (c *controller) Devices(args DevicesArgs) ([]Device, error) { } var result []Device for _, d := range devices { + d.controller = c result = append(result, d) } return result, nil @@ -228,6 +229,7 @@ func (c *controller) CreateDevice(args CreateDeviceArgs) (Device, error) { if err != nil { return nil, errors.Trace(err) } + device.controller = c return device, nil } @@ -385,6 +387,20 @@ func (c *controller) post(path, op string, params url.Values) (interface{}, erro return parsed, nil } +func (c *controller) delete(path string) error { + path = EnsureTrailingSlash(path) + requestID := nextRequestID() + logger.Tracef("request %x: DELTE %s%s", requestID, c.client.APIURL, path) + err := c.client.Delete(&url.URL{Path: path}) + if err != nil { + logger.Tracef("response %x: error: %q", requestID, err.Error()) + logger.Tracef("error detail: %#v", err) + return errors.Trace(err) + } + logger.Tracef("response %x: complete", requestID) + return nil +} + func (c *controller) getQuery(path string, params url.Values) (interface{}, error) { return c._get(path, params) } diff --git a/controller_test.go b/controller_test.go index 80e087e..b8931ac 100644 --- a/controller_test.go +++ b/controller_test.go @@ -313,3 +313,24 @@ func (s *controllerSuite) TestReleaseMachinesUnexpected(c *gc.C) { } var versionResponse = `{"version": "unknown", "subversion": "", "capabilities": ["networks-management", "static-ipaddresses", "ipv6-deployment-ubuntu", "devices-management", "storage-deployment-ubuntu", "network-deployment-ubuntu"]}` + +type cleanup interface { + AddCleanup(testing.CleanupFunc) +} + +// createTestServerController creates a controller backed on to a test server +// that has sufficient knowledge of versions and users to be able to create a +// valid controller. +func createTestServerController(c *gc.C, suite cleanup) (*SimpleTestServer, Controller) { + server := NewSimpleServer() + server.AddGetResponse("/api/2.0/version/", http.StatusOK, versionResponse) + server.Start() + suite.AddCleanup(func(*gc.C) { server.Close() }) + + controller, err := NewController(ControllerArgs{ + BaseURL: server.URL, + APIKey: "fake:as:key", + }) + c.Assert(err, jc.ErrorIsNil) + return server, controller +} diff --git a/device.go b/device.go index b807150..b6b3c42 100644 --- a/device.go +++ b/device.go @@ -4,12 +4,16 @@ package gomaasapi import ( + "net/http" + "github.com/juju/errors" "github.com/juju/schema" "github.com/juju/version" ) type device struct { + controller *controller + resourceURI string systemID string @@ -45,6 +49,23 @@ func (d *device) Zone() Zone { return d.zone } +// Delete implements Device. +func (d *device) Delete() error { + err := d.controller.delete(d.resourceURI) + if err != nil { + if svrErr, ok := errors.Cause(err).(ServerError); ok { + switch svrErr.StatusCode { + case http.StatusNotFound: + return errors.Wrap(err, NewNoMatchError(svrErr.BodyMessage)) + case http.StatusForbidden: + return errors.Wrap(err, NewPermissionError(svrErr.BodyMessage)) + } + } + return NewUnexpectedError(err) + } + return nil +} + func readDevice(controllerVersion version.Number, source interface{}) (*device, error) { readFunc, err := getDeviceDeserializationFunc(controllerVersion) if err != nil { diff --git a/device_test.go b/device_test.go index 80a6c84..b77ceb8 100644 --- a/device_test.go +++ b/device_test.go @@ -4,12 +4,17 @@ package gomaasapi import ( + "net/http" + + "github.com/juju/testing" jc "github.com/juju/testing/checkers" "github.com/juju/version" gc "gopkg.in/check.v1" ) -type deviceSuite struct{} +type deviceSuite struct { + testing.CleanupSuite +} var _ = gc.Suite(&deviceSuite{}) @@ -45,6 +50,45 @@ func (*deviceSuite) TestHighVersion(c *gc.C) { c.Assert(devices, gc.HasLen, 1) } +func (s *deviceSuite) setupDelete(c *gc.C) (*SimpleTestServer, *device) { + server, controller := createTestServerController(c, s) + server.AddGetResponse("/api/2.0/devices/", http.StatusOK, devicesResponse) + + devices, err := controller.Devices(DevicesArgs{}) + c.Assert(err, jc.ErrorIsNil) + c.Assert(devices, gc.HasLen, 1) + return server, devices[0].(*device) +} + +func (s *deviceSuite) TestDelete(c *gc.C) { + server, device := s.setupDelete(c) + // Successful delete is 204 - StatusNoContent + server.AddDeleteResponse(device.resourceURI, http.StatusNoContent, "") + err := device.Delete() + c.Assert(err, jc.ErrorIsNil) +} + +func (s *deviceSuite) TestDelete404(c *gc.C) { + _, device := s.setupDelete(c) + // No path, so 404 + err := device.Delete() + c.Assert(err, jc.Satisfies, IsNoMatchError) +} + +func (s *deviceSuite) TestDeleteForbidden(c *gc.C) { + server, device := s.setupDelete(c) + server.AddDeleteResponse(device.resourceURI, http.StatusForbidden, "") + err := device.Delete() + c.Assert(err, jc.Satisfies, IsPermissionError) +} + +func (s *deviceSuite) TestDeleteUnknown(c *gc.C) { + server, device := s.setupDelete(c) + server.AddDeleteResponse(device.resourceURI, http.StatusConflict, "") + err := device.Delete() + c.Assert(err, jc.Satisfies, IsUnexpectedError) +} + const ( deviceResponse = ` { diff --git a/interface.go b/interface.go index 722ff15..bb9c598 100644 --- a/interface.go +++ b/interface.go @@ -126,6 +126,8 @@ type Device interface { Zone() Zone // Parent, Owner, MAC Addresses if needed + + Delete() error } // Machine represents a physical machine. diff --git a/testing.go b/testing.go index db83f71..74426e2 100644 --- a/testing.go +++ b/testing.go @@ -89,20 +89,24 @@ type simpleResponse struct { type SimpleTestServer struct { *httptest.Server - getResponses map[string][]simpleResponse - getResponseIndex map[string]int - postResponses map[string][]simpleResponse - postResponseIndex map[string]int + getResponses map[string][]simpleResponse + getResponseIndex map[string]int + postResponses map[string][]simpleResponse + postResponseIndex map[string]int + deleteResponses map[string][]simpleResponse + deleteResponseIndex map[string]int requests []*http.Request } func NewSimpleServer() *SimpleTestServer { server := &SimpleTestServer{ - getResponses: make(map[string][]simpleResponse), - getResponseIndex: make(map[string]int), - postResponses: make(map[string][]simpleResponse), - postResponseIndex: make(map[string]int), + getResponses: make(map[string][]simpleResponse), + getResponseIndex: make(map[string]int), + postResponses: make(map[string][]simpleResponse), + postResponseIndex: make(map[string]int), + deleteResponses: make(map[string][]simpleResponse), + deleteResponseIndex: make(map[string]int), } server.Server = httptest.NewUnstartedServer(http.HandlerFunc(server.handler)) return server @@ -116,6 +120,10 @@ func (s *SimpleTestServer) AddPostResponse(path string, status int, body string) s.postResponses[path] = append(s.postResponses[path], simpleResponse{status: status, body: body}) } +func (s *SimpleTestServer) AddDeleteResponse(path string, status int, body string) { + s.deleteResponses[path] = append(s.deleteResponses[path], simpleResponse{status: status, body: body}) +} + func (s *SimpleTestServer) LastRequest() *http.Request { pos := len(s.requests) - 1 if pos < 0 { @@ -143,6 +151,13 @@ func (s *SimpleTestServer) handler(writer http.ResponseWriter, request *http.Req if err != nil { panic(err) } + case "DELETE": + responses = s.deleteResponses + responseIndex = s.deleteResponseIndex + _, err := readAndClose(request.Body) + if err != nil { + panic(err) + } default: panic("unsupported method " + method) } From bbba4bd904b3dd50947da6776c58407a29c4f592 Mon Sep 17 00:00:00 2001 From: Tim Penhey Date: Mon, 4 Apr 2016 15:41:32 +1200 Subject: [PATCH 5/6] Continue refactgor into machines. --- controller_test.go | 1 + machine_test.go | 14 +------------- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/controller_test.go b/controller_test.go index 73d5c24..8fa9c9e 100644 --- a/controller_test.go +++ b/controller_test.go @@ -373,6 +373,7 @@ type cleanup interface { // valid controller. func createTestServerController(c *gc.C, suite cleanup) (*SimpleTestServer, Controller) { server := NewSimpleServer() + server.AddGetResponse("/api/2.0/users/?op=whoami", http.StatusOK, `"captain awesome"`) server.AddGetResponse("/api/2.0/version/", http.StatusOK, versionResponse) server.Start() suite.AddCleanup(func(*gc.C) { server.Close() }) diff --git a/machine_test.go b/machine_test.go index 74afe8d..3327802 100644 --- a/machine_test.go +++ b/machine_test.go @@ -70,25 +70,13 @@ func (*machineSuite) TestHighVersion(c *gc.C) { // Since the start method uses controller pieces, we get the machine from // the controller. func (s *machineSuite) startSetup(c *gc.C) (*SimpleTestServer, *machine) { - server := NewSimpleServer() + server, controller := createTestServerController(c, s) // Just have machines return one machine server.AddGetResponse("/api/2.0/machines/", http.StatusOK, "["+machineResponse+"]") - server.AddGetResponse("/api/2.0/users/?op=whoami", http.StatusOK, `"captain awesome"`) - server.AddGetResponse("/api/2.0/version/", http.StatusOK, versionResponse) - server.Start() - s.AddCleanup(func(*gc.C) { server.Close() }) - - controller, err := NewController(ControllerArgs{ - BaseURL: server.URL, - APIKey: "fake:as:key", - }) - c.Assert(err, jc.ErrorIsNil) - machines, err := controller.Machines(MachinesArgs{}) c.Assert(err, jc.ErrorIsNil) c.Check(machines, gc.HasLen, 1) machine := machines[0].(*machine) - return server, machine } From 347c0cfe7776bb20fce7bc4d22fdd65f4d035e82 Mon Sep 17 00:00:00 2001 From: Tim Penhey Date: Tue, 5 Apr 2016 14:22:11 +1200 Subject: [PATCH 6/6] Fix another typo. --- controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controller.go b/controller.go index ede3744..e185192 100644 --- a/controller.go +++ b/controller.go @@ -407,7 +407,7 @@ func (c *controller) post(path, op string, params url.Values) (interface{}, erro func (c *controller) delete(path string) error { path = EnsureTrailingSlash(path) requestID := nextRequestID() - logger.Tracef("request %x: DELTE %s%s", requestID, c.client.APIURL, path) + logger.Tracef("request %x: DELETE %s%s", requestID, c.client.APIURL, path) err := c.client.Delete(&url.URL{Path: path}) if err != nil { logger.Tracef("response %x: error: %q", requestID, err.Error())