Merge ~r00ta/maas:lp-2027621 into maas:master
- Git
- lp:~r00ta/maas
- lp-2027621
- Merge into master
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Jacopo Rota | ||||
Approved revision: | 983c55ba1a9b58c2f7a5cb6b0838eb0db27eddd8 | ||||
Merge reported by: | MAAS Lander | ||||
Merged at revision: | not available | ||||
Proposed branch: | ~r00ta/maas:lp-2027621 | ||||
Merge into: | maas:master | ||||
Diff against target: |
286 lines (+123/-21) 2 files modified
src/maasserver/dhcp.py (+15/-9) src/maasserver/tests/test_dhcp.py (+108/-12) |
||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
MAAS Lander | Approve | ||
Anton Troyanov | Approve | ||
Review via email: mp+446748@code.launchpad.net |
Commit message
fix dhcp make_failover_
Description of the change
This MP aims to fix https:/
The function make_failover_
The fix is
1) pass the `ip_version` to this function
2) select the interfaces of the rack controller that are on the vlan and that match the `ip_version`
3) select the best interface according to a specific logic already implemented
4) get an ip from that interface that is on the vlan and that matches the `ip_version`
A test to verify this scenario has been added.
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b lp-2027621 lp:~r00ta/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: FAILED
LOG: http://
COMMIT: 04242f723144301
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b lp-2027621 lp:~r00ta/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: SUCCESS
COMMIT: 983c55ba1a9b58c
MAAS Lander (maas-lander) wrote : | # |
LANDING
-b lp-2027621 lp:~r00ta/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: FAILED BUILD
LOG: http://
Preview Diff
1 | diff --git a/src/maasserver/dhcp.py b/src/maasserver/dhcp.py | |||
2 | index 32ad98a..8f51cbf 100644 | |||
3 | --- a/src/maasserver/dhcp.py | |||
4 | +++ b/src/maasserver/dhcp.py | |||
5 | @@ -227,15 +227,17 @@ def ip_is_on_vlan(ip_address, vlan): | |||
6 | 227 | ) | 227 | ) |
7 | 228 | 228 | ||
8 | 229 | 229 | ||
10 | 230 | def get_ip_address_for_interface(interface, vlan): | 230 | def get_ip_address_for_interface(interface, vlan, ip_version: int): |
11 | 231 | """Return the IP address for `interface` on `vlan`.""" | 231 | """Return the IP address for `interface` on `vlan`.""" |
12 | 232 | for ip_address in interface.ip_addresses.all(): | 232 | for ip_address in interface.ip_addresses.all(): |
14 | 233 | if ip_is_on_vlan(ip_address, vlan): | 233 | if ip_is_version(ip_address, ip_version) and ip_is_on_vlan( |
15 | 234 | ip_address, vlan | ||
16 | 235 | ): | ||
17 | 234 | return ip_address | 236 | return ip_address |
18 | 235 | return None | 237 | return None |
19 | 236 | 238 | ||
20 | 237 | 239 | ||
22 | 238 | def get_ip_address_for_rack_controller(rack_controller, vlan): | 240 | def get_ip_address_for_rack_controller(rack_controller, vlan, ip_version: int): |
23 | 239 | """Return the IP address for `rack_controller` on `vlan`.""" | 241 | """Return the IP address for `rack_controller` on `vlan`.""" |
24 | 240 | # First we build a list of all interfaces that have an IP address | 242 | # First we build a list of all interfaces that have an IP address |
25 | 241 | # on that vlan. Then we pick the best interface for that vlan | 243 | # on that vlan. Then we pick the best interface for that vlan |
26 | @@ -248,10 +250,12 @@ def get_ip_address_for_rack_controller(rack_controller, vlan): | |||
27 | 248 | matching_interfaces = set() | 250 | matching_interfaces = set() |
28 | 249 | for interface in interfaces: | 251 | for interface in interfaces: |
29 | 250 | for ip_address in interface.ip_addresses.all(): | 252 | for ip_address in interface.ip_addresses.all(): |
31 | 251 | if ip_is_on_vlan(ip_address, vlan): | 253 | if ip_is_version(ip_address, ip_version) and ip_is_on_vlan( |
32 | 254 | ip_address, vlan | ||
33 | 255 | ): | ||
34 | 252 | matching_interfaces.add(interface) | 256 | matching_interfaces.add(interface) |
35 | 253 | interface = get_best_interface(matching_interfaces) | 257 | interface = get_best_interface(matching_interfaces) |
37 | 254 | return get_ip_address_for_interface(interface, vlan) | 258 | return get_ip_address_for_interface(interface, vlan, ip_version) |
38 | 255 | 259 | ||
39 | 256 | 260 | ||
40 | 257 | def get_ntp_server_addresses_for_rack(rack: RackController) -> dict: | 261 | def get_ntp_server_addresses_for_rack(rack: RackController) -> dict: |
41 | @@ -498,17 +502,19 @@ def make_subnet_config( | |||
42 | 498 | return subnet_config | 502 | return subnet_config |
43 | 499 | 503 | ||
44 | 500 | 504 | ||
46 | 501 | def make_failover_peer_config(vlan, rack_controller): | 505 | def make_failover_peer_config(vlan, rack_controller, ip_version: int): |
47 | 502 | """Return DHCP failover peer configuration dict for a rack controller.""" | 506 | """Return DHCP failover peer configuration dict for a rack controller.""" |
48 | 503 | is_primary = vlan.primary_rack_id == rack_controller.id | 507 | is_primary = vlan.primary_rack_id == rack_controller.id |
49 | 504 | interface_ip_address = get_ip_address_for_rack_controller( | 508 | interface_ip_address = get_ip_address_for_rack_controller( |
51 | 505 | rack_controller, vlan | 509 | rack_controller, vlan, ip_version |
52 | 506 | ) | 510 | ) |
53 | 507 | if is_primary: | 511 | if is_primary: |
54 | 508 | peer_rack = vlan.secondary_rack | 512 | peer_rack = vlan.secondary_rack |
55 | 509 | else: | 513 | else: |
56 | 510 | peer_rack = vlan.primary_rack | 514 | peer_rack = vlan.primary_rack |
58 | 511 | peer_address = get_ip_address_for_rack_controller(peer_rack, vlan) | 515 | peer_address = get_ip_address_for_rack_controller( |
59 | 516 | peer_rack, vlan, ip_version | ||
60 | 517 | ) | ||
61 | 512 | name = "failover-vlan-%d" % vlan.id | 518 | name = "failover-vlan-%d" % vlan.id |
62 | 513 | return ( | 519 | return ( |
63 | 514 | name, | 520 | name, |
64 | @@ -631,7 +637,7 @@ def get_dhcp_configure_for( | |||
65 | 631 | if has_secondary: | 637 | if has_secondary: |
66 | 632 | # Generate the failover peer for this VLAN. | 638 | # Generate the failover peer for this VLAN. |
67 | 633 | peer_name, peer_config, peer_rack = make_failover_peer_config( | 639 | peer_name, peer_config, peer_rack = make_failover_peer_config( |
69 | 634 | vlan, rack_controller | 640 | vlan, rack_controller, ip_version |
70 | 635 | ) | 641 | ) |
71 | 636 | else: | 642 | else: |
72 | 637 | peer_name, peer_config, peer_rack = None, None, None | 643 | peer_name, peer_config, peer_rack = None, None, None |
73 | diff --git a/src/maasserver/tests/test_dhcp.py b/src/maasserver/tests/test_dhcp.py | |||
74 | index e9d0cfa..04d15dc 100644 | |||
75 | --- a/src/maasserver/tests/test_dhcp.py | |||
76 | +++ b/src/maasserver/tests/test_dhcp.py | |||
77 | @@ -815,10 +815,16 @@ class TestGetIPAddressForInterface(MAASServerTestCase): | |||
78 | 815 | interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, vlan=vlan) | 815 | interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, vlan=vlan) |
79 | 816 | subnet = factory.make_Subnet(vlan=vlan) | 816 | subnet = factory.make_Subnet(vlan=vlan) |
80 | 817 | ip_address = factory.make_StaticIPAddress( | 817 | ip_address = factory.make_StaticIPAddress( |
82 | 818 | alloc_type=IPADDRESS_TYPE.AUTO, subnet=subnet, interface=interface | 818 | ip=factory.pick_ip_in_Subnet(subnet), |
83 | 819 | alloc_type=IPADDRESS_TYPE.AUTO, | ||
84 | 820 | subnet=subnet, | ||
85 | 821 | interface=interface, | ||
86 | 819 | ) | 822 | ) |
87 | 820 | self.assertEqual( | 823 | self.assertEqual( |
89 | 821 | ip_address, dhcp.get_ip_address_for_interface(interface, vlan) | 824 | ip_address, |
90 | 825 | dhcp.get_ip_address_for_interface( | ||
91 | 826 | interface, vlan, subnet.get_ip_version() | ||
92 | 827 | ), | ||
93 | 822 | ) | 828 | ) |
94 | 823 | 829 | ||
95 | 824 | def test_returns_None(self): | 830 | def test_returns_None(self): |
96 | @@ -826,10 +832,15 @@ class TestGetIPAddressForInterface(MAASServerTestCase): | |||
97 | 826 | interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, vlan=vlan) | 832 | interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, vlan=vlan) |
98 | 827 | subnet = factory.make_Subnet(vlan=vlan) | 833 | subnet = factory.make_Subnet(vlan=vlan) |
99 | 828 | factory.make_StaticIPAddress( | 834 | factory.make_StaticIPAddress( |
101 | 829 | alloc_type=IPADDRESS_TYPE.AUTO, subnet=subnet, interface=interface | 835 | ip=factory.pick_ip_in_Subnet(subnet), |
102 | 836 | alloc_type=IPADDRESS_TYPE.AUTO, | ||
103 | 837 | subnet=subnet, | ||
104 | 838 | interface=interface, | ||
105 | 830 | ) | 839 | ) |
106 | 831 | self.assertIsNone( | 840 | self.assertIsNone( |
108 | 832 | dhcp.get_ip_address_for_interface(interface, factory.make_VLAN()) | 841 | dhcp.get_ip_address_for_interface( |
109 | 842 | interface, factory.make_VLAN(), subnet.get_ip_version() | ||
110 | 843 | ) | ||
111 | 833 | ) | 844 | ) |
112 | 834 | 845 | ||
113 | 835 | 846 | ||
114 | @@ -844,11 +855,16 @@ class TestGetIPAddressForRackController(MAASServerTestCase): | |||
115 | 844 | ) | 855 | ) |
116 | 845 | subnet = factory.make_Subnet(vlan=vlan) | 856 | subnet = factory.make_Subnet(vlan=vlan) |
117 | 846 | ip_address = factory.make_StaticIPAddress( | 857 | ip_address = factory.make_StaticIPAddress( |
119 | 847 | alloc_type=IPADDRESS_TYPE.AUTO, subnet=subnet, interface=interface | 858 | ip=factory.pick_ip_in_Subnet(subnet), |
120 | 859 | alloc_type=IPADDRESS_TYPE.AUTO, | ||
121 | 860 | subnet=subnet, | ||
122 | 861 | interface=interface, | ||
123 | 848 | ) | 862 | ) |
124 | 849 | self.assertEqual( | 863 | self.assertEqual( |
125 | 850 | ip_address, | 864 | ip_address, |
127 | 851 | dhcp.get_ip_address_for_rack_controller(rack_controller, vlan), | 865 | dhcp.get_ip_address_for_rack_controller( |
128 | 866 | rack_controller, vlan, subnet.get_ip_version() | ||
129 | 867 | ), | ||
130 | 852 | ) | 868 | ) |
131 | 853 | 869 | ||
132 | 854 | def test_returns_ip_address_from_best_interface_on_rack_controller(self): | 870 | def test_returns_ip_address_from_best_interface_on_rack_controller(self): |
133 | @@ -868,7 +884,10 @@ class TestGetIPAddressForRackController(MAASServerTestCase): | |||
134 | 868 | ) | 884 | ) |
135 | 869 | subnet = factory.make_Subnet(vlan=vlan) | 885 | subnet = factory.make_Subnet(vlan=vlan) |
136 | 870 | factory.make_StaticIPAddress( | 886 | factory.make_StaticIPAddress( |
138 | 871 | alloc_type=IPADDRESS_TYPE.AUTO, subnet=subnet, interface=interface | 887 | ip=factory.pick_ip_in_Subnet(subnet), |
139 | 888 | alloc_type=IPADDRESS_TYPE.AUTO, | ||
140 | 889 | subnet=subnet, | ||
141 | 890 | interface=interface, | ||
142 | 872 | ) | 891 | ) |
143 | 873 | bond_ip_address = factory.make_StaticIPAddress( | 892 | bond_ip_address = factory.make_StaticIPAddress( |
144 | 874 | alloc_type=IPADDRESS_TYPE.AUTO, | 893 | alloc_type=IPADDRESS_TYPE.AUTO, |
145 | @@ -877,7 +896,9 @@ class TestGetIPAddressForRackController(MAASServerTestCase): | |||
146 | 877 | ) | 896 | ) |
147 | 878 | self.assertEqual( | 897 | self.assertEqual( |
148 | 879 | bond_ip_address, | 898 | bond_ip_address, |
150 | 880 | dhcp.get_ip_address_for_rack_controller(rack_controller, vlan), | 899 | dhcp.get_ip_address_for_rack_controller( |
151 | 900 | rack_controller, vlan, subnet.get_ip_version() | ||
152 | 901 | ), | ||
153 | 881 | ) | 902 | ) |
154 | 882 | 903 | ||
155 | 883 | 904 | ||
156 | @@ -2121,11 +2142,12 @@ class TestMakeFailoverPeerConfig(MAASServerTestCase): | |||
157 | 2121 | primary_rack=primary_rack, | 2142 | primary_rack=primary_rack, |
158 | 2122 | secondary_rack=secondary_rack, | 2143 | secondary_rack=secondary_rack, |
159 | 2123 | ) | 2144 | ) |
161 | 2124 | subnet = factory.make_Subnet(vlan=vlan) | 2145 | subnet = factory.make_Subnet(vlan=vlan, version=4) |
162 | 2125 | primary_interface = factory.make_Interface( | 2146 | primary_interface = factory.make_Interface( |
163 | 2126 | INTERFACE_TYPE.PHYSICAL, node=primary_rack, vlan=vlan | 2147 | INTERFACE_TYPE.PHYSICAL, node=primary_rack, vlan=vlan |
164 | 2127 | ) | 2148 | ) |
165 | 2128 | primary_ip = factory.make_StaticIPAddress( | 2149 | primary_ip = factory.make_StaticIPAddress( |
166 | 2150 | ip=factory.pick_ip_in_Subnet(subnet), | ||
167 | 2129 | alloc_type=IPADDRESS_TYPE.AUTO, | 2151 | alloc_type=IPADDRESS_TYPE.AUTO, |
168 | 2130 | subnet=subnet, | 2152 | subnet=subnet, |
169 | 2131 | interface=primary_interface, | 2153 | interface=primary_interface, |
170 | @@ -2134,6 +2156,7 @@ class TestMakeFailoverPeerConfig(MAASServerTestCase): | |||
171 | 2134 | INTERFACE_TYPE.PHYSICAL, node=secondary_rack, vlan=vlan | 2156 | INTERFACE_TYPE.PHYSICAL, node=secondary_rack, vlan=vlan |
172 | 2135 | ) | 2157 | ) |
173 | 2136 | secondary_ip = factory.make_StaticIPAddress( | 2158 | secondary_ip = factory.make_StaticIPAddress( |
174 | 2159 | ip=factory.pick_ip_in_Subnet(subnet), | ||
175 | 2137 | alloc_type=IPADDRESS_TYPE.AUTO, | 2160 | alloc_type=IPADDRESS_TYPE.AUTO, |
176 | 2138 | subnet=subnet, | 2161 | subnet=subnet, |
177 | 2139 | interface=secondary_interface, | 2162 | interface=secondary_interface, |
178 | @@ -2150,7 +2173,7 @@ class TestMakeFailoverPeerConfig(MAASServerTestCase): | |||
179 | 2150 | }, | 2173 | }, |
180 | 2151 | secondary_rack, | 2174 | secondary_rack, |
181 | 2152 | ), | 2175 | ), |
183 | 2153 | dhcp.make_failover_peer_config(vlan, primary_rack), | 2176 | dhcp.make_failover_peer_config(vlan, primary_rack, 4), |
184 | 2154 | ) | 2177 | ) |
185 | 2155 | 2178 | ||
186 | 2156 | def test_renders_config_for_secondary(self): | 2179 | def test_renders_config_for_secondary(self): |
187 | @@ -2161,11 +2184,12 @@ class TestMakeFailoverPeerConfig(MAASServerTestCase): | |||
188 | 2161 | primary_rack=primary_rack, | 2184 | primary_rack=primary_rack, |
189 | 2162 | secondary_rack=secondary_rack, | 2185 | secondary_rack=secondary_rack, |
190 | 2163 | ) | 2186 | ) |
192 | 2164 | subnet = factory.make_Subnet(vlan=vlan) | 2187 | subnet = factory.make_Subnet(vlan=vlan, version=4) |
193 | 2165 | primary_interface = factory.make_Interface( | 2188 | primary_interface = factory.make_Interface( |
194 | 2166 | INTERFACE_TYPE.PHYSICAL, node=primary_rack, vlan=vlan | 2189 | INTERFACE_TYPE.PHYSICAL, node=primary_rack, vlan=vlan |
195 | 2167 | ) | 2190 | ) |
196 | 2168 | primary_ip = factory.make_StaticIPAddress( | 2191 | primary_ip = factory.make_StaticIPAddress( |
197 | 2192 | ip=factory.pick_ip_in_Subnet(subnet), | ||
198 | 2169 | alloc_type=IPADDRESS_TYPE.AUTO, | 2193 | alloc_type=IPADDRESS_TYPE.AUTO, |
199 | 2170 | subnet=subnet, | 2194 | subnet=subnet, |
200 | 2171 | interface=primary_interface, | 2195 | interface=primary_interface, |
201 | @@ -2174,6 +2198,7 @@ class TestMakeFailoverPeerConfig(MAASServerTestCase): | |||
202 | 2174 | INTERFACE_TYPE.PHYSICAL, node=secondary_rack, vlan=vlan | 2198 | INTERFACE_TYPE.PHYSICAL, node=secondary_rack, vlan=vlan |
203 | 2175 | ) | 2199 | ) |
204 | 2176 | secondary_ip = factory.make_StaticIPAddress( | 2200 | secondary_ip = factory.make_StaticIPAddress( |
205 | 2201 | ip=factory.pick_ip_in_Subnet(subnet), | ||
206 | 2177 | alloc_type=IPADDRESS_TYPE.AUTO, | 2202 | alloc_type=IPADDRESS_TYPE.AUTO, |
207 | 2178 | subnet=subnet, | 2203 | subnet=subnet, |
208 | 2179 | interface=secondary_interface, | 2204 | interface=secondary_interface, |
209 | @@ -2190,7 +2215,78 @@ class TestMakeFailoverPeerConfig(MAASServerTestCase): | |||
210 | 2190 | }, | 2215 | }, |
211 | 2191 | primary_rack, | 2216 | primary_rack, |
212 | 2192 | ), | 2217 | ), |
214 | 2193 | dhcp.make_failover_peer_config(vlan, secondary_rack), | 2218 | dhcp.make_failover_peer_config(vlan, secondary_rack, 4), |
215 | 2219 | ) | ||
216 | 2220 | |||
217 | 2221 | # See https://bugs.launchpad.net/maas/+bug/2027621 | ||
218 | 2222 | def test_renders_config_for_secondary_should_not_mix_v4_and_v6_addresses( | ||
219 | 2223 | self, | ||
220 | 2224 | ): | ||
221 | 2225 | primary_rack = factory.make_RackController() | ||
222 | 2226 | secondary_rack = factory.make_RackController() | ||
223 | 2227 | vlan = factory.make_VLAN( | ||
224 | 2228 | dhcp_on=True, | ||
225 | 2229 | primary_rack=primary_rack, | ||
226 | 2230 | secondary_rack=secondary_rack, | ||
227 | 2231 | ) | ||
228 | 2232 | subnet_v4 = factory.make_Subnet(vlan=vlan, version=4) | ||
229 | 2233 | subnet_v6 = factory.make_Subnet(vlan=vlan, version=6) | ||
230 | 2234 | primary_interface = factory.make_Interface( | ||
231 | 2235 | INTERFACE_TYPE.PHYSICAL, node=primary_rack, vlan=vlan | ||
232 | 2236 | ) | ||
233 | 2237 | primary_ip_v4 = factory.make_StaticIPAddress( | ||
234 | 2238 | ip=factory.pick_ip_in_Subnet(subnet_v4), | ||
235 | 2239 | alloc_type=IPADDRESS_TYPE.AUTO, | ||
236 | 2240 | subnet=subnet_v4, | ||
237 | 2241 | interface=primary_interface, | ||
238 | 2242 | ) | ||
239 | 2243 | primary_ip_v6 = factory.make_StaticIPAddress( | ||
240 | 2244 | ip=factory.pick_ip_in_Subnet(subnet_v6), | ||
241 | 2245 | alloc_type=IPADDRESS_TYPE.AUTO, | ||
242 | 2246 | subnet=subnet_v6, | ||
243 | 2247 | interface=primary_interface, | ||
244 | 2248 | ) | ||
245 | 2249 | secondary_interface = factory.make_Interface( | ||
246 | 2250 | INTERFACE_TYPE.PHYSICAL, node=secondary_rack, vlan=vlan | ||
247 | 2251 | ) | ||
248 | 2252 | secondary_ip_v4 = factory.make_StaticIPAddress( | ||
249 | 2253 | ip=factory.pick_ip_in_Subnet(subnet_v4), | ||
250 | 2254 | alloc_type=IPADDRESS_TYPE.AUTO, | ||
251 | 2255 | subnet=subnet_v4, | ||
252 | 2256 | interface=secondary_interface, | ||
253 | 2257 | ) | ||
254 | 2258 | secondary_ip_v6 = factory.make_StaticIPAddress( | ||
255 | 2259 | ip=factory.pick_ip_in_Subnet(subnet_v6), | ||
256 | 2260 | alloc_type=IPADDRESS_TYPE.AUTO, | ||
257 | 2261 | subnet=subnet_v6, | ||
258 | 2262 | interface=secondary_interface, | ||
259 | 2263 | ) | ||
260 | 2264 | failover_peer_name = "failover-vlan-%d" % vlan.id | ||
261 | 2265 | self.assertEqual( | ||
262 | 2266 | ( | ||
263 | 2267 | failover_peer_name, | ||
264 | 2268 | { | ||
265 | 2269 | "name": failover_peer_name, | ||
266 | 2270 | "mode": "secondary", | ||
267 | 2271 | "address": str(secondary_ip_v4.ip), | ||
268 | 2272 | "peer_address": str(primary_ip_v4.ip), | ||
269 | 2273 | }, | ||
270 | 2274 | primary_rack, | ||
271 | 2275 | ), | ||
272 | 2276 | dhcp.make_failover_peer_config(vlan, secondary_rack, 4), | ||
273 | 2277 | ) | ||
274 | 2278 | self.assertEqual( | ||
275 | 2279 | ( | ||
276 | 2280 | failover_peer_name, | ||
277 | 2281 | { | ||
278 | 2282 | "name": failover_peer_name, | ||
279 | 2283 | "mode": "secondary", | ||
280 | 2284 | "address": str(secondary_ip_v6.ip), | ||
281 | 2285 | "peer_address": str(primary_ip_v6.ip), | ||
282 | 2286 | }, | ||
283 | 2287 | primary_rack, | ||
284 | 2288 | ), | ||
285 | 2289 | dhcp.make_failover_peer_config(vlan, secondary_rack, 6), | ||
286 | 2194 | ) | 2290 | ) |
287 | 2195 | 2291 | ||
288 | 2196 | 2292 |
LGTM!