Merge ~mpontillo/maas:commissioning-prevent-ip-creation-on-same-subnet--bug-1803188--2.4 into maas:2.4

Proposed by Mike Pontillo on 2018-12-11
Status: Merged
Approved by: Mike Pontillo on 2018-12-11
Approved revision: cb149591de8a698dd578e041b25a1a7bbbd7b13d
Merged at revision: fab931122fd159f31eba70ea52618dfee715c6d9
Proposed branch: ~mpontillo/maas:commissioning-prevent-ip-creation-on-same-subnet--bug-1803188--2.4
Merge into: maas:2.4
Diff against target: 94 lines (+51/-8)
2 files modified
src/maasserver/models/interface.py (+33/-8)
src/maasserver/models/tests/test_interface.py (+18/-0)
Reviewer Review Type Date Requested Status
Mike Pontillo (community) Approve on 2018-12-11
MAAS Lander unittests Pending
Review via email: mp+360729@code.launchpad.net

Commit message

Backport 594f8f8bb: LP #1803188: Prevent creation of multiple IPs (per subnet, per interface) during commissioning.

In some situations, the IP address from the preboot environment differs
from the IP address acquired by the DHCP client during commissioning. This
results in MAAS linking the interface to the subnet twice, which is
unexpected and can cause problems for future usage of the machine.

To post a comment you must log in.
Mike Pontillo (mpontillo) wrote :

Self-approve backport.

review: Approve

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 5839e73..5e13965 100644
--- a/src/maasserver/models/interface.py
+++ b/src/maasserver/models/interface.py
@@ -724,6 +724,10 @@ class Interface(CleanSave, TimestampedModel):
724 StaticIPAddress.objects.filter(724 StaticIPAddress.objects.filter(
725 interface=self, alloc_type=IPADDRESS_TYPE.DISCOVERED).delete()725 interface=self, alloc_type=IPADDRESS_TYPE.DISCOVERED).delete()
726726
727 # Keep track of which subnets were found, in order to avoid linking
728 # duplicates.
729 created_on_subnets = set()
730
727 # Sort the cidr list by prefixlen.731 # Sort the cidr list by prefixlen.
728 for ip in sorted(cidr_list, key=lambda x: int(x.split('/')[1])):732 for ip in sorted(cidr_list, key=lambda x: int(x.split('/')[1])):
729 network = IPNetwork(ip)733 network = IPNetwork(ip)
@@ -822,20 +826,41 @@ class Interface(CleanSave, TimestampedModel):
822 " on " + node.fqdn if node is not None else '')826 " on " + node.fqdn if node is not None else '')
823 prev_address.delete()827 prev_address.delete()
824828
825 # XXX lamont 2016-11-03 Bug#1639090
826 # At the moment, IPv6 autoconf (SLAAC) is required so that we get829 # At the moment, IPv6 autoconf (SLAAC) is required so that we get
827 # the correct subnet block created above. However, if we add SLAAC830 # the correct subnet block created above. However, if we add SLAAC
828 # addresses to the DB, then we wind up creating 2 autoassigned831 # addresses to the DB, then we wind up creating 2 autoassigned
829 # addresses on the interface. We need to discuss how to model them832 # addresses on the interface. We need to discuss how to model them
830 # and incorporate the change for 2.2. For now, just drop them with833 # and incorporate the change for 2.2. For now, just drop them with
831 # prejudice. (Bug#1639288)834 # prejudice. (Bug#1639288)
832 if address != str(self._eui64_address(subnet.cidr)):835 if address == str(self._eui64_address(subnet.cidr)):
833 # Create the newly discovered IP address.836 maaslog.warning(
834 new_address = StaticIPAddress(837 "IP address (%s)%s was skipped because "
835 alloc_type=IPADDRESS_TYPE.DISCOVERED, ip=address,838 "it is an EUI-64 (SLAAC) address.",
836 subnet=subnet)839 address,
837 new_address.save()840 " on " + self.node.fqdn if self.node is not None else '')
838 self.ip_addresses.add(new_address)841 continue
842
843 # Remember which subnets we created addresses on; we don't want to
844 # link more than one address per subnet in case of a duplicate.
845 # Duplicates could happen, in theory, if the IP address configured
846 # in the preboot environment differs from the IP address acquired
847 # by the DHCP client. See bug #1803188.
848 if subnet in created_on_subnets:
849 maaslog.warning(
850 "IP address (%s)%s was skipped because it was found on "
851 "the same subnet as a previous address: %s.",
852 address,
853 " on " + self.node.fqdn if self.node is not None else '',
854 network)
855 continue
856
857 # Create the newly discovered IP address.
858 new_address = StaticIPAddress(
859 alloc_type=IPADDRESS_TYPE.DISCOVERED, ip=address,
860 subnet=subnet)
861 new_address.save()
862 self.ip_addresses.add(new_address)
863 created_on_subnets.add(subnet)
839864
840 def _eui64_address(self, net_cidr):865 def _eui64_address(self, net_cidr):
841 """Return the SLAAC address for this interface."""866 """Return the SLAAC address for this interface."""
diff --git a/src/maasserver/models/tests/test_interface.py b/src/maasserver/models/tests/test_interface.py
index afb0d38..c7262ad 100644
--- a/src/maasserver/models/tests/test_interface.py
+++ b/src/maasserver/models/tests/test_interface.py
@@ -1912,6 +1912,24 @@ class UpdateIpAddressesTest(MAASServerTestCase):
1912 self.assertEqual(0, iface.ip_addresses.count())1912 self.assertEqual(0, iface.ip_addresses.count())
1913 self.assertEqual(1, Subnet.objects.filter(cidr=network.cidr).count())1913 self.assertEqual(1, Subnet.objects.filter(cidr=network.cidr).count())
19141914
1915 def test__does_not_add_addresses_from_duplicate_subnet(self):
1916 # See also LP#1803188.
1917 mac_address = factory.make_MAC()
1918 vlan = factory.make_VLAN()
1919 factory.make_Subnet(cidr="10.0.0.0/8", vlan=vlan)
1920 factory.make_Subnet(cidr="2001::/64", vlan=vlan)
1921 node = factory.make_Node()
1922 iface = factory.make_Interface(
1923 INTERFACE_TYPE.PHYSICAL, mac_address=mac_address, vlan=vlan,
1924 node=node)
1925 iface.update_ip_addresses([
1926 "10.0.0.1/8",
1927 "10.0.0.2/8",
1928 "2001::1/64",
1929 "2001::2/64",
1930 ])
1931 self.assertEqual(2, iface.ip_addresses.count())
1932
1915 def test__finds_ipv6_subnet_regardless_of_order(self):1933 def test__finds_ipv6_subnet_regardless_of_order(self):
1916 iface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)1934 iface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
1917 network = factory.make_ipv6_network()1935 network = factory.make_ipv6_network()

Subscribers

People subscribed via source and target branches