MAAS

Merge ~cgrabowski/maas:do_not_create_duplicate_vlan_on_controller_report_neighbors into maas:master

  1. Git
  2. lp:~cgrabowski/maas
  3. do_not_create_duplicate_vlan_on_controller_report_neighbors
  4. Merge into master
Proposed by Christian Grabowski
Status: Merged
Approved by: Christian Grabowski
Approved revision: 7a7ce709f9baa2d6f480e9a6d56418a21e3c11f4
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~cgrabowski/maas:do_not_create_duplicate_vlan_on_controller_report_neighbors
Merge into: maas:master
Diff against target: 145 lines (+67/-5)
4 files modified
src/maasserver/models/interface.py (+16/-1)
src/maasserver/models/node.py (+1/-1)
src/maasserver/models/tests/test_interface.py (+17/-2)
src/maasserver/models/tests/test_node.py (+33/-1)
Reviewer Review Type Date Requested Status
MAAS Lander Approve
Alexsander de Souza Approve
Review via email: mp+423776@code.launchpad.net

Commit message

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

To post a comment you must log in.
Revision history for this message
MAAS Lander (maas-lander) wrote :

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
Revision history for this message
Alexsander de Souza (alexsander-souza) wrote :

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
~cgrabowski/maas:do_not_create_duplicate_vlan_on_controller_report_neighbors updated
aa67806... by Christian Grabowski

pass IP to test_create_vlan_if_necessary report_vid calls

Revision history for this message
Christian Grabowski (cgrabowski) wrote :

> 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.

Revision history for this message
MAAS Lander (maas-lander) wrote :

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
~cgrabowski/maas:do_not_create_duplicate_vlan_on_controller_report_neighbors updated
9e81695... by Christian Grabowski

fix formatting

Revision history for this message
Alexsander de Souza (alexsander-souza) wrote :

+1

review: Approve
~cgrabowski/maas:do_not_create_duplicate_vlan_on_controller_report_neighbors updated
7a7ce70... by Christian Grabowski

remove unused var

Revision history for this message
MAAS Lander (maas-lander) wrote :

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
Revision history for this message
Christian Grabowski (cgrabowski) wrote :

jenkins: !test

Revision history for this message
MAAS Lander (maas-lander) wrote :

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

Update scan failed

At least one of the branches involved have failed to scan. You can manually schedule a rescan if required.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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):
1533 tags.remove(tag)1533 tags.remove(tag)
1534 self.tags = tags1534 self.tags = tags
15351535
1536 def report_vid(self, vid):1536 def report_vid(self, vid, ip=None):
1537 """Report that the specified VID was seen on this interface.1537 """Report that the specified VID was seen on this interface.
15381538
1539 Automatically creates the related VLAN on this interface's associated1539 Automatically creates the related VLAN on this interface's associated
@@ -1542,8 +1542,23 @@ class Interface(CleanSave, TimestampedModel):
1542 if self.vlan is not None and vid is not None:1542 if self.vlan is not None and vid is not None:
1543 fabric = self.vlan.fabric1543 fabric = self.vlan.fabric
1544 # Circular imports1544 # Circular imports
1545 from maasserver.models.subnet import Subnet
1545 from maasserver.models.vlan import VLAN1546 from maasserver.models.vlan import VLAN
15461547
1548 if ip:
1549 subnet = Subnet.objects.get_best_subnet_for_ip(ip)
1550 if subnet:
1551 vlan = subnet.vlan
1552 vlan.fabric = fabric
1553 vlan.save()
1554 maaslog.info(
1555 "%s: Automatically updated VLAN %d (observed on %s).",
1556 self.get_log_string(),
1557 vid,
1558 vlan.fabric.get_name(),
1559 )
1560 return
1561
1547 vlan, created = VLAN.objects.get_or_create(1562 vlan, created = VLAN.objects.get_or_create(
1548 fabric=fabric,1563 fabric=fabric,
1549 vid=vid,1564 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):
6416 vid=vid,6416 vid=vid,
6417 )6417 )
6418 if vid is not None:6418 if vid is not None:
6419 interface.report_vid(vid)6419 interface.report_vid(vid, ip=neighbour["ip"])
64206420
6421 def report_mdns_entries(self, entries):6421 def report_mdns_entries(self, entries):
6422 """Update the mDNS entries on this controller.6422 """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):
4049 vlan = fabric.get_default_vlan()4049 vlan = fabric.get_default_vlan()
4050 iface = factory.make_Interface(vlan=vlan)4050 iface = factory.make_Interface(vlan=vlan)
4051 vid = random.randint(1, 4094)4051 vid = random.randint(1, 4094)
4052 ip = factory.make_ip_address()
4052 vlan_before = get_one(VLAN.objects.filter(fabric=fabric, vid=vid))4053 vlan_before = get_one(VLAN.objects.filter(fabric=fabric, vid=vid))
4053 self.assertIsNone(vlan_before)4054 self.assertIsNone(vlan_before)
4054 iface.report_vid(vid)4055 iface.report_vid(vid, ip=ip)
4055 vlan_after = get_one(VLAN.objects.filter(fabric=fabric, vid=vid))4056 vlan_after = get_one(VLAN.objects.filter(fabric=fabric, vid=vid))
4056 self.assertIsNotNone(vlan_after)4057 self.assertIsNotNone(vlan_after)
4057 # Report it one more time to make sure we can handle it if we already4058 # Report it one more time to make sure we can handle it if we already
4058 # observed it. (expect nothing to happen.)4059 # observed it. (expect nothing to happen.)
4059 iface.report_vid(vid)4060 iface.report_vid(vid, ip=ip)
40604061
4061 def test_logs_vlan_creation_and_sets_description(self):4062 def test_logs_vlan_creation_and_sets_description(self):
4062 fabric = factory.make_Fabric()4063 fabric = factory.make_Fabric()
@@ -4075,6 +4076,20 @@ class TestReportVID(MAASServerTestCase):
4075 new_vlan.description,4076 new_vlan.description,
4076 )4077 )
40774078
4079 def test_report_vid_handles_existing_vlan(self):
4080 fabric1 = factory.make_Fabric()
4081 fabric2 = factory.make_Fabric()
4082 observing_vlan = fabric1.get_default_vlan()
4083 neighbour_vlan = factory.make_VLAN(fabric=fabric2)
4084 subnet1 = factory.make_Subnet(vlan=observing_vlan)
4085 subnet2 = factory.make_Subnet(vlan=neighbour_vlan)
4086 iface = factory.make_Interface(subnet=subnet1)
4087 iface.report_vid(
4088 neighbour_vlan.vid, ip=subnet2.get_next_ip_for_allocation()
4089 )
4090 neighbour_vlan.refresh_from_db()
4091 self.assertEqual(observing_vlan.fabric, neighbour_vlan.fabric)
4092
40784093
4079class TestInterfaceGetDefaultBridgeName(MAASServerTestCase):4094class TestInterfaceGetDefaultBridgeName(MAASServerTestCase):
40804095
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 (
103 StaticIPAddress,103 StaticIPAddress,
104 Subnet,104 Subnet,
105 UnknownInterface,105 UnknownInterface,
106 VLAN,
106 VolumeGroup,107 VolumeGroup,
107)108)
108from maasserver.models import Bcache, BMC109from maasserver.models import Bcache, BMC
@@ -10913,7 +10914,38 @@ class TestReportNeighbours(MAASServerTestCase):
10913 },10914 },
10914 ]10915 ]
10915 rack.report_neighbours(neighbours)10916 rack.report_neighbours(neighbours)
10916 report_vid.assert_has_calls([call(3), call(7)])10917 report_vid.assert_has_calls(
10918 [call(3, ip=neighbours[0]["ip"]), call(7, ip=neighbours[1]["ip"])]
10919 )
10920
10921 def test_updates_fabric_of_existing_vlan(self):
10922 rack = factory.make_RackController()
10923 observing_fabric = factory.make_Fabric()
10924 other_fabric = factory.make_Fabric()
10925 vlan = factory.make_VLAN(fabric=observing_fabric)
10926 observed_vlan = factory.make_VLAN(fabric=other_fabric)
10927 subnet1 = factory.make_Subnet(vlan=vlan)
10928 subnet2 = factory.make_Subnet(vlan=observed_vlan)
10929 factory.make_Interface(name="eth0", subnet=subnet1, node=rack)
10930 iface2 = factory.make_Interface(name="eth1", node=rack)
10931 neighbours = [
10932 {
10933 "interface": "eth0",
10934 "ip": subnet2.get_next_ip_for_allocation(),
10935 "time": datetime.now(),
10936 "mac": iface2.mac_address,
10937 "vid": observed_vlan.vid,
10938 }
10939 ]
10940 rack.report_neighbours(neighbours)
10941 observed_vlan.refresh_from_db()
10942 self.assertEqual(observing_fabric, observed_vlan.fabric)
10943 self.assertEqual(
10944 VLAN.objects.filter(
10945 vid=observed_vlan.vid, fabric=observing_fabric
10946 ).count(),
10947 1,
10948 )
1091710949
1091810950
10919class TestReportMDNSEntries(MAASServerTestCase):10951class TestReportMDNSEntries(MAASServerTestCase):

Subscribers

People subscribed via source and target branches