Merge ~cgrabowski/maas:fix_vlan_discovery into maas:master

[Christian Grabowski]

update report_neighbours test

ignore observed vlan if already exists

This addresses LP:1978981 by instead of updating the existing VLAN's fabric, it ignores the report. This does change the fix for LP:1975477 in that its original fix would identify the existing VLAN by finding a matching subnet if possible, and updating its fabric to the observed one, avoiding the collision prior. So this now both avoids the collision issue and prevents the vlan from being moved unexpectedly.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b fix_vlan_discovery 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/12987/console
COMMIT: 50f817870f78b6fadfe6f7c0e371d92ef7a93d0c

review: Needs Fixing

[Alberto Donato (ack)]

LGTM +1

review: Approve

[Adam Collard (adam-collard)]

jenkins: !test

diff --git a/src/maasserver/models/interface.py b/src/maasserver/models/interface.py
index e61e672..e58701f 100644
--- a/src/maasserver/models/interface.py
+++ b/src/maasserver/models/interface.py
@@ -1548,16 +1548,8 @@ class Interface(CleanSave, TimestampedModel):
 
             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(),
-                    )
+                # VLAN already exists, don't update it
+                if subnet and subnet.vlan.vid == vid:
                     return
 
             vlan, created = VLAN.objects.get_or_create(
diff --git a/src/maasserver/models/tests/test_interface.py b/src/maasserver/models/tests/test_interface.py
index 8356f5f..53f37f3 100644
--- a/src/maasserver/models/tests/test_interface.py
+++ b/src/maasserver/models/tests/test_interface.py
@@ -4105,7 +4105,7 @@ class TestReportVID(MAASServerTestCase):
             new_vlan.description,
         )
 
-    def test_report_vid_handles_existing_vlan(self):
+    def test_report_vid_does_not_modify_existing_vlan(self):
         fabric1 = factory.make_Fabric()
         fabric2 = factory.make_Fabric()
         observing_vlan = fabric1.get_default_vlan()
@@ -4117,7 +4117,7 @@ class TestReportVID(MAASServerTestCase):
             neighbour_vlan.vid, ip=subnet2.get_next_ip_for_allocation()
         )
         neighbour_vlan.refresh_from_db()
-        self.assertEqual(observing_vlan.fabric, neighbour_vlan.fabric)
+        self.assertEqual(observing_vlan.fabric, fabric1)
 
 
 class TestInterfaceGetDefaultBridgeName(MAASServerTestCase):
diff --git a/src/maasserver/models/tests/test_node.py b/src/maasserver/models/tests/test_node.py
index 50042f2..4a607d8 100644
--- a/src/maasserver/models/tests/test_node.py
+++ b/src/maasserver/models/tests/test_node.py
@@ -10957,7 +10957,7 @@ class TestReportNeighbours(MAASServerTestCase):
             [call(3, ip=neighbours[0]["ip"]), call(7, ip=neighbours[1]["ip"])]
         )
 
-    def test_updates_fabric_of_existing_vlan(self):
+    def test_does_not_updates_fabric_of_existing_vlan(self):
         rack = factory.make_RackController()
         observing_fabric = factory.make_Fabric()
         other_fabric = factory.make_Fabric()
@@ -10978,10 +10978,10 @@ class TestReportNeighbours(MAASServerTestCase):
         ]
         rack.report_neighbours(neighbours)
         observed_vlan.refresh_from_db()
-        self.assertEqual(observing_fabric, observed_vlan.fabric)
+        self.assertEqual(other_fabric, observed_vlan.fabric)
         self.assertEqual(
             VLAN.objects.filter(
-                vid=observed_vlan.vid, fabric=observing_fabric
+                vid=observed_vlan.vid, fabric=other_fabric
             ).count(),
             1,
         )