From 0911dfaa0abe2c69cd81f4fb39e7744cf670a4ca Mon Sep 17 00:00:00 2001 From: Tim Penhey Date: Thu, 7 Apr 2016 14:01:49 +1200 Subject: [PATCH 1/4] Create physical interface. --- interfaces.go | 4 ++ machine.go | 78 +++++++++++++++++++++++++++++ machine_test.go | 129 ++++++++++++++++++++++++++++++++++++++++++++---- testing.go | 3 ++ 4 files changed, 204 insertions(+), 10 deletions(-) diff --git a/interfaces.go b/interfaces.go index ef32ac1..d523f70 100644 --- a/interfaces.go +++ b/interfaces.go @@ -194,7 +194,11 @@ type Machine interface { Zone() Zone + // Start the machine and install the operating system specified in the args. Start(StartArgs) error + + // CreatePhysicalInterface will create a physical interface for this machine. + CreatePhysicalInterface(CreatePhysicalInterfaceArgs) (Interface, error) } // Space is a name for a collection of Subnets. diff --git a/machine.go b/machine.go index 383b71e..6c3c001 100644 --- a/machine.go +++ b/machine.go @@ -5,7 +5,9 @@ package gomaasapi import ( "encoding/base64" + "fmt" "net/http" + "strings" "github.com/juju/errors" "github.com/juju/schema" @@ -184,6 +186,82 @@ func (m *machine) Start(args StartArgs) error { return nil } +// CreatePhysicalInterfaceArgs is an argument struct for passing parameters to +// the Machine.CreatePhysicalInterface method. +type CreatePhysicalInterfaceArgs struct { + // Name of the interface (required). + Name string + // MACAddress is the MAC address of the interface (required). + MACAddress string + // VLAN is the untagged VLAN the interface is connected to (required). + VLAN VLAN + // Tags to attach to the interface (optional). + Tags []string + // MTU - Maximum transmission unit. (optional) + MTU int + // AcceptRA - Accept router advertisements. (IPv6 only) + AcceptRA bool + // Autoconf - Perform stateless autoconfiguration. (IPv6 only) + Autoconf bool +} + +// Validate checks the required fields are set for the arg structure. +func (a *CreatePhysicalInterfaceArgs) Validate() error { + if a.Name == "" { + return errors.NotValidf("missing Name") + } + if a.MACAddress == "" { + return errors.NotValidf("missing MACAddress") + } + if a.VLAN == nil { + return errors.NotValidf("missing VLAN") + } + return nil +} + +// CreatePhysicalInterface implements Machine. +func (m *machine) CreatePhysicalInterface(args CreatePhysicalInterfaceArgs) (Interface, error) { + if err := args.Validate(); err != nil { + return nil, errors.Trace(err) + } + params := NewURLParams() + params.Values.Add("name", args.Name) + params.Values.Add("mac_address", args.MACAddress) + params.Values.Add("vlan", fmt.Sprint(args.VLAN.ID())) + params.MaybeAdd("tags", strings.Join(args.Tags, ",")) + params.MaybeAddInt("mtu", args.MTU) + params.MaybeAddBool("accept_ra", args.AcceptRA) + params.MaybeAddBool("autoconf", args.Autoconf) + result, err := m.controller.post(m.interfacesURI(), "create_physical", params.Values) + if err != nil { + if svrErr, ok := errors.Cause(err).(ServerError); ok { + switch svrErr.StatusCode { + case http.StatusNotFound, http.StatusConflict: + return nil, errors.Wrap(err, NewBadRequestError(svrErr.BodyMessage)) + case http.StatusForbidden: + return nil, errors.Wrap(err, NewPermissionError(svrErr.BodyMessage)) + case http.StatusServiceUnavailable: + return nil, errors.Wrap(err, NewCannotCompleteError(svrErr.BodyMessage)) + } + } + return nil, NewUnexpectedError(err) + } + + iface, err := readInterface(m.controller.apiVersion, result) + if err != nil { + return nil, errors.Trace(err) + } + + m.interfaceSet = append(m.interfaceSet, iface) + return iface, nil +} + +// interfacesURI used to add interfaces for this machine. The operations +// are on the nodes endpoint, not machines. +func (m *machine) interfacesURI() string { + return strings.Replace(m.resourceURI, "machines", "nodes", 1) + "interfaces/" +} + func readMachine(controllerVersion version.Number, source interface{}) (*machine, error) { readFunc, err := getMachineDeserializationFunc(controllerVersion) if err != nil { diff --git a/machine_test.go b/machine_test.go index c7cb0f8..76ce4f6 100644 --- a/machine_test.go +++ b/machine_test.go @@ -7,6 +7,7 @@ import ( "encoding/base64" "net/http" + "github.com/juju/errors" "github.com/juju/testing" jc "github.com/juju/testing/checkers" "github.com/juju/version" @@ -14,7 +15,7 @@ import ( ) type machineSuite struct { - testing.CleanupSuite + testing.LoggingCleanupSuite } var _ = gc.Suite(&machineSuite{}) @@ -76,9 +77,7 @@ func (*machineSuite) TestHighVersion(c *gc.C) { c.Assert(machines, gc.HasLen, 3) } -// Since the start method uses controller pieces, we get the machine from -// the controller. -func (s *machineSuite) startSetup(c *gc.C) (*SimpleTestServer, *machine) { +func (s *machineSuite) getServerAndMachine(c *gc.C) (*SimpleTestServer, *machine) { server, controller := createTestServerController(c, s) // Just have machines return one machine server.AddGetResponse("/api/2.0/machines/", http.StatusOK, "["+machineResponse+"]") @@ -90,7 +89,7 @@ func (s *machineSuite) startSetup(c *gc.C) (*SimpleTestServer, *machine) { } func (s *machineSuite) TestStart(c *gc.C) { - server, machine := s.startSetup(c) + server, machine := s.getServerAndMachine(c) response := updateJSONMap(c, machineResponse, map[string]interface{}{ "status_name": "Deploying", "status_message": "for testing", @@ -118,7 +117,7 @@ func (s *machineSuite) TestStart(c *gc.C) { } func (s *machineSuite) TestStartMachineNotFound(c *gc.C) { - server, machine := s.startSetup(c) + server, machine := s.getServerAndMachine(c) server.AddPostResponse(machine.resourceURI+"?op=deploy", http.StatusNotFound, "can't find machine") err := machine.Start(StartArgs{}) c.Assert(err, jc.Satisfies, IsBadRequestError) @@ -126,7 +125,7 @@ func (s *machineSuite) TestStartMachineNotFound(c *gc.C) { } func (s *machineSuite) TestStartMachineConflict(c *gc.C) { - server, machine := s.startSetup(c) + server, machine := s.getServerAndMachine(c) server.AddPostResponse(machine.resourceURI+"?op=deploy", http.StatusConflict, "machine not allocated") err := machine.Start(StartArgs{}) c.Assert(err, jc.Satisfies, IsBadRequestError) @@ -134,7 +133,7 @@ func (s *machineSuite) TestStartMachineConflict(c *gc.C) { } func (s *machineSuite) TestStartMachineForbidden(c *gc.C) { - server, machine := s.startSetup(c) + server, machine := s.getServerAndMachine(c) server.AddPostResponse(machine.resourceURI+"?op=deploy", http.StatusForbidden, "machine not yours") err := machine.Start(StartArgs{}) c.Assert(err, jc.Satisfies, IsPermissionError) @@ -142,7 +141,7 @@ func (s *machineSuite) TestStartMachineForbidden(c *gc.C) { } func (s *machineSuite) TestStartMachineServiceUnavailable(c *gc.C) { - server, machine := s.startSetup(c) + server, machine := s.getServerAndMachine(c) server.AddPostResponse(machine.resourceURI+"?op=deploy", http.StatusServiceUnavailable, "no ip addresses available") err := machine.Start(StartArgs{}) c.Assert(err, jc.Satisfies, IsCannotCompleteError) @@ -150,13 +149,123 @@ func (s *machineSuite) TestStartMachineServiceUnavailable(c *gc.C) { } func (s *machineSuite) TestStartMachineUnknown(c *gc.C) { - server, machine := s.startSetup(c) + server, machine := s.getServerAndMachine(c) server.AddPostResponse(machine.resourceURI+"?op=deploy", http.StatusMethodNotAllowed, "wat?") err := machine.Start(StartArgs{}) c.Assert(err, jc.Satisfies, IsUnexpectedError) c.Assert(err.Error(), gc.Equals, "unexpected: ServerError: 405 Method Not Allowed (wat?)") } +type fakeVLAN struct { + VLAN + id int +} + +func (f *fakeVLAN) ID() int { + return f.id +} + +func (s *controllerSuite) TestCreatePhysicalInterfaceArgsValidate(c *gc.C) { + for i, test := range []struct { + args CreatePhysicalInterfaceArgs + errText string + }{{ + errText: "missing Name not valid", + }, { + args: CreatePhysicalInterfaceArgs{Name: "eth3"}, + errText: "missing MACAddress not valid", + }, { + args: CreatePhysicalInterfaceArgs{Name: "eth3", MACAddress: "a-mac-address"}, + errText: `missing VLAN not valid`, + }, { + args: CreatePhysicalInterfaceArgs{Name: "eth3", MACAddress: "a-mac-address", VLAN: &fakeVLAN{}}, + }} { + c.Logf("test %d", i) + err := test.args.Validate() + if test.errText == "" { + c.Check(err, jc.ErrorIsNil) + } else { + c.Check(err, jc.Satisfies, errors.IsNotValid) + c.Check(err.Error(), gc.Equals, test.errText) + } + } +} + +func (s *machineSuite) TestCreatePhysicalInterfaceValidates(c *gc.C) { + _, machine := s.getServerAndMachine(c) + _, err := machine.CreatePhysicalInterface(CreatePhysicalInterfaceArgs{}) + c.Assert(err, jc.Satisfies, errors.IsNotValid) +} + +func (s *machineSuite) TestCreatePhysicalInterface(c *gc.C) { + server, machine := s.getServerAndMachine(c) + server.AddPostResponse(machine.interfacesURI()+"?op=create_physical", http.StatusOK, interfaceResponse) + + iface, err := machine.CreatePhysicalInterface(CreatePhysicalInterfaceArgs{ + Name: "eth43", + MACAddress: "some-mac-address", + VLAN: &fakeVLAN{id: 33}, + Tags: []string{"foo", "bar"}, + }) + c.Assert(err, jc.ErrorIsNil) + c.Assert(iface, gc.NotNil) + + request := server.LastRequest() + form := request.PostForm + c.Assert(form.Get("name"), gc.Equals, "eth43") + c.Assert(form.Get("mac_address"), gc.Equals, "some-mac-address") + c.Assert(form.Get("vlan"), gc.Equals, "33") + c.Assert(form.Get("tags"), gc.Equals, "foo,bar") +} + +func (s *machineSuite) minimalCreatePhysicalInterfaceArgs() CreatePhysicalInterfaceArgs { + return CreatePhysicalInterfaceArgs{ + Name: "eth43", + MACAddress: "some-mac-address", + VLAN: &fakeVLAN{id: 33}, + } +} + +func (s *machineSuite) TestCreatePhysicalInterfaceNotFound(c *gc.C) { + server, machine := s.getServerAndMachine(c) + server.AddPostResponse(machine.interfacesURI()+"?op=create_physical", http.StatusNotFound, "can't find machine") + _, err := machine.CreatePhysicalInterface(s.minimalCreatePhysicalInterfaceArgs()) + c.Assert(err, jc.Satisfies, IsBadRequestError) + c.Assert(err.Error(), gc.Equals, "can't find machine") +} + +func (s *machineSuite) TestCreatePhysicalInterfaceConflict(c *gc.C) { + server, machine := s.getServerAndMachine(c) + server.AddPostResponse(machine.interfacesURI()+"?op=create_physical", http.StatusConflict, "machine not allocated") + _, err := machine.CreatePhysicalInterface(s.minimalCreatePhysicalInterfaceArgs()) + c.Assert(err, jc.Satisfies, IsBadRequestError) + c.Assert(err.Error(), gc.Equals, "machine not allocated") +} + +func (s *machineSuite) TestCreatePhysicalInterfaceForbidden(c *gc.C) { + server, machine := s.getServerAndMachine(c) + server.AddPostResponse(machine.interfacesURI()+"?op=create_physical", http.StatusForbidden, "machine not yours") + _, err := machine.CreatePhysicalInterface(s.minimalCreatePhysicalInterfaceArgs()) + c.Assert(err, jc.Satisfies, IsPermissionError) + c.Assert(err.Error(), gc.Equals, "machine not yours") +} + +func (s *machineSuite) TestCreatePhysicalInterfaceServiceUnavailable(c *gc.C) { + server, machine := s.getServerAndMachine(c) + server.AddPostResponse(machine.interfacesURI()+"?op=create_physical", http.StatusServiceUnavailable, "no ip addresses available") + _, err := machine.CreatePhysicalInterface(s.minimalCreatePhysicalInterfaceArgs()) + c.Assert(err, jc.Satisfies, IsCannotCompleteError) + c.Assert(err.Error(), gc.Equals, "no ip addresses available") +} + +func (s *machineSuite) TestCreatePhysicalInterfaceUnknown(c *gc.C) { + server, machine := s.getServerAndMachine(c) + server.AddPostResponse(machine.interfacesURI()+"?op=create_physical", http.StatusMethodNotAllowed, "wat?") + _, err := machine.CreatePhysicalInterface(s.minimalCreatePhysicalInterfaceArgs()) + c.Assert(err, jc.Satisfies, IsUnexpectedError) + c.Assert(err.Error(), gc.Equals, "unexpected: ServerError: 405 Method Not Allowed (wat?)") +} + const ( machineResponse = ` { diff --git a/testing.go b/testing.go index 1954be7..c697626 100644 --- a/testing.go +++ b/testing.go @@ -114,14 +114,17 @@ func NewSimpleServer() *SimpleTestServer { } func (s *SimpleTestServer) AddGetResponse(path string, status int, body string) { + logger.Debugf("add get response for: %s, %d", path, status) s.getResponses[path] = append(s.getResponses[path], simpleResponse{status: status, body: body}) } func (s *SimpleTestServer) AddPostResponse(path string, status int, body string) { + logger.Debugf("add post response for: %s, %d", path, status) s.postResponses[path] = append(s.postResponses[path], simpleResponse{status: status, body: body}) } func (s *SimpleTestServer) AddDeleteResponse(path string, status int, body string) { + logger.Debugf("add delete response for: %s, %d", path, status) s.deleteResponses[path] = append(s.deleteResponses[path], simpleResponse{status: status, body: body}) } From 8976bedfc669b97fa35fa814687c843790bf0bef Mon Sep 17 00:00:00 2001 From: Tim Penhey Date: Thu, 7 Apr 2016 15:08:21 +1200 Subject: [PATCH 2/4] CreatePhysicalInterface should be on Device not Machine. --- device.go | 78 ++++++++++++++++++++++++++++++ device_test.go | 121 ++++++++++++++++++++++++++++++++++++++++++++-- interface.go | 8 --- interface_test.go | 1 - interfaces.go | 10 ++-- link.go | 14 ++++-- machine.go | 78 ------------------------------ machine_test.go | 111 ------------------------------------------ 8 files changed, 209 insertions(+), 212 deletions(-) diff --git a/device.go b/device.go index b6b3c42..4462ac6 100644 --- a/device.go +++ b/device.go @@ -4,7 +4,9 @@ package gomaasapi import ( + "fmt" "net/http" + "strings" "github.com/juju/errors" "github.com/juju/schema" @@ -49,6 +51,82 @@ func (d *device) Zone() Zone { return d.zone } +// CreatePhysicalInterfaceArgs is an argument struct for passing parameters to +// the Machine.CreatePhysicalInterface method. +type CreatePhysicalInterfaceArgs struct { + // Name of the interface (required). + Name string + // MACAddress is the MAC address of the interface (required). + MACAddress string + // VLAN is the untagged VLAN the interface is connected to (required). + VLAN VLAN + // Tags to attach to the interface (optional). + Tags []string + // MTU - Maximum transmission unit. (optional) + MTU int + // AcceptRA - Accept router advertisements. (IPv6 only) + AcceptRA bool + // Autoconf - Perform stateless autoconfiguration. (IPv6 only) + Autoconf bool +} + +// Validate checks the required fields are set for the arg structure. +func (a *CreatePhysicalInterfaceArgs) Validate() error { + if a.Name == "" { + return errors.NotValidf("missing Name") + } + if a.MACAddress == "" { + return errors.NotValidf("missing MACAddress") + } + if a.VLAN == nil { + return errors.NotValidf("missing VLAN") + } + return nil +} + +// interfacesURI used to add interfaces for this device. The operations +// are on the nodes endpoint, not devices. +func (d *device) interfacesURI() string { + return strings.Replace(d.resourceURI, "devices", "nodes", 1) + "interfaces/" +} + +// CreatePhysicalInterface implements Device. +func (d *device) CreatePhysicalInterface(args CreatePhysicalInterfaceArgs) (Interface, error) { + if err := args.Validate(); err != nil { + return nil, errors.Trace(err) + } + params := NewURLParams() + params.Values.Add("name", args.Name) + params.Values.Add("mac_address", args.MACAddress) + params.Values.Add("vlan", fmt.Sprint(args.VLAN.ID())) + params.MaybeAdd("tags", strings.Join(args.Tags, ",")) + params.MaybeAddInt("mtu", args.MTU) + params.MaybeAddBool("accept_ra", args.AcceptRA) + params.MaybeAddBool("autoconf", args.Autoconf) + result, err := d.controller.post(d.interfacesURI(), "create_physical", params.Values) + if err != nil { + if svrErr, ok := errors.Cause(err).(ServerError); ok { + switch svrErr.StatusCode { + case http.StatusNotFound, http.StatusConflict: + return nil, errors.Wrap(err, NewBadRequestError(svrErr.BodyMessage)) + case http.StatusForbidden: + return nil, errors.Wrap(err, NewPermissionError(svrErr.BodyMessage)) + case http.StatusServiceUnavailable: + return nil, errors.Wrap(err, NewCannotCompleteError(svrErr.BodyMessage)) + } + } + return nil, NewUnexpectedError(err) + } + + iface, err := readInterface(d.controller.apiVersion, result) + if err != nil { + return nil, errors.Trace(err) + } + + // m.interfaceSet = append(m.interfaceSet, iface) + return iface, nil +} + // Delete implements Device. func (d *device) Delete() error { err := d.controller.delete(d.resourceURI) diff --git a/device_test.go b/device_test.go index b77ceb8..3b18330 100644 --- a/device_test.go +++ b/device_test.go @@ -6,6 +6,7 @@ package gomaasapi import ( "net/http" + "github.com/juju/errors" "github.com/juju/testing" jc "github.com/juju/testing/checkers" "github.com/juju/version" @@ -50,7 +51,117 @@ func (*deviceSuite) TestHighVersion(c *gc.C) { c.Assert(devices, gc.HasLen, 1) } -func (s *deviceSuite) setupDelete(c *gc.C) (*SimpleTestServer, *device) { +type fakeVLAN struct { + VLAN + id int +} + +func (f *fakeVLAN) ID() int { + return f.id +} + +func (s *controllerSuite) TestCreatePhysicalInterfaceArgsValidate(c *gc.C) { + for i, test := range []struct { + args CreatePhysicalInterfaceArgs + errText string + }{{ + errText: "missing Name not valid", + }, { + args: CreatePhysicalInterfaceArgs{Name: "eth3"}, + errText: "missing MACAddress not valid", + }, { + args: CreatePhysicalInterfaceArgs{Name: "eth3", MACAddress: "a-mac-address"}, + errText: `missing VLAN not valid`, + }, { + args: CreatePhysicalInterfaceArgs{Name: "eth3", MACAddress: "a-mac-address", VLAN: &fakeVLAN{}}, + }} { + c.Logf("test %d", i) + err := test.args.Validate() + if test.errText == "" { + c.Check(err, jc.ErrorIsNil) + } else { + c.Check(err, jc.Satisfies, errors.IsNotValid) + c.Check(err.Error(), gc.Equals, test.errText) + } + } +} + +func (s *deviceSuite) TestCreatePhysicalInterfaceValidates(c *gc.C) { + _, device := s.getServerAndDevice(c) + _, err := device.CreatePhysicalInterface(CreatePhysicalInterfaceArgs{}) + c.Assert(err, jc.Satisfies, errors.IsNotValid) +} + +func (s *deviceSuite) TestCreatePhysicalInterface(c *gc.C) { + server, device := s.getServerAndDevice(c) + server.AddPostResponse(device.interfacesURI()+"?op=create_physical", http.StatusOK, interfaceResponse) + + iface, err := device.CreatePhysicalInterface(CreatePhysicalInterfaceArgs{ + Name: "eth43", + MACAddress: "some-mac-address", + VLAN: &fakeVLAN{id: 33}, + Tags: []string{"foo", "bar"}, + }) + c.Assert(err, jc.ErrorIsNil) + c.Assert(iface, gc.NotNil) + + request := server.LastRequest() + form := request.PostForm + c.Assert(form.Get("name"), gc.Equals, "eth43") + c.Assert(form.Get("mac_address"), gc.Equals, "some-mac-address") + c.Assert(form.Get("vlan"), gc.Equals, "33") + c.Assert(form.Get("tags"), gc.Equals, "foo,bar") +} + +func (s *deviceSuite) minimalCreatePhysicalInterfaceArgs() CreatePhysicalInterfaceArgs { + return CreatePhysicalInterfaceArgs{ + Name: "eth43", + MACAddress: "some-mac-address", + VLAN: &fakeVLAN{id: 33}, + } +} + +func (s *deviceSuite) TestCreatePhysicalInterfaceNotFound(c *gc.C) { + server, device := s.getServerAndDevice(c) + server.AddPostResponse(device.interfacesURI()+"?op=create_physical", http.StatusNotFound, "can't find device") + _, err := device.CreatePhysicalInterface(s.minimalCreatePhysicalInterfaceArgs()) + c.Assert(err, jc.Satisfies, IsBadRequestError) + c.Assert(err.Error(), gc.Equals, "can't find device") +} + +func (s *deviceSuite) TestCreatePhysicalInterfaceConflict(c *gc.C) { + server, device := s.getServerAndDevice(c) + server.AddPostResponse(device.interfacesURI()+"?op=create_physical", http.StatusConflict, "device not allocated") + _, err := device.CreatePhysicalInterface(s.minimalCreatePhysicalInterfaceArgs()) + c.Assert(err, jc.Satisfies, IsBadRequestError) + c.Assert(err.Error(), gc.Equals, "device not allocated") +} + +func (s *deviceSuite) TestCreatePhysicalInterfaceForbidden(c *gc.C) { + server, device := s.getServerAndDevice(c) + server.AddPostResponse(device.interfacesURI()+"?op=create_physical", http.StatusForbidden, "device not yours") + _, err := device.CreatePhysicalInterface(s.minimalCreatePhysicalInterfaceArgs()) + c.Assert(err, jc.Satisfies, IsPermissionError) + c.Assert(err.Error(), gc.Equals, "device not yours") +} + +func (s *deviceSuite) TestCreatePhysicalInterfaceServiceUnavailable(c *gc.C) { + server, device := s.getServerAndDevice(c) + server.AddPostResponse(device.interfacesURI()+"?op=create_physical", http.StatusServiceUnavailable, "no ip addresses available") + _, err := device.CreatePhysicalInterface(s.minimalCreatePhysicalInterfaceArgs()) + c.Assert(err, jc.Satisfies, IsCannotCompleteError) + c.Assert(err.Error(), gc.Equals, "no ip addresses available") +} + +func (s *deviceSuite) TestCreatePhysicalInterfaceUnknown(c *gc.C) { + server, device := s.getServerAndDevice(c) + server.AddPostResponse(device.interfacesURI()+"?op=create_physical", http.StatusMethodNotAllowed, "wat?") + _, err := device.CreatePhysicalInterface(s.minimalCreatePhysicalInterfaceArgs()) + c.Assert(err, jc.Satisfies, IsUnexpectedError) + c.Assert(err.Error(), gc.Equals, "unexpected: ServerError: 405 Method Not Allowed (wat?)") +} + +func (s *deviceSuite) getServerAndDevice(c *gc.C) (*SimpleTestServer, *device) { server, controller := createTestServerController(c, s) server.AddGetResponse("/api/2.0/devices/", http.StatusOK, devicesResponse) @@ -61,7 +172,7 @@ func (s *deviceSuite) setupDelete(c *gc.C) (*SimpleTestServer, *device) { } func (s *deviceSuite) TestDelete(c *gc.C) { - server, device := s.setupDelete(c) + server, device := s.getServerAndDevice(c) // Successful delete is 204 - StatusNoContent server.AddDeleteResponse(device.resourceURI, http.StatusNoContent, "") err := device.Delete() @@ -69,21 +180,21 @@ func (s *deviceSuite) TestDelete(c *gc.C) { } func (s *deviceSuite) TestDelete404(c *gc.C) { - _, device := s.setupDelete(c) + _, device := s.getServerAndDevice(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, device := s.getServerAndDevice(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, device := s.getServerAndDevice(c) server.AddDeleteResponse(device.resourceURI, http.StatusConflict, "") err := device.Delete() c.Assert(err, jc.Satisfies, IsUnexpectedError) diff --git a/interface.go b/interface.go index e61b4a8..551ae3d 100644 --- a/interface.go +++ b/interface.go @@ -24,7 +24,6 @@ type interface_ struct { macAddress string effectiveMTU int - params string parents []string children []string @@ -89,11 +88,6 @@ func (i *interface_) EffectiveMTU() int { return i.effectiveMTU } -// Params implements Interface. -func (i *interface_) Params() string { - return i.params -} - func readInterface(controllerVersion version.Number, source interface{}) (*interface_, error) { readFunc, err := getInterfaceDeserializationFunc(controllerVersion) if err != nil { @@ -174,7 +168,6 @@ func interface_2_0(source map[string]interface{}) (*interface_, error) { "mac_address": schema.String(), "effective_mtu": schema.ForceInt(), - "params": schema.String(), "parents": schema.List(schema.String()), "children": schema.List(schema.String()), @@ -211,7 +204,6 @@ func interface_2_0(source map[string]interface{}) (*interface_, error) { macAddress: valid["mac_address"].(string), effectiveMTU: valid["effective_mtu"].(int), - params: valid["params"].(string), parents: convertToStringSlice(valid["parents"]), children: convertToStringSlice(valid["children"]), diff --git a/interface_test.go b/interface_test.go index 8a05a8f..6e50477 100644 --- a/interface_test.go +++ b/interface_test.go @@ -39,7 +39,6 @@ func (s *interfaceSuite) checkInterface(c *gc.C, iface *interface_) { c.Check(iface.MACAddress(), gc.Equals, "52:54:00:c9:6a:45") c.Check(iface.EffectiveMTU(), gc.Equals, 1500) - c.Check(iface.Params(), gc.Equals, "some params") c.Check(iface.Parents(), jc.DeepEquals, []string{"bond0"}) c.Check(iface.Children(), jc.DeepEquals, []string{"eth0.1", "eth0.2"}) diff --git a/interfaces.go b/interfaces.go index d523f70..2b88d3a 100644 --- a/interfaces.go +++ b/interfaces.go @@ -160,6 +160,9 @@ type Device interface { // Parent, Owner, MAC Addresses if needed + // CreatePhysicalInterface will create a physical interface for this machine. + CreatePhysicalInterface(CreatePhysicalInterfaceArgs) (Interface, error) + // Delete will remove this Device. Delete() error } @@ -196,9 +199,6 @@ type Machine interface { // Start the machine and install the operating system specified in the args. Start(StartArgs) error - - // CreatePhysicalInterface will create a physical interface for this machine. - CreatePhysicalInterface(CreatePhysicalInterfaceArgs) (Interface, error) } // Space is a name for a collection of Subnets. @@ -244,9 +244,9 @@ type Interface interface { MACAddress() string EffectiveMTU() int - Params() string - // Need to work out types for children, discovered, parents + // Params is a JSON field, and defaults to an empty string, but is almost + // always a JSON object in practice. Gleefully ignoring it until we need it. } // Link represents a network link between an Interface and a Subnet. diff --git a/link.go b/link.go index d4441a0..22fb1c5 100644 --- a/link.go +++ b/link.go @@ -83,7 +83,10 @@ func link_2_0(source map[string]interface{}) (*link, error) { "mode": schema.String(), "subnet": schema.StringMap(schema.Any()), } - checker := schema.FieldMap(fields, nil) // no defaults + defaults := schema.Defaults{ + "subnet": schema.Omit, + } + checker := schema.FieldMap(fields, defaults) coerced, err := checker.Coerce(source, nil) if err != nil { return nil, WrapWithDeserializationError(err, "link 2.0 schema check failed") @@ -92,9 +95,12 @@ func link_2_0(source map[string]interface{}) (*link, error) { // From here we know that the map returned from the schema coercion // contains fields of the right type. - subnet, err := subnet_2_0(valid["subnet"].(map[string]interface{})) - if err != nil { - return nil, errors.Trace(err) + var subnet *subnet + if value, ok := valid["subnet"]; ok { + subnet, err = subnet_2_0(value.(map[string]interface{})) + if err != nil { + return nil, errors.Trace(err) + } } result := &link{ diff --git a/machine.go b/machine.go index 6c3c001..383b71e 100644 --- a/machine.go +++ b/machine.go @@ -5,9 +5,7 @@ package gomaasapi import ( "encoding/base64" - "fmt" "net/http" - "strings" "github.com/juju/errors" "github.com/juju/schema" @@ -186,82 +184,6 @@ func (m *machine) Start(args StartArgs) error { return nil } -// CreatePhysicalInterfaceArgs is an argument struct for passing parameters to -// the Machine.CreatePhysicalInterface method. -type CreatePhysicalInterfaceArgs struct { - // Name of the interface (required). - Name string - // MACAddress is the MAC address of the interface (required). - MACAddress string - // VLAN is the untagged VLAN the interface is connected to (required). - VLAN VLAN - // Tags to attach to the interface (optional). - Tags []string - // MTU - Maximum transmission unit. (optional) - MTU int - // AcceptRA - Accept router advertisements. (IPv6 only) - AcceptRA bool - // Autoconf - Perform stateless autoconfiguration. (IPv6 only) - Autoconf bool -} - -// Validate checks the required fields are set for the arg structure. -func (a *CreatePhysicalInterfaceArgs) Validate() error { - if a.Name == "" { - return errors.NotValidf("missing Name") - } - if a.MACAddress == "" { - return errors.NotValidf("missing MACAddress") - } - if a.VLAN == nil { - return errors.NotValidf("missing VLAN") - } - return nil -} - -// CreatePhysicalInterface implements Machine. -func (m *machine) CreatePhysicalInterface(args CreatePhysicalInterfaceArgs) (Interface, error) { - if err := args.Validate(); err != nil { - return nil, errors.Trace(err) - } - params := NewURLParams() - params.Values.Add("name", args.Name) - params.Values.Add("mac_address", args.MACAddress) - params.Values.Add("vlan", fmt.Sprint(args.VLAN.ID())) - params.MaybeAdd("tags", strings.Join(args.Tags, ",")) - params.MaybeAddInt("mtu", args.MTU) - params.MaybeAddBool("accept_ra", args.AcceptRA) - params.MaybeAddBool("autoconf", args.Autoconf) - result, err := m.controller.post(m.interfacesURI(), "create_physical", params.Values) - if err != nil { - if svrErr, ok := errors.Cause(err).(ServerError); ok { - switch svrErr.StatusCode { - case http.StatusNotFound, http.StatusConflict: - return nil, errors.Wrap(err, NewBadRequestError(svrErr.BodyMessage)) - case http.StatusForbidden: - return nil, errors.Wrap(err, NewPermissionError(svrErr.BodyMessage)) - case http.StatusServiceUnavailable: - return nil, errors.Wrap(err, NewCannotCompleteError(svrErr.BodyMessage)) - } - } - return nil, NewUnexpectedError(err) - } - - iface, err := readInterface(m.controller.apiVersion, result) - if err != nil { - return nil, errors.Trace(err) - } - - m.interfaceSet = append(m.interfaceSet, iface) - return iface, nil -} - -// interfacesURI used to add interfaces for this machine. The operations -// are on the nodes endpoint, not machines. -func (m *machine) interfacesURI() string { - return strings.Replace(m.resourceURI, "machines", "nodes", 1) + "interfaces/" -} - func readMachine(controllerVersion version.Number, source interface{}) (*machine, error) { readFunc, err := getMachineDeserializationFunc(controllerVersion) if err != nil { diff --git a/machine_test.go b/machine_test.go index 76ce4f6..392d990 100644 --- a/machine_test.go +++ b/machine_test.go @@ -7,7 +7,6 @@ import ( "encoding/base64" "net/http" - "github.com/juju/errors" "github.com/juju/testing" jc "github.com/juju/testing/checkers" "github.com/juju/version" @@ -156,116 +155,6 @@ func (s *machineSuite) TestStartMachineUnknown(c *gc.C) { c.Assert(err.Error(), gc.Equals, "unexpected: ServerError: 405 Method Not Allowed (wat?)") } -type fakeVLAN struct { - VLAN - id int -} - -func (f *fakeVLAN) ID() int { - return f.id -} - -func (s *controllerSuite) TestCreatePhysicalInterfaceArgsValidate(c *gc.C) { - for i, test := range []struct { - args CreatePhysicalInterfaceArgs - errText string - }{{ - errText: "missing Name not valid", - }, { - args: CreatePhysicalInterfaceArgs{Name: "eth3"}, - errText: "missing MACAddress not valid", - }, { - args: CreatePhysicalInterfaceArgs{Name: "eth3", MACAddress: "a-mac-address"}, - errText: `missing VLAN not valid`, - }, { - args: CreatePhysicalInterfaceArgs{Name: "eth3", MACAddress: "a-mac-address", VLAN: &fakeVLAN{}}, - }} { - c.Logf("test %d", i) - err := test.args.Validate() - if test.errText == "" { - c.Check(err, jc.ErrorIsNil) - } else { - c.Check(err, jc.Satisfies, errors.IsNotValid) - c.Check(err.Error(), gc.Equals, test.errText) - } - } -} - -func (s *machineSuite) TestCreatePhysicalInterfaceValidates(c *gc.C) { - _, machine := s.getServerAndMachine(c) - _, err := machine.CreatePhysicalInterface(CreatePhysicalInterfaceArgs{}) - c.Assert(err, jc.Satisfies, errors.IsNotValid) -} - -func (s *machineSuite) TestCreatePhysicalInterface(c *gc.C) { - server, machine := s.getServerAndMachine(c) - server.AddPostResponse(machine.interfacesURI()+"?op=create_physical", http.StatusOK, interfaceResponse) - - iface, err := machine.CreatePhysicalInterface(CreatePhysicalInterfaceArgs{ - Name: "eth43", - MACAddress: "some-mac-address", - VLAN: &fakeVLAN{id: 33}, - Tags: []string{"foo", "bar"}, - }) - c.Assert(err, jc.ErrorIsNil) - c.Assert(iface, gc.NotNil) - - request := server.LastRequest() - form := request.PostForm - c.Assert(form.Get("name"), gc.Equals, "eth43") - c.Assert(form.Get("mac_address"), gc.Equals, "some-mac-address") - c.Assert(form.Get("vlan"), gc.Equals, "33") - c.Assert(form.Get("tags"), gc.Equals, "foo,bar") -} - -func (s *machineSuite) minimalCreatePhysicalInterfaceArgs() CreatePhysicalInterfaceArgs { - return CreatePhysicalInterfaceArgs{ - Name: "eth43", - MACAddress: "some-mac-address", - VLAN: &fakeVLAN{id: 33}, - } -} - -func (s *machineSuite) TestCreatePhysicalInterfaceNotFound(c *gc.C) { - server, machine := s.getServerAndMachine(c) - server.AddPostResponse(machine.interfacesURI()+"?op=create_physical", http.StatusNotFound, "can't find machine") - _, err := machine.CreatePhysicalInterface(s.minimalCreatePhysicalInterfaceArgs()) - c.Assert(err, jc.Satisfies, IsBadRequestError) - c.Assert(err.Error(), gc.Equals, "can't find machine") -} - -func (s *machineSuite) TestCreatePhysicalInterfaceConflict(c *gc.C) { - server, machine := s.getServerAndMachine(c) - server.AddPostResponse(machine.interfacesURI()+"?op=create_physical", http.StatusConflict, "machine not allocated") - _, err := machine.CreatePhysicalInterface(s.minimalCreatePhysicalInterfaceArgs()) - c.Assert(err, jc.Satisfies, IsBadRequestError) - c.Assert(err.Error(), gc.Equals, "machine not allocated") -} - -func (s *machineSuite) TestCreatePhysicalInterfaceForbidden(c *gc.C) { - server, machine := s.getServerAndMachine(c) - server.AddPostResponse(machine.interfacesURI()+"?op=create_physical", http.StatusForbidden, "machine not yours") - _, err := machine.CreatePhysicalInterface(s.minimalCreatePhysicalInterfaceArgs()) - c.Assert(err, jc.Satisfies, IsPermissionError) - c.Assert(err.Error(), gc.Equals, "machine not yours") -} - -func (s *machineSuite) TestCreatePhysicalInterfaceServiceUnavailable(c *gc.C) { - server, machine := s.getServerAndMachine(c) - server.AddPostResponse(machine.interfacesURI()+"?op=create_physical", http.StatusServiceUnavailable, "no ip addresses available") - _, err := machine.CreatePhysicalInterface(s.minimalCreatePhysicalInterfaceArgs()) - c.Assert(err, jc.Satisfies, IsCannotCompleteError) - c.Assert(err.Error(), gc.Equals, "no ip addresses available") -} - -func (s *machineSuite) TestCreatePhysicalInterfaceUnknown(c *gc.C) { - server, machine := s.getServerAndMachine(c) - server.AddPostResponse(machine.interfacesURI()+"?op=create_physical", http.StatusMethodNotAllowed, "wat?") - _, err := machine.CreatePhysicalInterface(s.minimalCreatePhysicalInterfaceArgs()) - c.Assert(err, jc.Satisfies, IsUnexpectedError) - c.Assert(err.Error(), gc.Equals, "unexpected: ServerError: 405 Method Not Allowed (wat?)") -} - const ( machineResponse = ` { From 1f606ec171daad40885d9206a5b6ead93a5929ac Mon Sep 17 00:00:00 2001 From: Tim Penhey Date: Fri, 8 Apr 2016 14:33:12 +1200 Subject: [PATCH 3/4] Rename the method. --- device.go | 12 ++++++------ device_test.go | 42 +++++++++++++++++++++--------------------- interfaces.go | 4 ++-- 3 files changed, 29 insertions(+), 29 deletions(-) diff --git a/device.go b/device.go index 4462ac6..3e28c38 100644 --- a/device.go +++ b/device.go @@ -51,9 +51,9 @@ func (d *device) Zone() Zone { return d.zone } -// CreatePhysicalInterfaceArgs is an argument struct for passing parameters to -// the Machine.CreatePhysicalInterface method. -type CreatePhysicalInterfaceArgs struct { +// CreateInterfaceArgs is an argument struct for passing parameters to +// the Machine.CreateInterface method. +type CreateInterfaceArgs struct { // Name of the interface (required). Name string // MACAddress is the MAC address of the interface (required). @@ -71,7 +71,7 @@ type CreatePhysicalInterfaceArgs struct { } // Validate checks the required fields are set for the arg structure. -func (a *CreatePhysicalInterfaceArgs) Validate() error { +func (a *CreateInterfaceArgs) Validate() error { if a.Name == "" { return errors.NotValidf("missing Name") } @@ -90,8 +90,8 @@ func (d *device) interfacesURI() string { return strings.Replace(d.resourceURI, "devices", "nodes", 1) + "interfaces/" } -// CreatePhysicalInterface implements Device. -func (d *device) CreatePhysicalInterface(args CreatePhysicalInterfaceArgs) (Interface, error) { +// CreateInterface implements Device. +func (d *device) CreateInterface(args CreateInterfaceArgs) (Interface, error) { if err := args.Validate(); err != nil { return nil, errors.Trace(err) } diff --git a/device_test.go b/device_test.go index 3b18330..bf371de 100644 --- a/device_test.go +++ b/device_test.go @@ -60,20 +60,20 @@ func (f *fakeVLAN) ID() int { return f.id } -func (s *controllerSuite) TestCreatePhysicalInterfaceArgsValidate(c *gc.C) { +func (s *controllerSuite) TestCreateInterfaceArgsValidate(c *gc.C) { for i, test := range []struct { - args CreatePhysicalInterfaceArgs + args CreateInterfaceArgs errText string }{{ errText: "missing Name not valid", }, { - args: CreatePhysicalInterfaceArgs{Name: "eth3"}, + args: CreateInterfaceArgs{Name: "eth3"}, errText: "missing MACAddress not valid", }, { - args: CreatePhysicalInterfaceArgs{Name: "eth3", MACAddress: "a-mac-address"}, + args: CreateInterfaceArgs{Name: "eth3", MACAddress: "a-mac-address"}, errText: `missing VLAN not valid`, }, { - args: CreatePhysicalInterfaceArgs{Name: "eth3", MACAddress: "a-mac-address", VLAN: &fakeVLAN{}}, + args: CreateInterfaceArgs{Name: "eth3", MACAddress: "a-mac-address", VLAN: &fakeVLAN{}}, }} { c.Logf("test %d", i) err := test.args.Validate() @@ -86,17 +86,17 @@ func (s *controllerSuite) TestCreatePhysicalInterfaceArgsValidate(c *gc.C) { } } -func (s *deviceSuite) TestCreatePhysicalInterfaceValidates(c *gc.C) { +func (s *deviceSuite) TestCreateInterfaceValidates(c *gc.C) { _, device := s.getServerAndDevice(c) - _, err := device.CreatePhysicalInterface(CreatePhysicalInterfaceArgs{}) + _, err := device.CreateInterface(CreateInterfaceArgs{}) c.Assert(err, jc.Satisfies, errors.IsNotValid) } -func (s *deviceSuite) TestCreatePhysicalInterface(c *gc.C) { +func (s *deviceSuite) TestCreateInterface(c *gc.C) { server, device := s.getServerAndDevice(c) server.AddPostResponse(device.interfacesURI()+"?op=create_physical", http.StatusOK, interfaceResponse) - iface, err := device.CreatePhysicalInterface(CreatePhysicalInterfaceArgs{ + iface, err := device.CreateInterface(CreateInterfaceArgs{ Name: "eth43", MACAddress: "some-mac-address", VLAN: &fakeVLAN{id: 33}, @@ -113,50 +113,50 @@ func (s *deviceSuite) TestCreatePhysicalInterface(c *gc.C) { c.Assert(form.Get("tags"), gc.Equals, "foo,bar") } -func (s *deviceSuite) minimalCreatePhysicalInterfaceArgs() CreatePhysicalInterfaceArgs { - return CreatePhysicalInterfaceArgs{ +func (s *deviceSuite) minimalCreateInterfaceArgs() CreateInterfaceArgs { + return CreateInterfaceArgs{ Name: "eth43", MACAddress: "some-mac-address", VLAN: &fakeVLAN{id: 33}, } } -func (s *deviceSuite) TestCreatePhysicalInterfaceNotFound(c *gc.C) { +func (s *deviceSuite) TestCreateInterfaceNotFound(c *gc.C) { server, device := s.getServerAndDevice(c) server.AddPostResponse(device.interfacesURI()+"?op=create_physical", http.StatusNotFound, "can't find device") - _, err := device.CreatePhysicalInterface(s.minimalCreatePhysicalInterfaceArgs()) + _, err := device.CreateInterface(s.minimalCreateInterfaceArgs()) c.Assert(err, jc.Satisfies, IsBadRequestError) c.Assert(err.Error(), gc.Equals, "can't find device") } -func (s *deviceSuite) TestCreatePhysicalInterfaceConflict(c *gc.C) { +func (s *deviceSuite) TestCreateInterfaceConflict(c *gc.C) { server, device := s.getServerAndDevice(c) server.AddPostResponse(device.interfacesURI()+"?op=create_physical", http.StatusConflict, "device not allocated") - _, err := device.CreatePhysicalInterface(s.minimalCreatePhysicalInterfaceArgs()) + _, err := device.CreateInterface(s.minimalCreateInterfaceArgs()) c.Assert(err, jc.Satisfies, IsBadRequestError) c.Assert(err.Error(), gc.Equals, "device not allocated") } -func (s *deviceSuite) TestCreatePhysicalInterfaceForbidden(c *gc.C) { +func (s *deviceSuite) TestCreateInterfaceForbidden(c *gc.C) { server, device := s.getServerAndDevice(c) server.AddPostResponse(device.interfacesURI()+"?op=create_physical", http.StatusForbidden, "device not yours") - _, err := device.CreatePhysicalInterface(s.minimalCreatePhysicalInterfaceArgs()) + _, err := device.CreateInterface(s.minimalCreateInterfaceArgs()) c.Assert(err, jc.Satisfies, IsPermissionError) c.Assert(err.Error(), gc.Equals, "device not yours") } -func (s *deviceSuite) TestCreatePhysicalInterfaceServiceUnavailable(c *gc.C) { +func (s *deviceSuite) TestCreateInterfaceServiceUnavailable(c *gc.C) { server, device := s.getServerAndDevice(c) server.AddPostResponse(device.interfacesURI()+"?op=create_physical", http.StatusServiceUnavailable, "no ip addresses available") - _, err := device.CreatePhysicalInterface(s.minimalCreatePhysicalInterfaceArgs()) + _, err := device.CreateInterface(s.minimalCreateInterfaceArgs()) c.Assert(err, jc.Satisfies, IsCannotCompleteError) c.Assert(err.Error(), gc.Equals, "no ip addresses available") } -func (s *deviceSuite) TestCreatePhysicalInterfaceUnknown(c *gc.C) { +func (s *deviceSuite) TestCreateInterfaceUnknown(c *gc.C) { server, device := s.getServerAndDevice(c) server.AddPostResponse(device.interfacesURI()+"?op=create_physical", http.StatusMethodNotAllowed, "wat?") - _, err := device.CreatePhysicalInterface(s.minimalCreatePhysicalInterfaceArgs()) + _, err := device.CreateInterface(s.minimalCreateInterfaceArgs()) c.Assert(err, jc.Satisfies, IsUnexpectedError) c.Assert(err.Error(), gc.Equals, "unexpected: ServerError: 405 Method Not Allowed (wat?)") } diff --git a/interfaces.go b/interfaces.go index 2b88d3a..eea0d9d 100644 --- a/interfaces.go +++ b/interfaces.go @@ -160,8 +160,8 @@ type Device interface { // Parent, Owner, MAC Addresses if needed - // CreatePhysicalInterface will create a physical interface for this machine. - CreatePhysicalInterface(CreatePhysicalInterfaceArgs) (Interface, error) + // CreateInterface will create a physical interface for this machine. + CreateInterface(CreateInterfaceArgs) (Interface, error) // Delete will remove this Device. Delete() error From 6eee4110d41284654da1d2b1404370844f99182a Mon Sep 17 00:00:00 2001 From: Tim Penhey Date: Fri, 8 Apr 2016 14:35:08 +1200 Subject: [PATCH 4/4] Change comment to refer to bug. --- device.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/device.go b/device.go index 3e28c38..38dd174 100644 --- a/device.go +++ b/device.go @@ -123,7 +123,8 @@ func (d *device) CreateInterface(args CreateInterfaceArgs) (Interface, error) { return nil, errors.Trace(err) } - // m.interfaceSet = append(m.interfaceSet, iface) + // TODO: add to the interfaces for the device when the interfaces are returned. + // lp:bug 1567213. return iface, nil }