Merge ~cgrabowski/maas:backport_to_3.2_add_update_interface_To_controller_ws_handler into maas:3.2

[Christian Grabowski]

ensure node is up to date before dehydrating
(cherry picked from commit 624798b11123c4696d80371ff66ab1b3487e76e8)

include controller interface form
(cherry picked from commit 819793ab4be1211efd768a09117f5f111aed833a)

use boot interface in test
(cherry picked from commit deef8f19afee791102c6a3f1349b30a19a87bfda)

move _update_obj_tags to the Node handler for both Controller and Machine handlers
(cherry picked from commit 459bb180a305056d41bc81a7c425bf30e31e582c)

move form import to correct file
(cherry picked from commit 608e094b658d1c5a96f707f436d712074bc91ab7)

move _get_node_or_permission_error to Node handler for use with Controller and Machine handlers
(cherry picked from commit b017a09d54633068ac0454cb99fab932d00a67b1)

move update_interface into Node handler for controller to inherit
(cherry picked from commit 948601b5b7a57bd9e8b8453e6de00976ba6394bf)

[MAAS Lander (maas-lander)]

UNIT TESTS
-b backport_to_3.2_add_update_interface_To_controller_ws_handler lp:~cgrabowski/maas/+git/maas into -b 3.2 lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas-tester/227/consoleText
COMMIT: cdb8a1ae2046b3c01c388fae27a5edc250bd0f4f

review: Needs Fixing

02234c4... by Christian Grabowski on 2022-07-29

fix discrepancies between master and 3.2

[MAAS Lander (maas-lander)]

UNIT TESTS
-b backport_to_3.2_add_update_interface_To_controller_ws_handler lp:~cgrabowski/maas/+git/maas into -b 3.2 lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas-tester/230/consoleText
COMMIT: 02234c475bedeb541c8c33f34a44a789cb31e97f

review: Needs Fixing

[Christian Grabowski (cgrabowski)]

jenkins: !test

[MAAS Lander (maas-lander)]

UNIT TESTS
-b backport_to_3.2_add_update_interface_To_controller_ws_handler lp:~cgrabowski/maas/+git/maas into -b 3.2 lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 02234c475bedeb541c8c33f34a44a789cb31e97f

review: Approve

[Christian Grabowski (cgrabowski)]

self-approve backport

review: Approve

diff --git a/src/maasserver/websockets/handlers/controller.py b/src/maasserver/websockets/handlers/controller.py
index d5ab7c2..7365b5b 100644
--- a/src/maasserver/websockets/handlers/controller.py
+++ b/src/maasserver/websockets/handlers/controller.py
@@ -93,6 +93,7 @@ class ControllerHandler(NodeHandler):
             "set_script_result_suppressed",
             "set_script_result_unsuppressed",
             "get_latest_failed_testing_script_results",
+            "update_interface",
         ]
         form = ControllerForm
         exclude = [
@@ -171,7 +172,11 @@ class ControllerHandler(NodeHandler):
         data = super().dehydrate(obj, data, for_list=for_list)
 
         vlan_counts = Counter()
-        for vlan_id in obj.vlan_ids:
+        vlan_ids = [
+            interface.vlan_id
+            for interface in obj.current_config.interface_set.all()
+        ]
+        for vlan_id in vlan_ids:
             vlan_counts[self._vlans_ha[vlan_id]] += 1
 
         data.update(
@@ -187,10 +192,8 @@ class ControllerHandler(NodeHandler):
             }
         )
         if not for_list:
-            data["vlan_ids"] = [
-                interface.vlan_id
-                for interface in obj.current_config.interface_set.all()
-            ]
+            data["vlan_ids"] = vlan_ids
+
         return data
 
     def dehydrate_versions(self, info):
diff --git a/src/maasserver/websockets/handlers/machine.py b/src/maasserver/websockets/handlers/machine.py
index 8c27240..e1ad271 100644
--- a/src/maasserver/websockets/handlers/machine.py
+++ b/src/maasserver/websockets/handlers/machine.py
@@ -45,8 +45,6 @@ from maasserver.forms.interface import (
     AcquiredBridgeInterfaceForm,
     BondInterfaceForm,
     BridgeInterfaceForm,
-    DeployedInterfaceForm,
-    InterfaceForm,
     PhysicalInterfaceForm,
     VLANInterfaceForm,
 )
@@ -637,11 +635,6 @@ class MachineHandler(NodeHandler):
                 else:
                     form.save()
 
-    def _update_obj_tags(self, obj, params):
-        if "tags" in params:
-            obj.tags = params["tags"]
-            obj.save(update_fields=["tags"])
-
     def update_disk(self, params):
         """Update disk information."""
         node = self._get_node_or_permission_error(
@@ -1041,28 +1034,6 @@ class MachineHandler(NodeHandler):
         else:
             raise ValidationError(form.errors)
 
-    def update_interface(self, params):
-        """Update the interface."""
-        node = self._get_node_or_permission_error(
-            params, permission=self._meta.edit_permission
-        )
-        interface = Interface.objects.get(
-            node_config__node=node, id=params["interface_id"]
-        )
-        if node.status == NODE_STATUS.DEPLOYED:
-            interface_form = DeployedInterfaceForm
-        else:
-            interface_form = InterfaceForm.get_interface_form(interface.type)
-        form = interface_form(instance=interface, data=params)
-        if form.is_valid():
-            interface = form.save()
-            self._update_obj_tags(interface, params)
-        else:
-            raise ValidationError(form.errors)
-        if "mode" in params:
-            self.link_subnet(params)
-        return self.full_dehydrate(node)
-
     def delete_interface(self, params):
         """Delete the interface."""
         node = self._get_node_or_permission_error(
@@ -1073,45 +1044,6 @@ class MachineHandler(NodeHandler):
         )
         interface.delete()
 
-    def link_subnet(self, params):
-        """Create or update the link."""
-        node = self._get_node_or_permission_error(
-            params, permission=self._meta.edit_permission
-        )
-        interface = Interface.objects.get(
-            node_config__node=node, id=params["interface_id"]
-        )
-        subnet = None
-        if "subnet" in params:
-            subnet = Subnet.objects.get(id=params["subnet"])
-        if "link_id" in params:
-            if interface.ip_addresses.filter(id=params["link_id"]).exists():
-                # We are updating an already existing link.  Which may have
-                # been deleted.
-                interface.update_link_by_id(
-                    params["link_id"],
-                    params["mode"],
-                    subnet,
-                    ip_address=params.get("ip_address", None),
-                )
-        else:
-            # We are creating a new link.
-            interface.link_subnet(
-                params["mode"],
-                subnet,
-                ip_address=params.get("ip_address", None),
-            )
-
-    def unlink_subnet(self, params):
-        """Delete the link."""
-        node = self._get_node_or_permission_error(
-            params, permission=self._meta.edit_permission
-        )
-        interface = Interface.objects.get(
-            node_config__node=node, id=params["interface_id"]
-        )
-        interface.unlink_subnet_by_id(params["link_id"])
-
     @asynchronous(timeout=45)
     def check_power(self, params):
         """Check the power state of the node."""
@@ -1140,12 +1072,6 @@ class MachineHandler(NodeHandler):
         d.addCallback(partial(deferToDatabase, update_state))
         return d
 
-    def _get_node_or_permission_error(self, params, permission=None):
-        node = self.get_object(params, permission=permission)
-        if node.locked:
-            raise HandlerPermissionError()
-        return node
-
     def get_workload_annotations(self, params):
         """Get the owner data for a machine, known as workload annotations."""
         machine = self._get_node_or_permission_error(
diff --git a/src/maasserver/websockets/handlers/node.py b/src/maasserver/websockets/handlers/node.py
index 7c34b5a..ae118db 100644
--- a/src/maasserver/websockets/handlers/node.py
+++ b/src/maasserver/websockets/handlers/node.py
@@ -9,6 +9,7 @@ from itertools import chain
 import logging
 from operator import attrgetter, itemgetter
 
+from django.core.exceptions import ValidationError
 from django.db.models import Prefetch
 from lxml import etree
 
@@ -21,6 +22,11 @@ from maasserver.enum import (
     NODE_TYPE,
     POWER_STATE,
 )
+from maasserver.forms.interface import (
+    ControllerInterfaceForm,
+    DeployedInterfaceForm,
+    InterfaceForm,
+)
 from maasserver.models import (
     CacheSet,
     Config,
@@ -28,6 +34,7 @@ from maasserver.models import (
     Interface,
     NUMANode,
     PhysicalBlockDevice,
+    Subnet,
     Tag,
     VirtualBlockDevice,
     VolumeGroup,
@@ -1180,3 +1187,78 @@ class NodeHandler(TimestampedModelHandler):
                     f"Cannot add tag {tag.name} to node because it has a definition"
                 )
         node.tags.set(tags)
+
+    def update_interface(self, params):
+        """Update the interface."""
+        node = self._get_node_or_permission_error(
+            params, permission=self._meta.edit_permission
+        )
+        interface = Interface.objects.get(
+            node_config__node=node, id=params["interface_id"]
+        )
+        if node.is_controller:
+            interface_form = ControllerInterfaceForm
+        elif node.status == NODE_STATUS.DEPLOYED:
+            interface_form = DeployedInterfaceForm
+        else:
+            interface_form = InterfaceForm.get_interface_form(interface.type)
+        form = interface_form(instance=interface, data=params)
+        if form.is_valid():
+            interface = form.save()
+            self._update_obj_tags(interface, params)
+        else:
+            raise ValidationError(form.errors)
+        if "mode" in params:
+            self.link_subnet(params)
+        node.refresh_from_db()
+        return self.full_dehydrate(node)
+
+    def _get_node_or_permission_error(self, params, permission=None):
+        node = self.get_object(params, permission=permission)
+        if node.locked:
+            raise HandlerPermissionError()
+        return node
+
+    def _update_obj_tags(self, obj, params):
+        if "tags" in params:
+            obj.tags = params["tags"]
+            obj.save(update_fields=["tags"])
+
+    def link_subnet(self, params):
+        """Create or update the link."""
+        node = self._get_node_or_permission_error(
+            params, permission=self._meta.edit_permission
+        )
+        interface = Interface.objects.get(
+            node_config__node=node, id=params["interface_id"]
+        )
+        subnet = None
+        if "subnet" in params:
+            subnet = Subnet.objects.get(id=params["subnet"])
+        if "link_id" in params:
+            if interface.ip_addresses.filter(id=params["link_id"]).exists():
+                # We are updating an already existing link.  Which may have
+                # been deleted.
+                interface.update_link_by_id(
+                    params["link_id"],
+                    params["mode"],
+                    subnet,
+                    ip_address=params.get("ip_address", None),
+                )
+        else:
+            # We are creating a new link.
+            interface.link_subnet(
+                params["mode"],
+                subnet,
+                ip_address=params.get("ip_address", None),
+            )
+
+    def unlink_subnet(self, params):
+        """Delete the link."""
+        node = self._get_node_or_permission_error(
+            params, permission=self._meta.edit_permission
+        )
+        interface = Interface.objects.get(
+            node_config__node=node, id=params["interface_id"]
+        )
+        interface.unlink_subnet_by_id(params["link_id"])
diff --git a/src/maasserver/websockets/handlers/tests/test_controller.py b/src/maasserver/websockets/handlers/tests/test_controller.py
index f0aab9b..eda0b66 100644
--- a/src/maasserver/websockets/handlers/tests/test_controller.py
+++ b/src/maasserver/websockets/handlers/tests/test_controller.py
@@ -469,3 +469,30 @@ class TestControllerHandler(MAASServerTestCase):
         controller.refresh_from_db()
         # deleting a region+rack controller changes it to just a region controller
         self.assertEqual(controller.node_type, NODE_TYPE.REGION_CONTROLLER)
+
+    def test_update_interface(self):
+        admin = factory.make_admin()
+        vlan = factory.make_VLAN()
+        handler = ControllerHandler(admin, {}, None)
+        controller = factory.make_RegionRackController()
+        request = {
+            "name": controller.boot_interface.name,
+            "mac_address": controller.boot_interface.mac_address,
+            "tags": [],
+            "fabric": vlan.fabric.id,
+            "vlan": vlan.id,
+            "mode": "link_up",
+            "link_connected": True,
+            "link_speed": 0,
+            "interface_speed": 0,
+            "formatted_link_speed": 0,
+            "formatted_interface_speed": 0,
+            "type": controller.boot_interface.type,
+            "parents": controller.boot_interface.parents,
+            "primary": None,
+            "system_id": controller.system_id,
+            "interface_id": controller.boot_interface.id,
+        }
+        handler.update_interface(request)
+        controller.refresh_from_db()
+        self.assertEqual(controller.boot_interface.vlan, vlan)