Merge lp:~andreserl/maas/lp1665143_2.1 into lp:maas/2.1

[Andres Rodriguez]

When discovering a block device, select the shortest by-id path link to the device.

[Blake Rouse (blake-rouse)]

review: Needs Fixing

[Blake Rouse (blake-rouse)]

Ah my bad, was looking at the diff wrong. I see you added another test. This makes since now.

review: Approve

[MAAS Lander (maas-lander)]

The attempt to merge lp:~andreserl/maas/lp1665143_2.1 into lp:maas/2.1 failed. Below is the output from the failed tests.

Hit:1 http://prodstack-zone-1.clouds.archive.ubuntu.com/ubuntu xenial InRelease
Get:2 http://security.ubuntu.com/ubuntu xenial-security InRelease [102 kB]
Get:3 http://prodstack-zone-1.clouds.archive.ubuntu.com/ubuntu xenial-updates InRelease [102 kB]
Get:4 http://prodstack-zone-1.clouds.archive.ubuntu.com/ubuntu xenial-backports InRelease [102 kB]
Get:5 http://prodstack-zone-1.clouds.archive.ubuntu.com/ubuntu xenial-updates/universe Sources [134 kB]
Get:6 http://prodstack-zone-1.clouds.archive.ubuntu.com/ubuntu xenial-updates/universe amd64 Packages [412 kB]
Get:7 http://prodstack-zone-1.clouds.archive.ubuntu.com/ubuntu xenial-updates/universe Translation-en [156 kB]
Fetched 1,008 kB in 0s (1,692 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
    --no-install-recommends install apache2 archdetect-deb authbind avahi-utils 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 isc-dhcp-server 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-attr 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...
authbind is already the newest version (2.1.1+nmu1).
avahi-utils is already the newest version (0.6.32~rc+dfsg-1ubuntu2).
build-essential is already the newest version (12.1ubuntu2).
debhelper is already the newest version (9.20160115ubuntu3).
distro-info is already the newest version (0.14build1).
git is already the newest version (1:2.7.4-0ubuntu1).
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...

[Andres Rodriguez (andreserl)]

updated test, self approving!

review: Approve

=== modified file 'docs/changelog.rst'
--- docs/changelog.rst	2017-02-17 05:59:11 +0000
+++ docs/changelog.rst	2017-03-02 01:57:37 +0000
@@ -34,6 +34,8 @@
 
 LP: #1665459    Fix anonymous auto-enlistment to properly detect rack-facing region IP address
 
+LP: #1665143    Select the shortest by-id path when discovering block devices to address issues when discovering nvme devices.
+
 
 Other notable changes
 ---------------------

=== modified file 'src/provisioningserver/refresh/node_info_scripts.py'
--- src/provisioningserver/refresh/node_info_scripts.py	2017-02-14 15:56:02 +0000
+++ src/provisioningserver/refresh/node_info_scripts.py	2017-03-02 01:57:37 +0000
@@ -276,11 +276,17 @@
 
     def _path_to_idpath(path):
         """Searches dev_disk_byid for a device symlinked to /dev/[path]"""
+        shortest_link = None
         if os.path.exists(dev_disk_byid):
             for link in os.listdir(dev_disk_byid):
                 if os.path.exists(path) and os.path.samefile(
                         os.path.join(dev_disk_byid, link), path):
-                    return os.path.join(dev_disk_byid, link)
+                    if shortest_link is None:
+                        shortest_link = link
+                    elif len(link) < len(shortest_link):
+                        shortest_link = link
+            if shortest_link:
+                return os.path.join(dev_disk_byid, shortest_link)
         return None
 
     # Grab the block devices from lsblk. Excludes RAM devices

=== modified file 'src/provisioningserver/refresh/tests/test_node_info_scripts.py'
--- src/provisioningserver/refresh/tests/test_node_info_scripts.py	2017-02-14 15:56:02 +0000
+++ src/provisioningserver/refresh/tests/test_node_info_scripts.py	2017-03-02 01:57:37 +0000
@@ -482,6 +482,86 @@
             "RPM": "5400",
             }], self.call_gather_physical_block_devices(byidroot))
 
+    def test__returns_block_device_with_shortest_byidpath_long_first(self):
+        name = factory.make_name('name')
+        model = factory.make_name('model')
+        serial = factory.make_name('serial')
+        size = random.randint(3000 * 1000, 1000 * 1000 * 1000)
+        block_size = random.choice([512, 1024, 4096])
+        check_output = self.patch(subprocess, "check_output")
+
+        # Create simulated /dev tree
+        devroot = self.make_dir()
+        os.mkdir(os.path.join(devroot, 'disk'))
+        byidroot = os.path.join(devroot, 'disk', 'by_id')
+        os.mkdir(byidroot)
+        os.mknod(os.path.join(devroot, name))
+        os.symlink(os.path.join(devroot, name),
+                   os.path.join(byidroot, 'deviceid-long'))
+        os.symlink(os.path.join(devroot, name),
+                   os.path.join(byidroot, 'deviceid'))
+
+        check_output.side_effect = [
+            self.make_lsblk_output(name=name, model=model),
+            self.make_udevadm_output(name, serial=serial, dev=devroot),
+            b'%d' % size,
+            b'%d' % block_size,
+            ]
+        self.assertEqual([{
+            "NAME": name,
+            "PATH": os.path.join(devroot, name),
+            "ID_PATH": os.path.join(byidroot, 'deviceid'),
+            "RO": "0",
+            "RM": "0",
+            "MODEL": model,
+            "ROTA": "1",
+            "SATA": "1",
+            "SERIAL": serial,
+            "SIZE": "%s" % size,
+            "BLOCK_SIZE": "%s" % block_size,
+            "RPM": "5400",
+            }], self.call_gather_physical_block_devices(byidroot))
+
+    def test__returns_block_device_with_first_byidpath_long_second(self):
+        name = factory.make_name('name')
+        model = factory.make_name('model')
+        serial = factory.make_name('serial')
+        size = random.randint(3000 * 1000, 1000 * 1000 * 1000)
+        block_size = random.choice([512, 1024, 4096])
+        check_output = self.patch(subprocess, "check_output")
+
+        # Create simulated /dev tree
+        devroot = self.make_dir()
+        os.mkdir(os.path.join(devroot, 'disk'))
+        byidroot = os.path.join(devroot, 'disk', 'by_id')
+        os.mkdir(byidroot)
+        os.mknod(os.path.join(devroot, name))
+        os.symlink(os.path.join(devroot, name),
+                   os.path.join(byidroot, 'deviceid'))
+        os.symlink(os.path.join(devroot, name),
+                   os.path.join(byidroot, 'deviceid-longest'))
+
+        check_output.side_effect = [
+            self.make_lsblk_output(name=name, model=model),
+            self.make_udevadm_output(name, serial=serial, dev=devroot),
+            b'%d' % size,
+            b'%d' % block_size,
+            ]
+        self.assertEqual([{
+            "NAME": name,
+            "PATH": os.path.join(devroot, name),
+            "ID_PATH": os.path.join(byidroot, 'deviceid'),
+            "RO": "0",
+            "RM": "0",
+            "MODEL": model,
+            "ROTA": "1",
+            "SATA": "1",
+            "SERIAL": serial,
+            "SIZE": "%s" % size,
+            "BLOCK_SIZE": "%s" % block_size,
+            "RPM": "5400",
+            }], self.call_gather_physical_block_devices(byidroot))
+
     def test__removes_duplicate_block_device_same_serial_and_model(self):
         """Multipath disks get multiple IDs, but same serial/model is same
         device and should only be enumerated once."""