Merge ~mpontillo/maas:fix-ha-rack-dhcp-fabric-creation--bug-1705654 into maas:master

Proposed by Mike Pontillo on 2017-07-21
Status: Merged
Approved by: Mike Pontillo on 2017-07-26
Approved revision: 6a30b4892814133268b596a845f923bf22d2b283
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~mpontillo/maas:fix-ha-rack-dhcp-fabric-creation--bug-1705654
Merge into: maas:master
Diff against target: 156 lines (+74/-7)
4 files modified
src/maasserver/models/node.py (+12/-6)
src/maasserver/models/signals/interfaces.py (+2/-0)
src/maasserver/models/signals/tests/test_interfaces.py (+13/-1)
src/maasserver/models/tests/test_node.py (+47/-0)
Reviewer Review Type Date Requested Status
Blake Rouse 2017-07-21 Approve on 2017-07-21
Review via email: mp+327859@code.launchpad.net

Commit message

Fix fabric creation for rack DHCP IP addresses.

LP: #1705654

To post a comment you must log in.
Blake Rouse (blake-rouse) wrote :

Looks good.

review: Approve
MAAS Lander (maas-lander) wrote :

LANDING
-b fix-ha-rack-dhcp-fabric-creation--bug-1705654 lp:~mpontillo/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/126/console

MAAS Lander (maas-lander) wrote :

LANDING
-b fix-ha-rack-dhcp-fabric-creation--bug-1705654 lp:~mpontillo/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/127/console

MAAS Lander (maas-lander) wrote :

LANDING
-b fix-ha-rack-dhcp-fabric-creation--bug-1705654 lp:~mpontillo/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/128/console

MAAS Lander (maas-lander) wrote :

LANDING
-b fix-ha-rack-dhcp-fabric-creation--bug-1705654 lp:~mpontillo/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/137/console

MAAS Lander (maas-lander) wrote :

LANDING
-b fix-ha-rack-dhcp-fabric-creation--bug-1705654 lp:~mpontillo/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/138/console

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
1diff --git a/src/maasserver/models/node.py b/src/maasserver/models/node.py
2index d27a111..7ff13c5 100644
3--- a/src/maasserver/models/node.py
4+++ b/src/maasserver/models/node.py
5@@ -4184,7 +4184,7 @@ class Controller(Node):
6 def _update_physical_links(self, interface, config, new_vlan,
7 update_fields):
8 update_ip_addresses = self._update_links(interface, config["links"])
9- linked_vlan = self._get_first_sticky_vlan_from_ip_addresses(
10+ linked_vlan = self._guess_best_vlan_from_ip_addresses(
11 update_ip_addresses)
12 if linked_vlan is not None:
13 interface.vlan = linked_vlan
14@@ -4381,7 +4381,7 @@ class Controller(Node):
15 If a static IP address is allocated, give preferential treatment to
16 the VLAN that IP address resides on.
17 """
18- linked_vlan = self._get_first_sticky_vlan_from_ip_addresses(
19+ linked_vlan = self._guess_best_vlan_from_ip_addresses(
20 update_ip_addresses)
21 if linked_vlan is not None:
22 interface.vlan = linked_vlan
23@@ -4417,8 +4417,11 @@ class Controller(Node):
24 """Return a set of subnets that `links` belongs to."""
25 subnets = set()
26 for link in links:
27- if link["mode"] == "static":
28- ip_addr = IPNetwork(link["address"])
29+ if link["mode"] == "static" or link["mode"] == "dhcp":
30+ address = link.get("address", None)
31+ if address is None:
32+ continue
33+ ip_addr = IPNetwork(address)
34 subnet = get_one(Subnet.objects.filter(cidr=str(ip_addr.cidr)))
35 if subnet is not None:
36 subnets.add(subnet)
37@@ -4439,12 +4442,15 @@ class Controller(Node):
38 return ip_address
39 return None
40
41- def _get_first_sticky_vlan_from_ip_addresses(self, ip_addresses):
42+ def _guess_best_vlan_from_ip_addresses(self, ip_addresses):
43 """Return the first VLAN for a STICKY IP address in `ip_addresses`."""
44+ second_best = None
45 for ip_address in ip_addresses:
46 if ip_address.alloc_type == IPADDRESS_TYPE.STICKY:
47 return ip_address.subnet.vlan
48- return None
49+ elif ip_address.alloc_type == IPADDRESS_TYPE.DISCOVERED:
50+ second_best = ip_address.subnet.vlan
51+ return second_best
52
53 def _update_links(
54 self, interface, links, force_vlan=False, use_interface_vlan=True):
55diff --git a/src/maasserver/models/signals/interfaces.py b/src/maasserver/models/signals/interfaces.py
56index 5c6e75a..baac5eb 100644
57--- a/src/maasserver/models/signals/interfaces.py
58+++ b/src/maasserver/models/signals/interfaces.py
59@@ -214,6 +214,8 @@ def interface_vlan_update(instance, old_values, **kwargs):
60 NODE_TYPE.REGION_CONTROLLER,
61 NODE_TYPE.RACK_CONTROLLER,
62 NODE_TYPE.REGION_AND_RACK_CONTROLLER):
63+ if old_vlan_id is None:
64+ return
65 # Interface VLAN was changed on a controller. Move all linked subnets
66 # to that new VLAN, unless the new VLAN is None. When the VLAN is
67 # None then the administrator is say that the interface is now
68diff --git a/src/maasserver/models/signals/tests/test_interfaces.py b/src/maasserver/models/signals/tests/test_interfaces.py
69index 6af1fa9..d15235b 100644
70--- a/src/maasserver/models/signals/tests/test_interfaces.py
71+++ b/src/maasserver/models/signals/tests/test_interfaces.py
72@@ -240,7 +240,7 @@ class TestInterfaceVLANUpdateController(MAASServerTestCase):
73 interface.save()
74 self.assertEquals(new_vlan, reload_object(subnet).vlan)
75
76- def test__doesnt_move_link_subnets_when_vlan_is_None(self):
77+ def test__doesnt_move_link_subnets_when_target_vlan_is_None(self):
78 node = self.maker()
79 interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=node)
80 old_vlan = interface.vlan
81@@ -251,6 +251,18 @@ class TestInterfaceVLANUpdateController(MAASServerTestCase):
82 interface.save()
83 self.assertEquals(old_vlan, reload_object(subnet).vlan)
84
85+ def test__doesnt_move_link_subnets_when_source_vlan_is_None(self):
86+ node = self.maker()
87+ interface = factory.make_Interface(
88+ INTERFACE_TYPE.PHYSICAL, node=node, vlan=None)
89+ old_vlan = interface.vlan
90+ subnet = factory.make_Subnet(vlan=old_vlan)
91+ factory.make_StaticIPAddress(
92+ subnet=subnet, interface=interface)
93+ interface.vlan = factory.make_VLAN()
94+ interface.save()
95+ self.assertEquals(interface.vlan, reload_object(subnet).vlan)
96+
97 def test__moves_children_vlans_to_same_fabric(self):
98 node = self.maker()
99 parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=node)
100diff --git a/src/maasserver/models/tests/test_node.py b/src/maasserver/models/tests/test_node.py
101index 88ad200..42fa230 100644
102--- a/src/maasserver/models/tests/test_node.py
103+++ b/src/maasserver/models/tests/test_node.py
104@@ -9241,6 +9241,53 @@ class TestUpdateInterfaces(MAASServerTestCase):
105 self.assertIsNotNone(eth0.vlan)
106 self.assertIsNone(eth1.vlan)
107
108+ def test_subnet_seen_on_second_controller_does_not_create_fabric(self):
109+ alice = self.create_empty_controller()
110+ bob = self.create_empty_controller()
111+ alice_interfaces = {
112+ 'eth0': {
113+ 'enabled': True,
114+ 'links': [{'address': '192.168.0.1/24', 'mode': 'dhcp'}],
115+ 'mac_address': '52:54:00:77:15:e3',
116+ 'parents': [],
117+ 'source': 'ipaddr',
118+ 'type': 'physical'
119+ },
120+ 'eth1': {
121+ 'enabled': False,
122+ 'links': [],
123+ 'mac_address': '52:54:00:77:15:e4',
124+ 'parents': [],
125+ 'source': 'ipaddr',
126+ 'type': 'physical'
127+ },
128+ }
129+ bob_interfaces = {
130+ 'eth0': {
131+ 'enabled': True,
132+ 'links': [{'address': '192.168.0.2/24', 'mode': 'dhcp'}],
133+ 'mac_address': '52:54:00:87:25:f3',
134+ 'parents': [],
135+ 'source': 'ipaddr',
136+ 'type': 'physical'
137+ },
138+ 'eth1': {
139+ 'enabled': False,
140+ 'links': [],
141+ 'mac_address': '52:54:00:87:25:f4',
142+ 'parents': [],
143+ 'source': 'ipaddr',
144+ 'type': 'physical'
145+ },
146+ }
147+ self.update_interfaces(alice, alice_interfaces)
148+ self.update_interfaces(bob, bob_interfaces)
149+ alice_eth0 = get_one(
150+ PhysicalInterface.objects.filter(node=alice, name='eth0'))
151+ bob_eth0 = get_one(
152+ PhysicalInterface.objects.filter(node=bob, name='eth0'))
153+ self.assertThat(alice_eth0.vlan, Equals(bob_eth0.vlan))
154+
155
156 class TestRackControllerRefresh(MAASTransactionServerTestCase):
157

Subscribers

People subscribed via source and target branches

to all changes: