MAAS

Merge lp:~lamont/maas/bug-1626722-slash-128-ipv6-addresses into lp:~maas-committers/maas/trunk

  1. bug-1626722-slash-128-ipv6-addresses
  2. Merge into trunk
Proposed by LaMont Jones
Status: Merged
Approved by: LaMont Jones
Approved revision: no longer in the source branch.
Merged at revision: 5393
Proposed branch: lp:~lamont/maas/bug-1626722-slash-128-ipv6-addresses
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 142 lines (+71/-15)
2 files modified
src/maasserver/models/interface.py (+35/-11)
src/maasserver/models/tests/test_interface.py (+36/-4)
To merge this branch: bzr merge lp:~lamont/maas/bug-1626722-slash-128-ipv6-addresses
Reviewer Review Type Date Requested Status
Mike Pontillo (community) Approve
Review via email: mp+306658@code.launchpad.net

Commit message

Special handling for discovered /128 IPv6 addresses.

Description of the change

Special handling for discovered /128 IPv6 addresses.

To post a comment you must log in.
Revision history for this message
Mike Pontillo (mpontillo) wrote :

Code looks good, but you're missing a unit test to ensure the sorting behavior is correct. Please add that before landing.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/models/interface.py'
2--- src/maasserver/models/interface.py 2016-09-13 22:22:18 +0000
3+++ src/maasserver/models/interface.py 2016-09-23 19:44:30 +0000
4@@ -633,7 +633,8 @@
5 StaticIPAddress.objects.filter(
6 interface=self, alloc_type=IPADDRESS_TYPE.DISCOVERED).delete()
7
8- for ip in cidr_list:
9+ # Sort the cidr list by prefixlen.
10+ for ip in sorted(cidr_list, key=lambda x: int(x.split('/')[1])):
11 network = IPNetwork(ip)
12 cidr = str(network.cidr)
13 address = str(network.ip)
14@@ -650,16 +651,39 @@
15 self.vlan = vlan
16 self.save()
17 except Subnet.DoesNotExist:
18- # XXX mpontillo 2015-11-01 Configuration != state. That is,
19- # this means we have just a subnet on an unknown Fabric/VLAN,
20- # and this fact should be recorded somewhere, so that the user
21- # gets a chance to configure it. Note, however, that if this
22- # is already a managed cluster interface, a Fabric/VLAN will
23- # already have been created.
24- subnet = Subnet.objects.create_from_cidr(cidr)
25- maaslog.info(
26- "Creating subnet %s connected to interface %s "
27- "of node %s.", cidr, self, self.get_node())
28+ subnet = None
29+ if network.version == 6 and network.prefixlen == 128:
30+ # Bug#1626722: /128 ipv6 addresses are special. DHCPv6
31+ # does not provide prefixlen in any way - "that is the duty
32+ # of router-advertisments." Find the correct subnet, if
33+ # any.
34+ # See also: launchpad.net/bugs/1609898.
35+ subnet = Subnet.objects.get_best_subnet_for_ip(address)
36+ if subnet is None:
37+ # XXX lamont 2016-09-23 Bug#1627160: This is an IP with
38+ # NO associated subnet (we would have added the subnet
39+ # earlier in this loop if there was an address
40+ # configured on the interface proper. We would also
41+ # have it if we were the DHCP provider for the network.
42+ # In other words, what we have is a DHCP client on a
43+ # subnet with no RA configured, and MAAS is not
44+ # providing DHCP, and has not been told about the
45+ # subnet... For now, assume that the subnet is a /64
46+ # (which the admin can edit later.) Eventually, we'll
47+ # want to look and see if the host has a link-local
48+ # route for any block containing the IP.
49+ cidr = '%s/64' % address
50+ if subnet is None:
51+ # XXX mpontillo 2015-11-01 Configuration != state. That is,
52+ # this means we have just a subnet on an unknown
53+ # Fabric/VLAN, and this fact should be recorded somewhere,
54+ # so that the user gets a chance to configure it. Note,
55+ # however, that if this is already a managed cluster
56+ # interface, a Fabric/VLAN will already have been created.
57+ subnet = Subnet.objects.create_from_cidr(cidr)
58+ maaslog.info(
59+ "Creating subnet %s connected to interface %s "
60+ "of node %s.", cidr, self, self.get_node())
61
62 # First check if this IP address exists in the database (at all).
63 prev_address = get_one(
64
65=== modified file 'src/maasserver/models/tests/test_interface.py'
66--- src/maasserver/models/tests/test_interface.py 2016-09-14 07:20:33 +0000
67+++ src/maasserver/models/tests/test_interface.py 2016-09-23 19:44:30 +0000
68@@ -1664,6 +1664,38 @@
69
70 class UpdateIpAddressesTest(MAASServerTestCase):
71
72+ def test__finds_ipv6_subnet(self):
73+ interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
74+ network = factory.make_ipv6_network()
75+ subnet = factory.make_Subnet(cidr=network.cidr)
76+ cidr = "%s/128" % str(IPAddress(network.first + 1))
77+ interface.update_ip_addresses([cidr])
78+
79+ self.assertFalse(Subnet.objects.filter(cidr=cidr).exists())
80+ self.assertEqual(interface.ip_addresses.first().subnet, subnet)
81+
82+ def test__finds_ipv6_subnet_regardless_of_order(self):
83+ iface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
84+ network = factory.make_ipv6_network()
85+ subnet = factory.make_Subnet(cidr=network.cidr)
86+ cidr_net = str(network.cidr)
87+ cidr_128 = "%s/128" % str(IPAddress(network.first + 1))
88+ iface.update_ip_addresses([cidr_128, cidr_net])
89+
90+ self.assertFalse(Subnet.objects.filter(cidr=cidr_128).exists())
91+ self.assertFalse(iface.ip_addresses.exclude(subnet=subnet).exists())
92+
93+ def test__creates_missing_slash_64_ipv6_subnet(self):
94+ interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
95+ network = factory.make_ipv6_network()
96+ cidr = "%s/128" % str(IPAddress(network.first + 1))
97+ interface.update_ip_addresses([cidr])
98+
99+ subnets = Subnet.objects.filter(cidr="%s/64" % str(network.ip))
100+ self.assertEqual(1, len(subnets))
101+ self.assertFalse(Subnet.objects.filter(cidr=cidr).exists())
102+ self.assertEqual(interface.ip_addresses.first().subnet, subnets[0])
103+
104 def test__creates_missing_subnet(self):
105 interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
106 network = factory.make_ip4_or_6_network()
107@@ -1701,7 +1733,7 @@
108
109 self.assertEqual(num_connections, interface.ip_addresses.count())
110 for i in range(num_connections):
111- ip = interface.ip_addresses.order_by('id')[i]
112+ ip = interface.ip_addresses.get(ip=cidr_list[i].split('/')[0])
113 self.assertThat(ip, MatchesStructure.byEquality(
114 alloc_type=IPADDRESS_TYPE.DISCOVERED, subnet=subnet_list[i],
115 ip=str(IPNetwork(cidr_list[i]).ip)))
116@@ -1783,7 +1815,7 @@
117 "Discovered IP address should have been deleted.")
118 self.assertEqual(num_connections, interface.ip_addresses.count())
119 for i in range(num_connections):
120- ip = interface.ip_addresses.order_by('id')[i]
121+ ip = interface.ip_addresses.get(ip=cidr_list[i].split('/')[0])
122 self.assertThat(ip, MatchesStructure.byEquality(
123 alloc_type=IPADDRESS_TYPE.DISCOVERED, subnet=subnet_list[i],
124 ip=str(IPNetwork(cidr_list[i]).ip)))
125@@ -1829,7 +1861,7 @@
126 "Unknown interfaces should have been deleted.")
127 self.assertEqual(num_connections, interface.ip_addresses.count())
128 for i in range(num_connections):
129- ip = interface.ip_addresses.order_by('id')[i]
130+ ip = interface.ip_addresses.get(ip=cidr_list[i].split('/')[0])
131 self.assertThat(ip, MatchesStructure.byEquality(
132 alloc_type=IPADDRESS_TYPE.DISCOVERED, subnet=subnet_list[i],
133 ip=str(IPNetwork(cidr_list[i]).ip)))
134@@ -1865,7 +1897,7 @@
135 "Sticky IP address should have been deleted.")
136 self.assertEqual(num_connections, interface.ip_addresses.count())
137 for i in range(num_connections):
138- ip = interface.ip_addresses.order_by('id')[i]
139+ ip = interface.ip_addresses.get(ip=cidr_list[i].split('/')[0])
140 self.assertThat(ip, MatchesStructure.byEquality(
141 alloc_type=IPADDRESS_TYPE.DISCOVERED, subnet=subnet_list[i],
142 ip=str(IPNetwork(cidr_list[i]).ip)))