Merge lp:~mpontillo/maas/fix-xenial-network-commissioning-bug-1542349-1.10 into lp:maas/1.10

[Mike Pontillo]

Merge revision 4666 from trunk to fix bug #1542349: try to ensure the activities that should occur when commissioning finishes happen just once.

Back-port of this fix to bug #1542349, recently landed on trunk:

https://code.launchpad.net/~mpontillo/maas/fix-xenial-network-commissioning-bug-1542349/+merge/285987

I was hoping for a trivial merge, but this branch may require some additional work due to the following code not being present on trunk:

    node.release_leases()
    node.stop_transition_monitor()

[Mike Pontillo (mpontillo)]

Approving based on trunk approval.

After a local run, it doesn't look like tests need fixing, but we'll see what the lander says.

review: Approve

[MAAS Lander (maas-lander)]

The attempt to merge lp:~mpontillo/maas/fix-xenial-network-commissioning-bug-1542349-1.10 into lp:maas/1.10 failed. Below is the output from the failed tests.

Hit:1 http://security.ubuntu.com/ubuntu xenial-security InRelease
Get:2 http://prodstack-zone-1.clouds.archive.ubuntu.com/ubuntu xenial InRelease [95.8 kB]
Hit:3 http://prodstack-zone-1.clouds.archive.ubuntu.com/ubuntu xenial-updates InRelease
Hit:4 http://prodstack-zone-1.clouds.archive.ubuntu.com/ubuntu xenial-backports InRelease
Get:5 http://prodstack-zone-1.clouds.archive.ubuntu.com/ubuntu xenial/main Sources [1,132 kB]
Get:6 http://prodstack-zone-1.clouds.archive.ubuntu.com/ubuntu xenial/universe Sources [7,685 kB]
Get:7 http://prodstack-zone-1.clouds.archive.ubuntu.com/ubuntu xenial/main amd64 Packages [1,469 kB]
Get:8 http://prodstack-zone-1.clouds.archive.ubuntu.com/ubuntu xenial/universe amd64 Packages [7,281 kB]
Get:9 http://prodstack-zone-1.clouds.archive.ubuntu.com/ubuntu xenial/universe Translation-en [4,860 kB]
Fetched 22.5 MB in 13s (1,673 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
    --no-install-recommends install apache2 authbind bind9 bind9utils build-essential 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-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-djorm-ext-pgarray python3-docutils python3-formencode python3-hivex python3-httplib2 python3-jinja2 python3-jsonschema python3-lxml python3-mock python3-netaddr python3-netifaces python3-oauth python3-openssl python3-paramiko python3-petname python3-pexpect python3-psycopg2 python3-pyinotify python3-pyparsing python3-seamicroclient python3-simplejson 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-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-1ubuntu1).
authbind is already the newest version (2.1.1+nmu1).
bind9 is already the newest version (1:9.9.5.dfsg-12.1ubuntu1).
bind9utils is already the newest version (1:9.9.5.dfsg-12.1ubuntu1).
build-essential is already the newest version (12.1ubuntu2).
curl is already the newest version (7.47.0-1ubuntu1).
debhelper is already the newest version (9.20160115ubuntu2).
distro-info is already the newest version (0.14build1).
dnsutils is already the newest version (1:9.9.5.dfsg-12.1ubuntu1).
freeipmi-tools is already the newest version (1.4.11-1ubuntu1).
git is already the newest version (1:2.7.0-1).
isc...

=== modified file 'src/metadataserver/api.py'
--- src/metadataserver/api.py	2015-12-11 20:33:25 +0000
+++ src/metadataserver/api.py	2016-02-17 07:41:00 +0000
@@ -348,21 +348,7 @@
         # At the end of a top-level event, we change the node status.
         if _is_top_level(activity_name) and event_type == 'finish':
             if node.status == NODE_STATUS.COMMISSIONING:
-
-                # Ensure that any IP lease are forcefully released in case
-                # the host didn't bother doing that.
-                node.release_leases()
-
-                node.stop_transition_monitor()
-                if result == 'SUCCESS':
-                    # Recalculate tags.
-                    populate_tags_for_single_node(Tag.objects.all(), node)
-
-                    # Setup the default storage layout and the initial
-                    # networking configuration for the node.
-                    node.set_default_storage_layout()
-                    node.set_initial_networking_configuration()
-                elif result in ['FAIL', 'FAILURE']:
+                if result in ['FAIL', 'FAILURE']:
                     node.status = NODE_STATUS.FAILED_COMMISSIONING
 
             elif node.status == NODE_STATUS.DEPLOYING:
@@ -372,6 +358,9 @@
                         "Installation failed (refer to the "
                         "installation log for more information).")
             elif node.status == NODE_STATUS.DISK_ERASING:
+                # XXX mpontillo 2016-02-12 we need to check that
+                # only one "top level" event is sent during disk
+                # erasing.
                 if result == 'SUCCESS':
                     # disk erasing complete, release node.
                     node.release()

=== added file 'src/metadataserver/models/tests/ip_addr_results_xenial.txt'
--- src/metadataserver/models/tests/ip_addr_results_xenial.txt	1970-01-01 00:00:00 +0000
+++ src/metadataserver/models/tests/ip_addr_results_xenial.txt	2016-02-17 07:41:00 +0000
@@ -0,0 +1,16 @@
+1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN group default 
+    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
+    inet 127.0.0.1/8 scope host lo
+       valid_lft forever preferred_lft forever
+    inet6 ::1/128 scope host 
+       valid_lft forever preferred_lft forever
+2: ens3: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP group default qlen 1000
+    link/ether 52:54:00:2d:39:49 brd ff:ff:ff:ff:ff:ff
+    inet 172.16.100.108/24 brd 172.16.100.255 scope global ens3
+       valid_lft forever preferred_lft forever
+    inet6 fe80::5054:ff:fe2d:3949/64 scope link 
+       valid_lft forever preferred_lft forever
+3: ens10: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN group default qlen 1000
+    link/ether 52:54:00:e5:c6:6b brd ff:ff:ff:ff:ff:ff
+4: ens11: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN group default qlen 1000
+    link/ether 52:54:00:ed:9f:9d brd ff:ff:ff:ff:ff:ff

=== modified file 'src/metadataserver/models/tests/test_commissioningscript.py'
--- src/metadataserver/models/tests/test_commissioningscript.py	2016-02-04 14:04:08 +0000
+++ src/metadataserver/models/tests/test_commissioningscript.py	2016-02-17 07:41:00 +0000
@@ -1460,24 +1460,36 @@
         'eth2': MAC("00:00:00:00:00:03"),
     }
 
+    EXPECTED_INTERFACES_XENIAL = {
+        'ens3': MAC("52:54:00:2d:39:49"),
+        'ens10': MAC("52:54:00:e5:c6:6b"),
+        'ens11': MAC("52:54:00:ed:9f:9d"),
+    }
+
     IP_ADDR_OUTPUT_FILE = os.path.join(
         os.path.dirname(__file__), 'ip_addr_results.txt')
     with open(IP_ADDR_OUTPUT_FILE, "rb") as fd:
         IP_ADDR_OUTPUT = fd.read()
+    IP_ADDR_OUTPUT_FILE = os.path.join(
+        os.path.dirname(__file__), 'ip_addr_results_xenial.txt')
+    with open(IP_ADDR_OUTPUT_FILE, "rb") as fd:
+        IP_ADDR_OUTPUT_XENIAL = fd.read()
 
     def assert_expected_interfaces_and_macs_exist(
-            self, node_interfaces, additional_interfaces={}):
+            self, node_interfaces, additional_interfaces={},
+            expected_interfaces=EXPECTED_INTERFACES):
         """Asserts to ensure that the type, name, and MAC address are
         appropriate, given Node's interfaces. (and an optional list of
         additional interfaces which must exist)
         """
-        expected_interfaces = self.EXPECTED_INTERFACES.copy()
+        expected_interfaces = expected_interfaces.copy()
         expected_interfaces.update(additional_interfaces)
 
         self.assertThat(len(node_interfaces), Equals(len(expected_interfaces)))
 
         for interface in node_interfaces:
-            if interface.name.startswith('eth'):
+            if (interface.name.startswith('eth') or
+                    interface.name.startswith('ens')):
                 parts = interface.name.split('.')
                 if len(parts) == 2 and parts[1].isdigit():
                     iftype = INTERFACE_TYPE.VLAN
@@ -1510,6 +1522,23 @@
         node_interfaces = Interface.objects.filter(node=node)
         self.assert_expected_interfaces_and_macs_exist(node_interfaces)
 
+    def test__add_all_interfaces_xenial(self):
+        """Test a node that has no previously known interfaces on which we
+        need to add a series of interfaces.
+        """
+        node = factory.make_Node()
+
+        # Delete all Interfaces created by factory attached to this node.
+        Interface.objects.filter(node_id=node.id).delete()
+
+        update_node_network_information(node, self.IP_ADDR_OUTPUT_XENIAL, 0)
+
+        # Makes sure all the test dataset MAC addresses were added to the node.
+        node_interfaces = Interface.objects.filter(node=node)
+        self.assert_expected_interfaces_and_macs_exist(
+            node_interfaces,
+            expected_interfaces=self.EXPECTED_INTERFACES_XENIAL)
+
     def test__one_mac_missing(self):
         """Test whether we correctly detach a NIC that no longer appears to be
         connected to the node.
@@ -1728,3 +1757,20 @@
             MatchesStructure.byEquality(
                 alloc_type=IPADDRESS_TYPE.DISCOVERED, subnet=subnet,
                 ip=address))
+
+    def test__creates_discovered_ip_address_on_xenial(self):
+        node = factory.make_Node()
+        cidr = '172.16.100.108/24'
+        subnet = factory.make_Subnet(
+            cidr=cidr, vlan=VLAN.objects.get_default_vlan())
+
+        update_node_network_information(node, self.IP_ADDR_OUTPUT_XENIAL, 0)
+        eth0 = Interface.objects.get(node=node, name='ens3')
+        address = str(IPNetwork(cidr).ip)
+        ipv4_ip = eth0.ip_addresses.get(ip=address)
+        self.assertThat(
+            ipv4_ip,
+            MatchesStructure.byEquality(
+                alloc_type=IPADDRESS_TYPE.DISCOVERED, subnet=subnet,
+                ip=address))
+        self.assertThat(eth0.ip_addresses.count(), Equals(1))

=== modified file 'src/metadataserver/tests/test_api_status.py'
--- src/metadataserver/tests/test_api_status.py	2015-12-01 18:12:59 +0000
+++ src/metadataserver/tests/test_api_status.py	2016-02-17 07:41:00 +0000
@@ -18,24 +18,19 @@
 from maasserver.enum import NODE_STATUS
 from maasserver.models import (
     Event,
-    Node,
     Tag,
 )
 from maasserver.testing.factory import factory
 from maasserver.testing.oauthclient import OAuthAuthenticatedClient
 from maasserver.testing.orm import reload_object
 from maasserver.testing.testcase import MAASServerTestCase
-from maastesting.matchers import (
-    MockCalledOnceWith,
-    MockNotCalled,
-)
+from maastesting.matchers import MockNotCalled
 from metadataserver import api
 from metadataserver.models import (
     NodeKey,
     NodeResult,
 )
 from metadataserver.nodeinituser import get_node_init_user
-from mock import ANY
 from provisioningserver.utils import typed
 
 
@@ -138,62 +133,6 @@
             data=urllib.parse.urlencode(payload))
         self.assertEqual(http.client.BAD_REQUEST, response.status_code)
 
-    def test_status_comissioning_success_populates_tags(self):
-        populate_tags_for_single_node = self.patch(
-            api, "populate_tags_for_single_node")
-        node = factory.make_Node(
-            interface=True, status=NODE_STATUS.COMMISSIONING)
-        client = make_node_client(node=node)
-        payload = {
-            'event_type': 'finish',
-            'result': 'SUCCESS',
-            'origin': 'curtin',
-            'name': 'cmd-install',
-            'description': 'Command Install',
-        }
-        response = call_status(client, node, payload)
-        self.assertEqual(http.client.OK, response.status_code)
-        self.assertThat(
-            populate_tags_for_single_node,
-            MockCalledOnceWith(ANY, node))
-
-    def test_status_comissioning_success_sets_default_storage_layout(self):
-        node = factory.make_Node(
-            interface=True, status=NODE_STATUS.COMMISSIONING)
-        self.patch_autospec(Node, "set_default_storage_layout")
-        client = make_node_client(node=node)
-        payload = {
-            'event_type': 'finish',
-            'result': 'SUCCESS',
-            'origin': 'curtin',
-            'name': 'cmd-install',
-            'description': 'Command Install',
-        }
-        response = call_status(client, node, payload)
-        self.assertEqual(http.client.OK, response.status_code)
-        self.assertThat(
-            Node.set_default_storage_layout,
-            MockCalledOnceWith(node))
-
-    def test_status_comissioning_success_sets_node_network_configuration(self):
-        node = factory.make_Node(
-            interface=True, status=NODE_STATUS.COMMISSIONING)
-        mock_set_initial_networking_configuration = self.patch_autospec(
-            Node, "set_initial_networking_configuration")
-        client = make_node_client(node=node)
-        payload = {
-            'event_type': 'finish',
-            'result': 'SUCCESS',
-            'origin': 'curtin',
-            'name': 'cmd-install',
-            'description': 'Command Install',
-        }
-        response = call_status(client, node, payload)
-        self.assertEqual(http.client.OK, response.status_code)
-        self.assertThat(
-            mock_set_initial_networking_configuration,
-            MockCalledOnceWith(node))
-
     def test_status_commissioning_failure_leaves_node_failed(self):
         node = factory.make_Node(
             interface=True, status=NODE_STATUS.COMMISSIONING)

=== modified file 'src/provisioningserver/utils/tests/test_ipaddr.py'
--- src/provisioningserver/utils/tests/test_ipaddr.py	2015-12-01 18:12:59 +0000
+++ src/provisioningserver/utils/tests/test_ipaddr.py	2016-02-17 07:41:00 +0000
@@ -27,6 +27,7 @@
 from testtools.matchers import (
     Contains,
     Equals,
+    Not,
 )
 
 
@@ -305,6 +306,40 @@
              '2620:1:260::1/64'],
             ip_link['eth1']['inet6'])
 
+    def test_parses_xenial_interfaces(self):
+        testdata = dedent("""
+1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN \
+group default
+    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
+    inet 127.0.0.1/8 scope host lo
+       valid_lft forever preferred_lft forever
+    inet6 ::1/128 scope host
+       valid_lft forever preferred_lft forever
+2: ens3: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP \
+group default qlen 1000
+    link/ether 52:54:00:2d:39:49 brd ff:ff:ff:ff:ff:ff
+    inet 172.16.100.108/24 brd 172.16.100.255 scope global ens3
+       valid_lft forever preferred_lft forever
+    inet6 fe80::5054:ff:fe2d:3949/64 scope link
+       valid_lft forever preferred_lft forever
+3: ens10: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN \
+group default qlen 1000
+    link/ether 52:54:00:e5:c6:6b brd ff:ff:ff:ff:ff:ff
+4: ens11: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN \
+group default qlen 1000
+    link/ether 52:54:00:ed:9f:9d brd ff:ff:ff:ff:ff:ff
+       """)
+        ip_link = parse_ip_addr(testdata)
+        self.assertEqual(2, ip_link['ens3']['index'])
+        self.assertEqual('52:54:00:2d:39:49', ip_link['ens3']['mac'])
+        self.assertEqual(['172.16.100.108/24'], ip_link['ens3']['inet'])
+        self.assertEqual(3, ip_link['ens10']['index'])
+        self.assertEqual('52:54:00:e5:c6:6b', ip_link['ens10']['mac'])
+        self.assertThat(ip_link['ens10'], Not(Contains('inet')))
+        self.assertEqual(4, ip_link['ens11']['index'])
+        self.assertEqual('52:54:00:ed:9f:9d', ip_link['ens11']['mac'])
+        self.assertThat(ip_link['ens11'], Not(Contains('inet')))
+
 
 class TestGetInterfaceType(MAASTestCase):