Merge pull request #5883 from babbageclunk/release-container-addresses

Add ReleaseContainerAddresses to providers that support networking

At the moment we don't clean up the addresses allocated by AllocateContainerAddresses, which means that they are leaked until the machine gets cleaned up (assuming it does). Add a corresponding ReleaseContainerAddresses to the providers with networking support: maas, ec2, vsphere and dummy - it's a stub in all of them except the MAAS provider, the only one that manages container addresses at the moment.

This will be used by a worker that runs after the container machine is removed from state (coming in a subsequent PR).

Update the gomaasapi dependency for access to some test methods needed in the tests for the MAAS 1 code path.

Part of https://bugs.launchpad.net/juju-core/+bug/1585878


(Review request: http://reviews.vapour.ws/r/5317/)

dependencies.tsv

@@ -21,7 +21,7 @@ github.com/juju/go4	git	40d72ab9641a2a8c36a9c46a51e28367115c8e59	2016-02-22T16:3
 github.com/juju/gojsonpointer	git	afe8b77aa08f272b49e01b82de78510c11f61500	2015-02-04T19:46:29Z
 github.com/juju/gojsonreference	git	f0d24ac5ee330baa21721cdff56d45e4ee42628e	2015-02-04T19:46:33Z
 github.com/juju/gojsonschema	git	e1ad140384f254c82f89450d9a7c8dd38a632838	2015-03-12T17:00:16Z
-github.com/juju/gomaasapi	git	5bd7212f416a2d801e4a39800b66e1ee4461c42e	2016-05-03T13:03:30Z
+github.com/juju/gomaasapi	git	c4008a71e7212cb6a99a9c17bb218034927d82b7	2016-07-28T00:29:23Z
 github.com/juju/govmomi	git	4354a88d4b34abe467215f77c2fc1cb9f78b66f7	2015-04-24T01:54:48Z
 github.com/juju/httpprof	git	14bf14c307672fd2456bdbf35d19cf0ccd3cf565	2014-12-17T16:00:36Z
 github.com/juju/httprequest	git	796aaafaf712f666df58d31a482c51233038bf9f	2016-05-03T15:03:27Z

environs/networking.go

@@ -46,6 +46,10 @@ type Networking interface {
 	// container NICs in preparedInfo, hosted by the hostInstanceID. Returns the
 	// network config including all allocated addresses on success.
 	AllocateContainerAddresses(hostInstanceID instance.Id, containerTag names.MachineTag, preparedInfo []network.InterfaceInfo) ([]network.InterfaceInfo, error)
+
+	// ReleaseContainerAddresses releases the previously allocated
+	// addresses matching the interface infos passed in.
+	ReleaseContainerAddresses(interfaces []network.InterfaceInfo) error
 }
 
 // NetworkingEnviron combines the standard Environ interface with the

provider/dummy/environs.go

@@ -1537,3 +1537,7 @@ func delay() {
 func (e *environ) AllocateContainerAddresses(hostInstanceID instance.Id, containerTag names.MachineTag, preparedInfo []network.InterfaceInfo) ([]network.InterfaceInfo, error) {
 	return nil, errors.NotSupportedf("container address allocation")
 }
+
+func (e *environ) ReleaseContainerAddresses(interfaces []network.InterfaceInfo) error {
+	return errors.NotSupportedf("container address allocation")
+}

provider/ec2/environ.go

@@ -1729,3 +1729,7 @@ func ec2ErrCode(err error) string {
 func (e *environ) AllocateContainerAddresses(hostInstanceID instance.Id, containerTag names.MachineTag, preparedInfo []network.InterfaceInfo) ([]network.InterfaceInfo, error) {
 	return nil, errors.NotSupportedf("container address allocation")
 }
+
+func (e *environ) ReleaseContainerAddresses(interfaces []network.InterfaceInfo) error {
+	return errors.NotSupportedf("container address allocation")
+}

provider/maas/environ.go

@@ -2199,3 +2199,64 @@ func (env *maasEnviron) allocateContainerAddresses2(hostInstanceID instance.Id,
 	logger.Debugf("allocated device interfaces: %+v", finalInterfaces)
 	return finalInterfaces, nil
 }
+
+func (env *maasEnviron) ReleaseContainerAddresses(interfaces []network.InterfaceInfo) error {
+	macAddresses := make([]string, len(interfaces))
+	for i, info := range interfaces {
+		macAddresses[i] = info.MACAddress
+	}
+	if !env.usingMAAS2() {
+		return env.releaseContainerAddresses1(macAddresses)
+	}
+	return env.releaseContainerAddresses2(macAddresses)
+}
+
+func (env *maasEnviron) releaseContainerAddresses1(macAddresses []string) error {
+	devicesAPI := env.getMAASClient().GetSubObject("devices")
+	values := url.Values{}
+	for _, address := range macAddresses {
+		values.Add("mac_address", address)
+	}
+	result, err := devicesAPI.CallGet("list", values)
+	if err != nil {
+		return errors.Trace(err)
+	}
+	devicesArray, err := result.GetArray()
+	if err != nil {
+		return errors.Trace(err)
+	}
+	deviceIds := make([]string, len(devicesArray))
+	for i, deviceItem := range devicesArray {
+		deviceMap, err := deviceItem.GetMap()
+		if err != nil {
+			return errors.Trace(err)
+		}
+		id, err := deviceMap["system_id"].GetString()
+		if err != nil {
+			return errors.Trace(err)
+		}
+		deviceIds[i] = id
+	}
+
+	for _, id := range deviceIds {
+		err := devicesAPI.GetSubObject(id).Delete()
+		if err != nil {
+			return errors.Annotatef(err, "deleting device %s", id)
+		}
+	}
+	return nil
+}
+
+func (env *maasEnviron) releaseContainerAddresses2(macAddresses []string) error {
+	devices, err := env.maasController.Devices(gomaasapi.DevicesArgs{MACAddresses: macAddresses})
+	if err != nil {
+		return errors.Trace(err)
+	}
+	for _, device := range devices {
+		err = device.Delete()
+		if err != nil {
+			return errors.Annotatef(err, "deleting device %s", device.SystemID())
+		}
+	}
+	return nil
+}

provider/maas/environ_whitebox_test.go

@@ -1098,3 +1098,32 @@ func (s *environSuite) TestStartInstanceDistributionOneAssigned(c *gc.C) {
 		"acquire", "acquire",
 	})
 }
+
+func (s *environSuite) TestReleaseContainerAddresses(c *gc.C) {
+	s.testMAASObject.TestServer.AddDevice(&gomaasapi.TestDevice{
+		SystemId:   "device1",
+		MACAddress: "mac1",
+	})
+	s.testMAASObject.TestServer.AddDevice(&gomaasapi.TestDevice{
+		SystemId:   "device2",
+		MACAddress: "mac2",
+	})
+	s.testMAASObject.TestServer.AddDevice(&gomaasapi.TestDevice{
+		SystemId:   "device3",
+		MACAddress: "mac3",
+	})
+
+	env := s.makeEnviron()
+	err := env.ReleaseContainerAddresses([]network.InterfaceInfo{
+		{MACAddress: "mac1"},
+		{MACAddress: "mac3"},
+		{MACAddress: "mac4"},
+	})
+	c.Assert(err, jc.ErrorIsNil)
+
+	var systemIds []string
+	for systemId, _ := range s.testMAASObject.TestServer.Devices() {
+		systemIds = append(systemIds, systemId)
+	}
+	c.Assert(systemIds, gc.DeepEquals, []string{"device2"})
+}

provider/maas/maas2_environ_whitebox_test.go

@@ -1005,6 +1005,7 @@ func (suite *maas2EnvironSuite) TestAllocateContainerAddressesSingleNic(c *gc.C)
 		systemID:     "foo",
 	}
 	controller := &fakeController{
+		Stub: &testing.Stub{},
 		machines: []gomaasapi.Machine{&fakeMachine{
 			Stub:         &testing.Stub{},
 			systemID:     "1",
@@ -1160,6 +1161,7 @@ func (suite *maas2EnvironSuite) TestAllocateContainerAddressesDualNic(c *gc.C) {
 		Stub:         &testing.Stub{},
 	}
 	controller := &fakeController{
+		Stub: &testing.Stub{},
 		machines: []gomaasapi.Machine{&fakeMachine{
 			Stub:         &testing.Stub{},
 			systemID:     "1",
@@ -1667,3 +1669,62 @@ func (suite *maas2EnvironSuite) TestConstraintsValidatorVocab(c *gc.C) {
 	_, err = validator.Validate(cons)
 	c.Assert(err, gc.ErrorMatches, "invalid constraint value: arch=ppc64el\nvalid values are: \\[amd64 armhf\\]")
 }
+
+func (suite *maas2EnvironSuite) TestReleaseContainerAddresses(c *gc.C) {
+	dev1 := newFakeDeviceWithMAC("eleven")
+	dev2 := newFakeDeviceWithMAC("will")
+	controller := newFakeController()
+	controller.devices = []gomaasapi.Device{dev1, dev2}
+
+	env := suite.makeEnviron(c, controller)
+	err := env.ReleaseContainerAddresses([]network.InterfaceInfo{
+		{MACAddress: "will"},
+		{MACAddress: "dustin"},
+		{MACAddress: "eleven"},
+	})
+	c.Assert(err, jc.ErrorIsNil)
+
+	args, ok := getArgs(c, controller.Calls()).(gomaasapi.DevicesArgs)
+	c.Assert(ok, jc.IsTrue)
+	expected := gomaasapi.DevicesArgs{MACAddresses: []string{"will", "dustin", "eleven"}}
+	c.Assert(args, gc.DeepEquals, expected)
+
+	dev1.CheckCallNames(c, "Delete")
+	dev2.CheckCallNames(c, "Delete")
+}
+
+func (suite *maas2EnvironSuite) TestReleaseContainerAddressesErrorGettingDevices(c *gc.C) {
+	controller := newFakeControllerWithErrors(errors.New("Everything done broke"))
+	env := suite.makeEnviron(c, controller)
+	err := env.ReleaseContainerAddresses([]network.InterfaceInfo{{MACAddress: "anything"}})
+	c.Assert(err, gc.ErrorMatches, "Everything done broke")
+}
+
+func (suite *maas2EnvironSuite) TestReleaseContainerAddressesErrorDeletingDevice(c *gc.C) {
+	dev1 := newFakeDeviceWithMAC("eleven")
+	dev1.systemID = "hopper"
+	dev1.SetErrors(errors.New("don't delete me"))
+	controller := newFakeController()
+	controller.devices = []gomaasapi.Device{dev1}
+
+	env := suite.makeEnviron(c, controller)
+	err := env.ReleaseContainerAddresses([]network.InterfaceInfo{
+		{MACAddress: "eleven"},
+	})
+	c.Assert(err, gc.ErrorMatches, "deleting device hopper: don't delete me")
+
+	_, ok := getArgs(c, controller.Calls()).(gomaasapi.DevicesArgs)
+	c.Assert(ok, jc.IsTrue)
+
+	dev1.CheckCallNames(c, "Delete")
+}
+
+func newFakeDeviceWithMAC(macAddress string) *fakeDevice {
+	return &fakeDevice{
+		Stub: &testing.Stub{},
+		interface_: &fakeInterface{
+			Stub:       &testing.Stub{},
+			macAddress: macAddress,
+		},
+	}
+}

provider/maas/maas2_test.go

@@ -87,8 +87,9 @@ func newFakeControllerWithFiles(files ...gomaasapi.File) *fakeController {
 	return &fakeController{Stub: &testing.Stub{}, files: files}
 }
 
-func (c *fakeController) Devices(gomaasapi.DevicesArgs) ([]gomaasapi.Device, error) {
-	return c.devices, nil
+func (c *fakeController) Devices(args gomaasapi.DevicesArgs) ([]gomaasapi.Device, error) {
+	c.MethodCall(c, "Devices", args)
+	return c.devices, c.NextErr()
 }
 
 func (c *fakeController) Machines(args gomaasapi.MachinesArgs) ([]gomaasapi.Machine, error) {
@@ -503,3 +504,8 @@ func (d *fakeDevice) CreateInterface(args gomaasapi.CreateInterfaceArgs) (gomaas
 	d.interfaceSet = append(d.interfaceSet, d.interface_)
 	return d.interface_, d.NextErr()
 }
+
+func (d *fakeDevice) Delete() error {
+	d.MethodCall(d, "Delete")
+	return d.NextErr()
+}

provider/vsphere/environ_network.go

@@ -49,6 +49,12 @@ func (env *environ) Ports() ([]network.PortRange, error) {
 	return nil, errors.Trace(errors.NotSupportedf("Ports"))
 }
 
+// AllocateContainerAddresses implements environs.Networking.
 func (e *environ) AllocateContainerAddresses(hostInstanceID instance.Id, containerTag names.MachineTag, preparedInfo []network.InterfaceInfo) ([]network.InterfaceInfo, error) {
 	return nil, errors.NotSupportedf("container address allocation")
 }
+
+// ReleaseContainerAddresses implements environs.Networking.
+func (e *environ) ReleaseContainerAddresses(interfaces []network.InterfaceInfo) error {
+	return errors.NotSupportedf("container address allocation")
+}