Merge pull request #26 from howbazaar/create-interfaces

Add CreatePhysicalInterface to Device.

Even though it is possible to create a physical interface for a machine, you would always do it on a device.

device.go

@@ -4,7 +4,9 @@
 package gomaasapi
 
 import (
+	"fmt"
 	"net/http"
+	"strings"
 
 	"github.com/juju/errors"
 	"github.com/juju/schema"
@@ -49,6 +51,83 @@ func (d *device) Zone() Zone {
 	return d.zone
 }
 
+// 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).
+	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 *CreateInterfaceArgs) 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/"
+}
+
+// CreateInterface implements Device.
+func (d *device) CreateInterface(args CreateInterfaceArgs) (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)
+	}
+
+	// TODO: add to the interfaces for the device when the interfaces are returned.
+	// lp:bug 1567213.
+	return iface, nil
+}
+
 // Delete implements Device.
 func (d *device) Delete() error {
 	err := d.controller.delete(d.resourceURI)

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) TestCreateInterfaceArgsValidate(c *gc.C) {
+	for i, test := range []struct {
+		args    CreateInterfaceArgs
+		errText string
+	}{{
+		errText: "missing Name not valid",
+	}, {
+		args:    CreateInterfaceArgs{Name: "eth3"},
+		errText: "missing MACAddress not valid",
+	}, {
+		args:    CreateInterfaceArgs{Name: "eth3", MACAddress: "a-mac-address"},
+		errText: `missing VLAN not valid`,
+	}, {
+		args: CreateInterfaceArgs{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) TestCreateInterfaceValidates(c *gc.C) {
+	_, device := s.getServerAndDevice(c)
+	_, err := device.CreateInterface(CreateInterfaceArgs{})
+	c.Assert(err, jc.Satisfies, errors.IsNotValid)
+}
+
+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.CreateInterface(CreateInterfaceArgs{
+		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) minimalCreateInterfaceArgs() CreateInterfaceArgs {
+	return CreateInterfaceArgs{
+		Name:       "eth43",
+		MACAddress: "some-mac-address",
+		VLAN:       &fakeVLAN{id: 33},
+	}
+}
+
+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.CreateInterface(s.minimalCreateInterfaceArgs())
+	c.Assert(err, jc.Satisfies, IsBadRequestError)
+	c.Assert(err.Error(), gc.Equals, "can't find device")
+}
+
+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.CreateInterface(s.minimalCreateInterfaceArgs())
+	c.Assert(err, jc.Satisfies, IsBadRequestError)
+	c.Assert(err.Error(), gc.Equals, "device not allocated")
+}
+
+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.CreateInterface(s.minimalCreateInterfaceArgs())
+	c.Assert(err, jc.Satisfies, IsPermissionError)
+	c.Assert(err.Error(), gc.Equals, "device not yours")
+}
+
+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.CreateInterface(s.minimalCreateInterfaceArgs())
+	c.Assert(err, jc.Satisfies, IsCannotCompleteError)
+	c.Assert(err.Error(), gc.Equals, "no ip addresses available")
+}
+
+func (s *deviceSuite) TestCreateInterfaceUnknown(c *gc.C) {
+	server, device := s.getServerAndDevice(c)
+	server.AddPostResponse(device.interfacesURI()+"?op=create_physical", http.StatusMethodNotAllowed, "wat?")
+	_, err := device.CreateInterface(s.minimalCreateInterfaceArgs())
+	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,29 +172,29 @@ 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()
 	c.Assert(err, jc.ErrorIsNil)
 }
 
 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)

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"]),

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"})

interfaces.go

@@ -160,6 +160,9 @@ type Device interface {
 
 	// Parent, Owner, MAC Addresses if needed
 
+	// CreateInterface will create a physical interface for this machine.
+	CreateInterface(CreateInterfaceArgs) (Interface, error)
+
 	// Delete will remove this Device.
 	Delete() error
 }
@@ -194,6 +197,7 @@ type Machine interface {
 
 	Zone() Zone
 
+	// Start the machine and install the operating system specified in the args.
 	Start(StartArgs) error
 }
 
@@ -240,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.

link.go

@@ -92,6 +92,7 @@ func link_2_0(source map[string]interface{}) (*link, error) {
 	}
 	defaults := schema.Defaults{
 		"ip_address": "",
+		"subnet":     schema.Omit,
 	}
 	checker := schema.FieldMap(fields, defaults)
 	coerced, err := checker.Coerce(source, nil)
@@ -102,9 +103,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{