Merge ~cgrabowski/maas:do_not_create_duplicate_vlan_on_controller_report_neighbors into maas:master

[Christian Grabowski]

update vlan's fabric when observed neighbor's vlan already exists

[MAAS Lander (maas-lander)]

UNIT TESTS
-b do_not_create_duplicate_vlan_on_controller_report_neighbors lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/12769/console
COMMIT: 71e12ebe19fc7826d66919fd4f2876ea6af6e9de

review: Needs Fixing

[Alexsander de Souza (alexsander-souza)]

I think the other testcases in TestReportVID must be updated also, because it looks like `neighbour["ip"]` won't be None usually. If you do this change, I think test_creates_vlan_if_necessary() will fail

review: Needs Information

aa67806... by Christian Grabowski on 2022-06-01

pass IP to test_create_vlan_if_necessary report_vid calls

[Christian Grabowski (cgrabowski)]

> I think the other testcases in TestReportVID must be updated also, because it
> looks like `neighbour["ip"]` won't be None usually. If you do this change, I
> think test_creates_vlan_if_necessary() will fail

Ah good point, updated.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b do_not_create_duplicate_vlan_on_controller_report_neighbors lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/12772/console
COMMIT: aa67806ec1598b8474ba1ec898a44598e0922fe4

review: Needs Fixing

9e81695... by Christian Grabowski on 2022-06-01

fix formatting

[Alexsander de Souza (alexsander-souza)]

+1

review: Approve

7a7ce70... by Christian Grabowski on 2022-06-01

remove unused var

[MAAS Lander (maas-lander)]

UNIT TESTS
-b do_not_create_duplicate_vlan_on_controller_report_neighbors lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/12774/console
COMMIT: 9e81695f9b3afb5a67cb0760862d6f8d5fd011dd

review: Needs Fixing

[Christian Grabowski (cgrabowski)]

jenkins: !test

[MAAS Lander (maas-lander)]

UNIT TESTS
-b do_not_create_duplicate_vlan_on_controller_report_neighbors lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 7a7ce709f9baa2d6f480e9a6d56418a21e3c11f4

review: Approve

diff --git a/src/maasserver/models/interface.py b/src/maasserver/models/interface.py
index 9392df7..7945291 100644
--- a/src/maasserver/models/interface.py
+++ b/src/maasserver/models/interface.py
@@ -1533,7 +1533,7 @@ class Interface(CleanSave, TimestampedModel):
             tags.remove(tag)
             self.tags = tags
 
-    def report_vid(self, vid):
+    def report_vid(self, vid, ip=None):
         """Report that the specified VID was seen on this interface.
 
         Automatically creates the related VLAN on this interface's associated
@@ -1542,8 +1542,23 @@ class Interface(CleanSave, TimestampedModel):
         if self.vlan is not None and vid is not None:
             fabric = self.vlan.fabric
             # Circular imports
+            from maasserver.models.subnet import Subnet
             from maasserver.models.vlan import VLAN
 
+            if ip:
+                subnet = Subnet.objects.get_best_subnet_for_ip(ip)
+                if subnet:
+                    vlan = subnet.vlan
+                    vlan.fabric = fabric
+                    vlan.save()
+                    maaslog.info(
+                        "%s: Automatically updated VLAN %d (observed on %s).",
+                        self.get_log_string(),
+                        vid,
+                        vlan.fabric.get_name(),
+                    )
+                    return
+
             vlan, created = VLAN.objects.get_or_create(
                 fabric=fabric,
                 vid=vid,
diff --git a/src/maasserver/models/node.py b/src/maasserver/models/node.py
index 385c2d9..2a01c4b 100644
--- a/src/maasserver/models/node.py
+++ b/src/maasserver/models/node.py
@@ -6416,7 +6416,7 @@ class Controller(Node):
                         vid=vid,
                     )
                 if vid is not None:
-                    interface.report_vid(vid)
+                    interface.report_vid(vid, ip=neighbour["ip"])
 
     def report_mdns_entries(self, entries):
         """Update the mDNS entries on this controller.
diff --git a/src/maasserver/models/tests/test_interface.py b/src/maasserver/models/tests/test_interface.py
index ca9e1eb..fcfdc85 100644
--- a/src/maasserver/models/tests/test_interface.py
+++ b/src/maasserver/models/tests/test_interface.py
@@ -4049,14 +4049,15 @@ class TestReportVID(MAASServerTestCase):
         vlan = fabric.get_default_vlan()
         iface = factory.make_Interface(vlan=vlan)
         vid = random.randint(1, 4094)
+        ip = factory.make_ip_address()
         vlan_before = get_one(VLAN.objects.filter(fabric=fabric, vid=vid))
         self.assertIsNone(vlan_before)
-        iface.report_vid(vid)
+        iface.report_vid(vid, ip=ip)
         vlan_after = get_one(VLAN.objects.filter(fabric=fabric, vid=vid))
         self.assertIsNotNone(vlan_after)
         # Report it one more time to make sure we can handle it if we already
         # observed it. (expect nothing to happen.)
-        iface.report_vid(vid)
+        iface.report_vid(vid, ip=ip)
 
     def test_logs_vlan_creation_and_sets_description(self):
         fabric = factory.make_Fabric()
@@ -4075,6 +4076,20 @@ class TestReportVID(MAASServerTestCase):
             new_vlan.description,
         )
 
+    def test_report_vid_handles_existing_vlan(self):
+        fabric1 = factory.make_Fabric()
+        fabric2 = factory.make_Fabric()
+        observing_vlan = fabric1.get_default_vlan()
+        neighbour_vlan = factory.make_VLAN(fabric=fabric2)
+        subnet1 = factory.make_Subnet(vlan=observing_vlan)
+        subnet2 = factory.make_Subnet(vlan=neighbour_vlan)
+        iface = factory.make_Interface(subnet=subnet1)
+        iface.report_vid(
+            neighbour_vlan.vid, ip=subnet2.get_next_ip_for_allocation()
+        )
+        neighbour_vlan.refresh_from_db()
+        self.assertEqual(observing_vlan.fabric, neighbour_vlan.fabric)
+
 
 class TestInterfaceGetDefaultBridgeName(MAASServerTestCase):
 
diff --git a/src/maasserver/models/tests/test_node.py b/src/maasserver/models/tests/test_node.py
index 33752b8..7a468c0 100644
--- a/src/maasserver/models/tests/test_node.py
+++ b/src/maasserver/models/tests/test_node.py
@@ -103,6 +103,7 @@ from maasserver.models import (
     StaticIPAddress,
     Subnet,
     UnknownInterface,
+    VLAN,
     VolumeGroup,
 )
 from maasserver.models import Bcache, BMC
@@ -10913,7 +10914,38 @@ class TestReportNeighbours(MAASServerTestCase):
             },
         ]
         rack.report_neighbours(neighbours)
-        report_vid.assert_has_calls([call(3), call(7)])
+        report_vid.assert_has_calls(
+            [call(3, ip=neighbours[0]["ip"]), call(7, ip=neighbours[1]["ip"])]
+        )
+
+    def test_updates_fabric_of_existing_vlan(self):
+        rack = factory.make_RackController()
+        observing_fabric = factory.make_Fabric()
+        other_fabric = factory.make_Fabric()
+        vlan = factory.make_VLAN(fabric=observing_fabric)
+        observed_vlan = factory.make_VLAN(fabric=other_fabric)
+        subnet1 = factory.make_Subnet(vlan=vlan)
+        subnet2 = factory.make_Subnet(vlan=observed_vlan)
+        factory.make_Interface(name="eth0", subnet=subnet1, node=rack)
+        iface2 = factory.make_Interface(name="eth1", node=rack)
+        neighbours = [
+            {
+                "interface": "eth0",
+                "ip": subnet2.get_next_ip_for_allocation(),
+                "time": datetime.now(),
+                "mac": iface2.mac_address,
+                "vid": observed_vlan.vid,
+            }
+        ]
+        rack.report_neighbours(neighbours)
+        observed_vlan.refresh_from_db()
+        self.assertEqual(observing_fabric, observed_vlan.fabric)
+        self.assertEqual(
+            VLAN.objects.filter(
+                vid=observed_vlan.vid, fabric=observing_fabric
+            ).count(),
+            1,
+        )
 
 
 class TestReportMDNSEntries(MAASServerTestCase):