Merge ~bjornt/maas:bug-1932136-disconnected-interfaces-enabled into maas:master

[Björn Tillenius]

LP #1932136: interface with a warning is not configured properly

Now all interfaces are considered enabled, since an admin should
explicitly disable an interface if he doesn't want it configured.

I also noted that we never set link_connected, so all interfaces were
considered connected. Now we set link_connected to False if there's no
link.

[Alberto Donato (ack)]

nice, +1

one comment inline

review: Approve

b08b264... by Björn Tillenius on 2021-06-18

Fix comment.

diff --git a/src/metadataserver/builtin_scripts/network.py b/src/metadataserver/builtin_scripts/network.py
index 886b631..95606a1 100644
--- a/src/metadataserver/builtin_scripts/network.py
+++ b/src/metadataserver/builtin_scripts/network.py
@@ -204,9 +204,9 @@ def update_physical_interface(
     # associated with them. We still model them as physical NICs.
     mac_address = port["address"] if port else network["hwaddr"]
     update_fields = set()
-    is_enabled = network["state"] == "up"
+    is_connected = network["state"] == "up"
     if port is not None:
-        is_enabled = is_enabled and port["link_detected"]
+        is_connected = is_connected and port["link_detected"]
     # If an interface with same name and different MAC exists in the
     # machine, delete it. Interface names are unique on a machine, so this
     # might be an old interface which was removed and recreated with a
@@ -219,7 +219,7 @@ def update_physical_interface(
         defaults={
             "node": node,
             "name": name,
-            "enabled": is_enabled,
+            "enabled": True,
             "acquired": (
                 node.is_controller or node.status == NODE_STATUS.DEPLOYED
             ),
@@ -235,8 +235,8 @@ def update_physical_interface(
         )
     # Don't update the VLAN unless:
     # (1) The interface's VLAN wasn't previously known.
-    # (2) The interface is administratively enabled.
-    if interface.vlan is None and is_enabled:
+    # (2) The interface is connected
+    if interface.vlan is None and is_connected:
         if hints is not None:
             new_vlan = guess_vlan_from_hints(node, name, hints)
         if new_vlan is None:
@@ -254,9 +254,9 @@ def update_physical_interface(
             update_fields.add("node")
         interface.name = name
         update_fields.add("name")
-    if interface.enabled != is_enabled:
-        interface.enabled = is_enabled
-        update_fields.add("enabled")
+    if interface.link_connected != is_connected:
+        interface.link_connected = is_connected
+        update_fields.add("link_connected")
 
     # Update all the IP address on this interface. Fix the VLAN the
     # interface belongs to so its the same as the links.
diff --git a/src/metadataserver/builtin_scripts/tests/test_network.py b/src/metadataserver/builtin_scripts/tests/test_network.py
index 6e27c06..dd1657c 100644
--- a/src/metadataserver/builtin_scripts/tests/test_network.py
+++ b/src/metadataserver/builtin_scripts/tests/test_network.py
@@ -504,13 +504,15 @@ class TestUpdateInterfaces(MAASServerTestCase, UpdateInterfacesMixin):
         eth0 = PhysicalInterface.objects.get(name="eth0", node=controller)
         self.assertEqual("11:11:11:11:11:11", eth0.mac_address)
         self.assertTrue(eth0.enabled)
+        self.assertTrue(eth0.link_connected)
         self.assertEqual(
             Fabric.objects.get_default_fabric().get_default_vlan(), eth0.vlan
         )
         self.assertEqual([], list(eth0.parents.all()))
         eth1 = PhysicalInterface.objects.get(name="eth1", node=controller)
         self.assertEqual("22:22:22:22:22:22", eth1.mac_address)
-        self.assertFalse(eth1.enabled)
+        self.assertTrue(eth1.enabled)
+        self.assertFalse(eth1.link_connected)
         self.assertIsNone(eth1.vlan)
         self.assertEqual([], list(eth1.parents.all()))
 
@@ -527,6 +529,7 @@ class TestUpdateInterfaces(MAASServerTestCase, UpdateInterfacesMixin):
         eth0 = PhysicalInterface.objects.get(name="eth0", node=controller)
         self.assertEqual(eth0_network.hwaddr, eth0.mac_address)
         self.assertTrue(eth0.enabled)
+        self.assertTrue(eth0.link_connected)
         self.assertEqual([], list(eth0.parents.all()))
         vlan0100 = Interface.objects.get(name="vlan0100", node=controller)
         self.assertEqual(INTERFACE_TYPE.VLAN, vlan0100.type)
@@ -535,6 +538,7 @@ class TestUpdateInterfaces(MAASServerTestCase, UpdateInterfacesMixin):
         # have different MAC addresses.
         self.assertEqual(eth0_network.hwaddr, vlan0100.mac_address)
         self.assertTrue(vlan0100.enabled)
+        self.assertTrue(vlan0100.link_connected)
         self.assertEqual([eth0], list(vlan0100.parents.all()))
         vlan101 = Interface.objects.get(name="vlan101", node=controller)
         self.assertEqual(INTERFACE_TYPE.VLAN, vlan101.type)
@@ -543,6 +547,7 @@ class TestUpdateInterfaces(MAASServerTestCase, UpdateInterfacesMixin):
         # have different MAC addresses.
         self.assertEqual(eth0_network.hwaddr, vlan101.mac_address)
         self.assertTrue(vlan101.enabled)
+        self.assertTrue(vlan101.link_connected)
         self.assertEqual([eth0], list(vlan101.parents.all()))
         eth0_0102 = Interface.objects.get(name="eth0.0102", node=controller)
         self.assertEqual(INTERFACE_TYPE.VLAN, eth0_0102.type)
@@ -551,6 +556,7 @@ class TestUpdateInterfaces(MAASServerTestCase, UpdateInterfacesMixin):
         # have different MAC addresses.
         self.assertEqual(eth0_network.hwaddr, eth0_0102.mac_address)
         self.assertTrue(eth0_0102.enabled)
+        self.assertTrue(eth0_0102.link_connected)
         self.assertEqual([eth0], list(eth0_0102.parents.all()))
 
     def test_vlans_with_alternate_naming_conventions_vm_host(self):