Merge ~ltrager/maas:release_auto_ip_on_power_off into maas:master

[Lee Trager]

Release AUTOIP addresses when powered off in a non-use status

Commissioning and testing now require AUTOIP addresses to be assigned to
allow for network validation testing. The signal to end these statuses
comes before the machine is actually turned off. Further users may keep
the machine on after commissioning/testing to debug script issues. To
ensure that AUTOIPs are released MAAS will now release any assigned
AUTOIP if the machine isn't currently in a status that needs an IP and
is off.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b release_auto_ip_on_power_off lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/6186/console
COMMIT: 979449786d721e16c0e607e6744933a63efcae87

review: Needs Fixing

[Blake Rouse (blake-rouse)]

Looks good.

review: Approve

[MAAS Lander (maas-lander)]

LANDING
-b release_auto_ip_on_power_off lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/6197/consoleText

de74f32... by Lee Trager on 2019-08-16

Fix broken test

diff --git a/src/maasserver/models/signals/nodes.py b/src/maasserver/models/signals/nodes.py
index c6b4a33..a1d2f06 100644
--- a/src/maasserver/models/signals/nodes.py
+++ b/src/maasserver/models/signals/nodes.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Respond to node changes."""
@@ -13,7 +13,10 @@ from django.db.models.signals import (
     pre_delete,
     pre_save,
 )
-from maasserver.enum import NODE_STATUS
+from maasserver.enum import (
+    NODE_STATUS,
+    POWER_STATE,
+)
 from maasserver.models import (
     Controller,
     Device,
@@ -136,5 +139,29 @@ for klass in NODE_CLASSES:
         sender=klass)
 
 
+def release_auto_ips(node, old_values, deleted=False):
+    """Release auto assigned IPs once the machine is off and ready."""
+    # Only machines use AUTO_IPs.
+    if not node.is_machine:
+        return
+    # Commissioning and testing may acquire an AUTO_IP for network testing.
+    # Users may keep the machine on after commissioning/testing to debug
+    # issues where the assigned IP is still in use. Wait till the machine
+    # is off and not in a status which will have an IP in use.
+    if (node.power_state == POWER_STATE.OFF and node.status not in (
+            NODE_STATUS.COMMISSIONING,
+            NODE_STATUS.DEPLOYED,
+            NODE_STATUS.DEPLOYING,
+            NODE_STATUS.DISK_ERASING,
+            NODE_STATUS.RESCUE_MODE,
+            NODE_STATUS.ENTERING_RESCUE_MODE,
+            NODE_STATUS.TESTING)):
+        node.release_interface_config()
+
+
+for klass in [Node, Machine]:
+    signals.watch_fields(release_auto_ips, klass, ['power_state'])
+
+
 # Enable all signals by default.
 signals.enable()
diff --git a/src/maasserver/models/signals/tests/test_nodes.py b/src/maasserver/models/signals/tests/test_nodes.py
index 5e8ab15..d909b4a 100644
--- a/src/maasserver/models/signals/tests/test_nodes.py
+++ b/src/maasserver/models/signals/tests/test_nodes.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test the behaviour of node signals."""
@@ -8,9 +8,17 @@ __all__ = []
 import random
 
 from maasserver.enum import (
+    IPADDRESS_TYPE,
     NODE_STATUS,
     NODE_STATUS_CHOICES,
     NODE_TYPE,
+    NODE_TYPE_CHOICES,
+    POWER_STATE,
+    POWER_STATE_CHOICES,
+)
+from maasserver.models import (
+    Node,
+    StaticIPAddress,
 )
 from maasserver.models.service import (
     RACK_SERVICES,
@@ -212,3 +220,95 @@ class TestNodeCreateServices(MAASServerTestCase):
         self.assertThat(
             {service.name for service in services},
             Equals(REGION_SERVICES))
+
+
+class TestNodeReleasesAutoIPs(MAASServerTestCase):
+    """Test that auto ips are released when node power is off."""
+
+    def __init__(self, *args, **kwargs):
+        self.reserved_statuses = [
+            NODE_STATUS.COMMISSIONING,
+            NODE_STATUS.DEPLOYED,
+            NODE_STATUS.DEPLOYING,
+            NODE_STATUS.DISK_ERASING,
+            NODE_STATUS.RESCUE_MODE,
+            NODE_STATUS.ENTERING_RESCUE_MODE,
+            NODE_STATUS.TESTING,
+        ]
+        self.scenarios = [
+            (status_label, {'status': status})
+            for status, status_label in NODE_STATUS_CHOICES
+            if status not in self.reserved_statuses
+        ]
+        super().__init__(*args, **kwargs)
+
+    def test__releases_interface_config_when_turned_off(self):
+        machine = factory.make_Machine_with_Interface_on_Subnet(
+            status=random.choice(self.reserved_statuses),
+            power_state=POWER_STATE.ON)
+        for interface in machine.interface_set.all():
+            interface.claim_auto_ips()
+
+        # Hack to get around node transition model
+        Node.objects.filter(id=machine.id).update(status=self.status)
+        machine = reload_object(machine)
+        machine.power_state = POWER_STATE.OFF
+        machine.save()
+
+        for ip in StaticIPAddress.objects.filter(
+                interface__node=machine, alloc_type=IPADDRESS_TYPE.AUTO):
+            self.assertIsNone(ip.ip)
+
+    def test__does_nothing_if_not_off(self):
+        machine = factory.make_Machine_with_Interface_on_Subnet(
+            status=random.choice(self.reserved_statuses),
+            power_state=POWER_STATE.ON)
+        for interface in machine.interface_set.all():
+            interface.claim_auto_ips()
+
+        # Hack to get around node transition model
+        Node.objects.filter(id=machine.id).update(status=self.status)
+        machine = reload_object(machine)
+        machine.power_state = factory.pick_choice(
+            POWER_STATE_CHOICES, but_not=[POWER_STATE.OFF])
+        machine.save()
+
+        for ip in StaticIPAddress.objects.filter(
+                interface__node=machine, alloc_type=IPADDRESS_TYPE.AUTO):
+            self.assertIsNotNone(ip.ip)
+
+    def test__does_nothing_if_reserved_status(self):
+        machine = factory.make_Machine_with_Interface_on_Subnet(
+            status=self.status, power_state=POWER_STATE.ON)
+        for interface in machine.interface_set.all():
+            interface.claim_auto_ips()
+
+        # Hack to get around node transition model
+        Node.objects.filter(id=machine.id).update(
+            status=random.choice(self.reserved_statuses))
+        machine = reload_object(machine)
+        machine.power_state = POWER_STATE.OFF
+        machine.save()
+
+        for ip in StaticIPAddress.objects.filter(
+                interface__node=machine, alloc_type=IPADDRESS_TYPE.AUTO):
+            self.assertIsNotNone(ip.ip)
+
+    def test__does_nothing_if_not_machine(self):
+        node = factory.make_Node_with_Interface_on_Subnet(
+            node_type=factory.pick_choice(
+                NODE_TYPE_CHOICES, but_not=[NODE_TYPE.MACHINE]),
+            status=random.choice(self.reserved_statuses),
+            power_state=POWER_STATE.ON)
+        for interface in node.interface_set.all():
+            interface.claim_auto_ips()
+
+        # Hack to get around node transition model
+        Node.objects.filter(id=node.id).update(status=self.status)
+        node = reload_object(node)
+        node.power_state = POWER_STATE.OFF
+        node.save()
+
+        for ip in StaticIPAddress.objects.filter(
+                interface__node=node, alloc_type=IPADDRESS_TYPE.AUTO):
+            self.assertIsNotNone(ip.ip)
diff --git a/src/maasserver/models/tests/test_node.py b/src/maasserver/models/tests/test_node.py
index f428cc4..92d391f 100644
--- a/src/maasserver/models/tests/test_node.py
+++ b/src/maasserver/models/tests/test_node.py
@@ -3868,7 +3868,10 @@ class TestNode(MAASServerTestCase):
         release = self.patch_autospec(node, 'release_interface_config')
         self.patch(Node, '_clear_status_expires')
         node.update_power_state(POWER_STATE.OFF)
-        self.assertThat(release, MockCalledOnceWith())
+        # release_interface_config() is called once by update_power_state and
+        # a second time by the release_auto_ips() signal. Whichever runs
+        # second is a noop but is needed for commissioning/testing.
+        self.assertThat(release, MockCallsMatch(call(), call()))
 
     def test_update_power_state_doesnt_release_interface_config_if_on(self):
         node = factory.make_Node(