Merge lp:~lamont/maas/bug-1584850-trunk into lp:~maas-committers/maas/trunk

Proposed by LaMont Jones
Status: Merged
Approved by: LaMont Jones
Approved revision: no longer in the source branch.
Merged at revision: 5051
Proposed branch: lp:~lamont/maas/bug-1584850-trunk
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 130 lines (+66/-6)
2 files modified
src/maasserver/models/staticipaddress.py (+26/-5)
src/maasserver/models/tests/test_staticipaddress.py (+40/-1)
To merge this branch: bzr merge lp:~lamont/maas/bug-1584850-trunk
Reviewer Review Type Date Requested Status
Gavin Panella (community) Approve
Review via email: mp+295630@code.launchpad.net

Commit message

Prefer boot-interface addresses to non-boot-interface addresses (in get_hostname_ip_mapping) and don't mix-n-match them.

Description of the change

Prefer boot-interface addresses to non-boot-interface addresses (in get_hostname_ip_mapping) and don't mix-n-match them.

To post a comment you must log in.
Revision history for this message
Gavin Panella (allenap) wrote :

See my review at https://code.launchpad.net/~lamont/maas/bug-1584850/+merge/295621.

FWIW, fixes are normally landed in trunk before they get a backport.

review: Needs Fixing
Revision history for this message
Gavin Panella (allenap) :
review: Approve
Revision history for this message
MAAS Lander (maas-lander) wrote :
Download full text (1.3 MiB)

The attempt to merge lp:~lamont/maas/bug-1584850-trunk into lp:maas failed. Below is the output from the failed tests.

Get:1 http://security.ubuntu.com/ubuntu xenial-security InRelease [94.5 kB]
Hit:2 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial InRelease
Get:3 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates InRelease [94.5 kB]
Hit:4 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-backports InRelease
Fetched 189 kB in 0s (420 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
    --no-install-recommends install apache2 archdetect-deb authbind bash bind9 bind9utils build-essential bzr bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libpq-dev make nodejs-legacy npm postgresql pxelinux python3-all python3-apt python3-bson python3-convoy python3-crochet python3-cssselect python3-curtin python3-dev python3-distro-info python3-django python3-django-nose python3-django-piston3 python3-dnspython python3-docutils python3-formencode python3-hivex python3-httplib2 python3-jinja2 python3-jsonschema python3-lxml python3-netaddr python3-netifaces python3-novaclient python3-oauth python3-oauthlib python3-openssl python3-paramiko python3-petname python3-pexpect python3-psycopg2 python3-pyinotify python3-pyparsing python3-pyvmomi python3-requests python3-seamicroclient python3-setuptools python3-simplestreams python3-sphinx python3-tempita python3-twisted python3-txtftp python3-tz python3-yaml python3-zope.interface python-bson python-crochet python-django python-django-piston python-djorm-ext-pgarray python-formencode python-lxml python-netaddr python-netifaces python-pocket-lint python-psycopg2 python-simplejson python-tempita python-twisted python-yaml socat syslinux-common tgt ubuntu-cloudimage-keyring wget xvfb
Reading package lists...
Building dependency tree...
Reading state information...
apache2 is already the newest version (2.4.18-2ubuntu3).
archdetect-deb is already the newest version (1.117ubuntu2).
authbind is already the newest version (2.1.1+nmu1).
bash is already the newest version (4.3-14ubuntu1).
build-essential is already the newest version (12.1ubuntu2).
bzr is already the newest version (2.7.0-2ubuntu1).
curl is already the newest version (7.47.0-1ubuntu2).
debhelper is already the newest version (9.20160115ubuntu3).
distro-info is already the newest version (0.14build1).
freeipmi-tools is already the newest version (1.4.11-1ubuntu1).
git is already the newest version (1:2.7.4-0ubuntu1).
isc-dhcp-common is already the newest version (4.3.3-5ubuntu12).
libjs-angularjs is already the newest version (1.2.28-1ubuntu2).
libjs-jquery is already the newest version (1.11.3+dfsg-4).
libjs-yui3-full is already the newest version (3.5.1-1ubuntu3).
libjs-yui3-min is already the newest version (3.5.1-1ubuntu3).
make is already the newest version (4.1-6).
postgresql is already the newest version (9.5+173).
pxelinux is already the newest version (3:6.03+dfsg-1...

Revision history for this message
MAAS Lander (maas-lander) wrote :
Download full text (1.3 MiB)

The attempt to merge lp:~lamont/maas/bug-1584850-trunk into lp:maas failed. Below is the output from the failed tests.

Hit:1 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial InRelease
Get:2 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates InRelease [94.5 kB]
Get:3 http://security.ubuntu.com/ubuntu xenial-security InRelease [94.5 kB]
Hit:4 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-backports InRelease
Get:5 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates/main Sources [40.5 kB]
Get:6 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates/main amd64 Packages [111 kB]
Get:7 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates/main Translation-en [45.4 kB]
Fetched 386 kB in 0s (829 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
    --no-install-recommends install apache2 archdetect-deb authbind bash bind9 bind9utils build-essential bzr bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libpq-dev make nodejs-legacy npm postgresql pxelinux python3-all python3-apt python3-bson python3-convoy python3-crochet python3-cssselect python3-curtin python3-dev python3-distro-info python3-django python3-django-nose python3-django-piston3 python3-dnspython python3-docutils python3-formencode python3-hivex python3-httplib2 python3-jinja2 python3-jsonschema python3-lxml python3-netaddr python3-netifaces python3-novaclient python3-oauth python3-oauthlib python3-openssl python3-paramiko python3-petname python3-pexpect python3-psycopg2 python3-pyinotify python3-pyparsing python3-pyvmomi python3-requests python3-seamicroclient python3-setuptools python3-simplestreams python3-sphinx python3-tempita python3-twisted python3-txtftp python3-tz python3-yaml python3-zope.interface python-bson python-crochet python-django python-django-piston python-djorm-ext-pgarray python-formencode python-lxml python-netaddr python-netifaces python-pocket-lint python-psycopg2 python-simplejson python-tempita python-twisted python-yaml socat syslinux-common tgt ubuntu-cloudimage-keyring wget xvfb
Reading package lists...
Building dependency tree...
Reading state information...
apache2 is already the newest version (2.4.18-2ubuntu3).
archdetect-deb is already the newest version (1.117ubuntu2).
authbind is already the newest version (2.1.1+nmu1).
bash is already the newest version (4.3-14ubuntu1).
build-essential is already the newest version (12.1ubuntu2).
bzr is already the newest version (2.7.0-2ubuntu1).
curl is already the newest version (7.47.0-1ubuntu2).
debhelper is already the newest version (9.20160115ubuntu3).
distro-info is already the newest version (0.14build1).
freeipmi-tools is already the newest version (1.4.11-1ubuntu1).
git is already the newest version (1:2.7.4-0ubuntu1).
isc-dhcp-common is already the newest version (4.3.3-5ubuntu12).
libjs-angularjs is already the newest version (1.2.28-1ubuntu2).
libjs-jquery is alre...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/models/staticipaddress.py'
2--- src/maasserver/models/staticipaddress.py 2016-05-11 19:01:48 +0000
3+++ src/maasserver/models/staticipaddress.py 2016-05-26 12:46:12 +0000
4@@ -304,7 +304,7 @@
5
6 # DISTINCT ON returns the first matching row for any given
7 # hostname, using the query's ordering. Here, we're trying to
8- # return the IP for the oldest Interface address.
9+ # return the IPs for the oldest Interface address.
10 #
11 # For nodes that have disable_ipv4 set, leave out any IPv4 address.
12 if isinstance(domain_or_subnet, Subnet):
13@@ -331,12 +331,16 @@
14 domain.ttl,
15 %s)""" % default_ttl
16 sql_query = """
17- SELECT DISTINCT ON (node.hostname, family(staticip.ip))
18+ SELECT DISTINCT ON (node.hostname, is_boot, family(staticip.ip))
19 node.hostname,
20 node.system_id,
21 node.node_type,
22 """ + ttl_clause + """ AS ttl,
23- domain.name, staticip.ip
24+ domain.name, staticip.ip,
25+ (
26+ node.boot_interface_id IS NOT NULL AND
27+ node.boot_interface_id = interface.id
28+ ) as is_boot
29 FROM
30 maasserver_interface AS interface
31 LEFT OUTER JOIN maasserver_interfacerelationship AS rel ON
32@@ -366,6 +370,7 @@
33 )
34 ORDER BY
35 node.hostname,
36+ is_boot,
37 family(staticip.ip),
38 CASE
39 WHEN interface.type = 'bond' AND
40@@ -401,14 +406,30 @@
41 # We get user reserved et al mappings first, so that we can overwrite
42 # TTL as we process the return from the SQL horror above.
43 mapping = self._get_user_reserved_mappings(domain_or_subnet)
44+ iface_is_boot = defaultdict(bool, {
45+ hostname: True for hostname in mapping.keys()
46+ })
47 cursor.execute(sql_query, (domain_or_subnet.id,))
48+ # The records from the query provide, for each hostname (after
49+ # stripping domain), the boot and non-boot interface ip address in ipv4
50+ # and ipv6. Our task: if there are boot interace IPs, they win. If
51+ # there are none, then whatever we got wins. The ORDER BY means that
52+ # we will see all of the non-boot interfaces before we see any boot
53+ # interface IPs. See Bug#1584850
54 for (node_name, system_id, node_type,
55- ttl, domain_name, ip) in cursor.fetchall():
56+ ttl, domain_name, ip, is_boot) in cursor.fetchall():
57 hostname = "%s.%s" % (strip_domain(node_name), domain_name)
58 mapping[hostname].node_type = node_type
59 mapping[hostname].system_id = system_id
60 mapping[hostname].ttl = ttl
61- mapping[hostname].ips.add(ip)
62+ # If this is a boot interface, and we have previously only found
63+ # non-boot interface IPs, discard them. See also Bug#1584850.
64+ if is_boot and not iface_is_boot[hostname]:
65+ mapping[hostname].ips = set()
66+ iface_is_boot[hostname] = True
67+ # At this point, if the interface type is correct, keep the IP.
68+ if is_boot == iface_is_boot[hostname]:
69+ mapping[hostname].ips.add(ip)
70 return mapping
71
72 def filter_by_ip_family(self, family):
73
74=== modified file 'src/maasserver/models/tests/test_staticipaddress.py'
75--- src/maasserver/models/tests/test_staticipaddress.py 2016-05-12 19:07:37 +0000
76+++ src/maasserver/models/tests/test_staticipaddress.py 2016-05-26 12:46:12 +0000
77@@ -5,7 +5,10 @@
78
79 __all__ = []
80
81-from random import randint
82+from random import (
83+ randint,
84+ shuffle,
85+)
86 from unittest import skip
87 from unittest.mock import sentinel
88
89@@ -756,6 +759,42 @@
90 self.assertEqual({node.fqdn: HostnameIPMapping(
91 node.system_id, 30, {sip1.ip}, node.node_type)}, mapping)
92
93+ def test_get_hostname_ip_mapping_does_not_return_discovered_and_auto(self):
94+ # Create a situation where we have an AUTO ip on the pxeboot interface,
95+ # and a discovered IP of the other address class (v4/v6) on another
96+ # interface.
97+ domain = Domain.objects.get_default_domain()
98+ cidrs = [factory.make_ipv6_network(), factory.make_ipv4_network()]
99+ shuffle(cidrs)
100+ subnets = [factory.make_Subnet(cidr=cidr) for cidr in cidrs]
101+ # First, make a node with an interface on subnets[0]. Assign the boot
102+ # interface an AUTO IP on subnets[0].
103+ node = factory.make_Node_with_Interface_on_Subnet(
104+ hostname=factory.make_name('host'), subnet=subnets[0],
105+ vlan=subnets[0].vlan, disable_ipv4=False)
106+ sip0 = node.boot_interface.ip_addresses.first()
107+ ip0 = factory.pick_ip_in_network(cidrs[0])
108+ sip0.ip = ip0
109+ sip0.alloc_type = IPADDRESS_TYPE.AUTO
110+ sip0.save()
111+ # Now create another interface, and give it a DISCOVERED IP of the
112+ # other family from subnets[1].
113+ iface1 = factory.make_Interface(node=node, vlan=subnets[1].vlan)
114+ ip1 = factory.pick_ip_in_network(cidrs[1])
115+ sip1 = factory.make_StaticIPAddress(
116+ subnet=subnets[1], ip=ip1,
117+ alloc_type=IPADDRESS_TYPE.DISCOVERED
118+ )
119+ iface1.ip_addresses.add(sip1)
120+ iface1.save()
121+ # The only mapping we should get is for the boot interface.
122+ mapping = StaticIPAddress.objects.get_hostname_ip_mapping(domain)
123+ expected_mapping = {
124+ node.fqdn: HostnameIPMapping(
125+ node.system_id, 30, {sip0.ip}, node.node_type
126+ )}
127+ self.assertEqual(expected_mapping, mapping)
128+
129
130 class TestStaticIPAddress(MAASServerTestCase):
131