Merge lp:~daniel-thewatkins/cloud-init/lp1411582 into lp:~cloud-init-dev/cloud-init/trunk

[Dan Watkins]

Azure's ephemeral disks are not guaranteed to be assigned the same name by the kernel every boot. This causes problems on ~2% of Azure instances, and can be fixed by using udev rules to give us a deterministic path to mount; this patch introduces those udev rules and modifies the Azure data source to use them.

Changes to a couple of config modules were also required. In some places, they just needed to learn to dereference symlinks. In cc_mounts this wasn't sufficient because the dereferenced device would have been put in /etc/fstab (rather defeating the point of using the udev rules in the first place). A fairly hefty refactor was required to separate "is this a valid block device?" from "what shall I put in fstab?".

[lp:~daniel-thewatkins/cloud-init/lp1411582]

1120. By Scott Moser on 2015-07-16

CloudSigma: encode/decode data before communicating over the serial channel

this fixes the cloudsigma datasource when used with python3.

[Scott Moser (smoser)]

mostly fine.
how does the udev rule get installed? i guess i'd like packaging in trunk updated too.

[lp:~daniel-thewatkins/cloud-init/lp1411582]

1121. By Dan Watkins on 2015-07-20

Return a sensible value for DataSourceGCE.availability_zone.

1122. By Dan Watkins on 2015-07-21

Refactor cc_mounts.sanitize_devname to make it easier to modify.

1123. By Dan Watkins on 2015-07-21

Extend disk_setup and mounts to handle /dev/disk symlinks.

1124. By Dan Watkins on 2015-07-21

Use /dev/disk devices for Azure ephemeral disk.

The ephemeral disk will not necessarily be assigned the same name at
each boot (LP: #1411582), so we use some udev rules to ensure we always
get the right one.

1125. By Dan Watkins on 2015-07-21

Add udev rules for Azure ephemeral disks.

And install them in the Debian packaging.

[Dan Watkins (daniel-thewatkins)]

Added that comment and updated the Debian packaging. No idea about RPM packaging, so can't do anything there.

=== modified file 'cloudinit/config/cc_disk_setup.py'
--- cloudinit/config/cc_disk_setup.py	2015-03-18 13:33:12 +0000
+++ cloudinit/config/cc_disk_setup.py	2015-07-21 12:06:21 +0000
@@ -648,6 +648,8 @@
                 table_type: Which partition table to use, defaults to MBR
                 device: the device to work on.
     """
+    LOG.debug('Ensuring that we have a real device, not a symbolic link')
+    device = os.path.realpath(device)
 
     LOG.debug("Checking values for %s definition" % device)
     overwrite = definition.get('overwrite', False)
@@ -745,6 +747,9 @@
     fs_replace = fs_cfg.get('replace_fs', False)
     overwrite = fs_cfg.get('overwrite', False)
 
+    LOG.debug('Ensuring that we have a real device, not a symbolic link')
+    device = os.path.realpath(device)
+
     # This allows you to define the default ephemeral or swap
     LOG.debug("Checking %s against default devices", device)
 

=== modified file 'cloudinit/config/cc_mounts.py'
--- cloudinit/config/cc_mounts.py	2014-10-01 19:58:30 +0000
+++ cloudinit/config/cc_mounts.py	2015-07-21 12:06:21 +0000
@@ -28,15 +28,15 @@
 from cloudinit import util
 
 # Shortname matches 'sda', 'sda1', 'xvda', 'hda', 'sdb', xvdb, vda, vdd1, sr0
-SHORTNAME_FILTER = r"^([x]{0,1}[shv]d[a-z][0-9]*|sr[0-9]+)$"
-SHORTNAME = re.compile(SHORTNAME_FILTER)
+DEVICE_NAME_FILTER = r"^([x]{0,1}[shv]d[a-z][0-9]*|sr[0-9]+)$"
+DEVICE_NAME_RE = re.compile(DEVICE_NAME_FILTER)
 WS = re.compile("[%s]+" % (whitespace))
 FSTAB_PATH = "/etc/fstab"
 
 LOG = logging.getLogger(__name__)
 
 
-def is_mdname(name):
+def is_meta_device_name(name):
     # return true if this is a metadata service name
     if name in ["ami", "root", "swap"]:
         return True
@@ -48,6 +48,25 @@
     return False
 
 
+def _get_nth_partition_for_device(device_path, partition_number):
+    potential_suffixes = [str(partition_number), 'p%s' % (partition_number,),
+                          '-part%s' % (partition_number,)]
+    for suffix in potential_suffixes:
+        potential_partition_device = '%s%s' % (device_path, suffix)
+        if os.path.exists(potential_partition_device):
+            return potential_partition_device
+    return None
+
+
+def _is_block_device(device_path, partition_path=None):
+    device_name = os.path.realpath(device_path).split('/')[-1]
+    sys_path = os.path.join('/sys/block/', device_name)
+    if partition_path is not None:
+        sys_path = os.path.join(
+            sys_path, os.path.realpath(partition_path).split('/')[-1])
+    return os.path.exists(sys_path)
+
+
 def sanitize_devname(startname, transformer, log):
     log.debug("Attempting to determine the real name of %s", startname)
 
@@ -58,21 +77,34 @@
         devname = "ephemeral0"
         log.debug("Adjusted mount option from ephemeral to ephemeral0")
 
-    (blockdev, part) = util.expand_dotted_devname(devname)
-
-    if is_mdname(blockdev):
-        orig = blockdev
-        blockdev = transformer(blockdev)
-        if not blockdev:
-            return None
-        if not blockdev.startswith("/"):
-            blockdev = "/dev/%s" % blockdev
-        log.debug("Mapped metadata name %s to %s", orig, blockdev)
-    else:
-        if SHORTNAME.match(startname):
-            blockdev = "/dev/%s" % blockdev
-
-    return devnode_for_dev_part(blockdev, part)
+    device_path, partition_number = util.expand_dotted_devname(devname)
+
+    if is_meta_device_name(device_path):
+        orig = device_path
+        device_path = transformer(device_path)
+        if not device_path:
+            return None
+        if not device_path.startswith("/"):
+            device_path = "/dev/%s" % (device_path,)
+        log.debug("Mapped metadata name %s to %s", orig, device_path)
+    else:
+        if DEVICE_NAME_RE.match(startname):
+            device_path = "/dev/%s" % (device_path,)
+
+    partition_path = None
+    if partition_number is None:
+        partition_path = _get_nth_partition_for_device(device_path, 1)
+    else:
+        partition_path = _get_nth_partition_for_device(device_path,
+                                                       partition_number)
+        if partition_path is None:
+            return None
+
+    if _is_block_device(device_path, partition_path):
+        if partition_path is not None:
+            return partition_path
+        return device_path
+    return None
 
 
 def suggested_swapsize(memsize=None, maxsize=None, fsys=None):
@@ -366,49 +398,3 @@
         util.subp(("mount", "-a"))
     except:
         util.logexc(log, "Activating mounts via 'mount -a' failed")
-
-
-def devnode_for_dev_part(device, partition):
-    """
-    Find the name of the partition. While this might seem rather
-    straight forward, its not since some devices are '<device><partition>'
-    while others are '<device>p<partition>'. For example, /dev/xvda3 on EC2
-    will present as /dev/xvda3p1 for the first partition since /dev/xvda3 is
-    a block device.
-    """
-    if not os.path.exists(device):
-        return None
-
-    short_name = os.path.basename(device)
-    sys_path = "/sys/block/%s" % short_name
-
-    if not os.path.exists(sys_path):
-        LOG.debug("did not find entry for %s in /sys/block", short_name)
-        return None
-
-    sys_long_path = sys_path + "/" + short_name
-
-    if partition is not None:
-        partition = str(partition)
-
-    if partition is None:
-        valid_mappings = [sys_long_path + "1", sys_long_path + "p1"]
-    elif partition != "0":
-        valid_mappings = [sys_long_path + "%s" % partition,
-                          sys_long_path + "p%s" % partition]
-    else:
-        valid_mappings = []
-
-    for cdisk in valid_mappings:
-        if not os.path.exists(cdisk):
-            continue
-
-        dev_path = "/dev/%s" % os.path.basename(cdisk)
-        if os.path.exists(dev_path):
-            return dev_path
-
-    if partition is None or partition == "0":
-        return device
-
-    LOG.debug("Did not fine partition %s for device %s", partition, device)
-    return None

=== modified file 'cloudinit/sources/DataSourceAzure.py'
--- cloudinit/sources/DataSourceAzure.py	2015-05-22 16:28:17 +0000
+++ cloudinit/sources/DataSourceAzure.py	2015-07-21 12:06:21 +0000
@@ -254,7 +254,7 @@
 
         self.metadata.update(fabric_data)
 
-        found_ephemeral = find_ephemeral_disk()
+        found_ephemeral = find_fabric_formatted_ephemeral_disk()
         if found_ephemeral:
             self.ds_cfg['disk_aliases']['ephemeral0'] = found_ephemeral
             LOG.debug("using detected ephemeral0 of %s", found_ephemeral)
@@ -276,30 +276,33 @@
     return len(fnmatch.filter(os.listdir(mp), '*[!cdrom]*'))
 
 
-def find_ephemeral_part():
-    """
-    Locate the default ephmeral0.1 device. This will be the first device
-    that has a LABEL of DEF_EPHEMERAL_LABEL and is a NTFS device. If Azure
-    gets more ephemeral devices, this logic will only identify the first
-    such device.
-    """
-    c_label_devs = util.find_devs_with("LABEL=%s" % DEF_EPHEMERAL_LABEL)
-    c_fstype_devs = util.find_devs_with("TYPE=ntfs")
-    for dev in c_label_devs:
-        if dev in c_fstype_devs:
-            return dev
+def find_fabric_formatted_ephemeral_part():
+    """
+    Locate the first fabric formatted ephemeral device.
+    """
+    potential_locations = ['/dev/disk/cloud/azure_resource-part1',
+                           '/dev/disk/azure/resource-part1']
+    device_location = None
+    for potential_location in potential_locations:
+        if os.path.exists(potential_location):
+            device_location = potential_location
+            break
+    if device_location is None:
+        return None
+    ntfs_devices = util.find_devs_with("TYPE=ntfs")
+    real_device = os.path.realpath(device_location)
+    if real_device in ntfs_devices:
+        return device_location
     return None
 
 
-def find_ephemeral_disk():
+def find_fabric_formatted_ephemeral_disk():
     """
     Get the ephemeral disk.
     """
-    part_dev = find_ephemeral_part()
-    if part_dev and str(part_dev[-1]).isdigit():
-        return part_dev[:-1]
-    elif part_dev:
-        return part_dev
+    part_dev = find_fabric_formatted_ephemeral_part()
+    if part_dev:
+        return part_dev.split('-')[0]
     return None
 
 
@@ -313,7 +316,7 @@
     new ephemeral device is detected, cloud-init overrides the default
     frequency for both disk-setup and mounts for the current boot only.
     """
-    device = find_ephemeral_part()
+    device = find_fabric_formatted_ephemeral_part()
     if not device:
         LOG.debug("no default fabric formated ephemeral0.1 found")
         return None

=== modified file 'packages/debian/dirs'
--- packages/debian/dirs	2011-02-04 21:58:24 +0000
+++ packages/debian/dirs	2015-07-21 12:06:21 +0000
@@ -3,3 +3,4 @@
 etc/init
 usr/share/doc/cloud
 etc/cloud
+lib/udev/rules.d

=== modified file 'setup.py'
--- setup.py	2015-01-21 20:28:32 +0000
+++ setup.py	2015-07-21 12:06:21 +0000
@@ -84,6 +84,7 @@
 USR = "/usr"
 ETC = "/etc"
 USR_LIB_EXEC = "/usr/lib"
+LIB = "/lib"
 if os.uname()[0] == 'FreeBSD':
     USR = "/usr/local"
     USR_LIB_EXEC = "/usr/local/lib"
@@ -167,6 +168,7 @@
             [f for f in glob('doc/examples/*') if is_f(f)]),
         (USR + '/share/doc/cloud-init/examples/seed',
             [f for f in glob('doc/examples/seed/*') if is_f(f)]),
+        (LIB + '/udev/rules.d', ['udev/66-azure-ephemeral.rules']),
     ]
     # Use a subclass for install that handles
     # adding on the right init system configuration files

=== modified file 'tests/unittests/test_datasource/test_azure.py'
--- tests/unittests/test_datasource/test_azure.py	2015-05-22 16:28:17 +0000
+++ tests/unittests/test_datasource/test_azure.py	2015-07-21 12:06:21 +0000
@@ -475,10 +475,12 @@
             mock.patch.object(DataSourceAzure, 'list_possible_azure_ds_devs',
                               mock.MagicMock(return_value=[])))
         self.patches.enter_context(
-            mock.patch.object(DataSourceAzure, 'find_ephemeral_disk',
+            mock.patch.object(DataSourceAzure,
+                              'find_fabric_formatted_ephemeral_disk',
                               mock.MagicMock(return_value=None)))
         self.patches.enter_context(
-            mock.patch.object(DataSourceAzure, 'find_ephemeral_part',
+            mock.patch.object(DataSourceAzure,
+                              'find_fabric_formatted_ephemeral_part',
                               mock.MagicMock(return_value=None)))
         self.patches.enter_context(
             mock.patch.object(DataSourceAzure, 'get_metadata_from_fabric',

=== added file 'tests/unittests/test_handler/test_handler_mounts.py'
--- tests/unittests/test_handler/test_handler_mounts.py	1970-01-01 00:00:00 +0000
+++ tests/unittests/test_handler/test_handler_mounts.py	2015-07-21 12:06:21 +0000
@@ -0,0 +1,133 @@
+import os.path
+import shutil
+import tempfile
+
+from cloudinit.config import cc_mounts
+
+from .. import helpers as test_helpers
+
+try:
+    from unittest import mock
+except ImportError:
+    import mock
+
+
+class TestSanitizeDevname(test_helpers.FilesystemMockingTestCase):
+
+    def setUp(self):
+        super(TestSanitizeDevname, self).setUp()
+        self.new_root = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, self.new_root)
+        self.patchOS(self.new_root)
+
+    def _touch(self, path):
+        path = os.path.join(self.new_root, path.lstrip('/'))
+        basedir = os.path.dirname(path)
+        if not os.path.exists(basedir):
+            os.makedirs(basedir)
+        open(path, 'a').close()
+
+    def _makedirs(self, directory):
+        directory = os.path.join(self.new_root, directory.lstrip('/'))
+        if not os.path.exists(directory):
+            os.makedirs(directory)
+
+    def mock_existence_of_disk(self, disk_path):
+        self._touch(disk_path)
+        self._makedirs(os.path.join('/sys/block', disk_path.split('/')[-1]))
+
+    def mock_existence_of_partition(self, disk_path, partition_number):
+        self.mock_existence_of_disk(disk_path)
+        self._touch(disk_path + str(partition_number))
+        disk_name = disk_path.split('/')[-1]
+        self._makedirs(os.path.join('/sys/block',
+                                    disk_name,
+                                    disk_name + str(partition_number)))
+
+    def test_existent_full_disk_path_is_returned(self):
+        disk_path = '/dev/sda'
+        self.mock_existence_of_disk(disk_path)
+        self.assertEqual(disk_path,
+                         cc_mounts.sanitize_devname(disk_path,
+                                                    lambda x: None,
+                                                    mock.Mock()))
+
+    def test_existent_disk_name_returns_full_path(self):
+        disk_name = 'sda'
+        disk_path = '/dev/' + disk_name
+        self.mock_existence_of_disk(disk_path)
+        self.assertEqual(disk_path,
+                         cc_mounts.sanitize_devname(disk_name,
+                                                    lambda x: None,
+                                                    mock.Mock()))
+
+    def test_existent_meta_disk_is_returned(self):
+        actual_disk_path = '/dev/sda'
+        self.mock_existence_of_disk(actual_disk_path)
+        self.assertEqual(
+            actual_disk_path,
+            cc_mounts.sanitize_devname('ephemeral0',
+                                       lambda x: actual_disk_path,
+                                       mock.Mock()))
+
+    def test_existent_meta_partition_is_returned(self):
+        disk_name, partition_part = '/dev/sda', '1'
+        actual_partition_path = disk_name + partition_part
+        self.mock_existence_of_partition(disk_name, partition_part)
+        self.assertEqual(
+            actual_partition_path,
+            cc_mounts.sanitize_devname('ephemeral0.1',
+                                       lambda x: disk_name,
+                                       mock.Mock()))
+
+    def test_existent_meta_partition_with_p_is_returned(self):
+        disk_name, partition_part = '/dev/sda', 'p1'
+        actual_partition_path = disk_name + partition_part
+        self.mock_existence_of_partition(disk_name, partition_part)
+        self.assertEqual(
+            actual_partition_path,
+            cc_mounts.sanitize_devname('ephemeral0.1',
+                                       lambda x: disk_name,
+                                       mock.Mock()))
+
+    def test_first_partition_returned_if_existent_disk_is_partitioned(self):
+        disk_name, partition_part = '/dev/sda', '1'
+        actual_partition_path = disk_name + partition_part
+        self.mock_existence_of_partition(disk_name, partition_part)
+        self.assertEqual(
+            actual_partition_path,
+            cc_mounts.sanitize_devname('ephemeral0',
+                                       lambda x: disk_name,
+                                       mock.Mock()))
+
+    def test_nth_partition_returned_if_requested(self):
+        disk_name, partition_part = '/dev/sda', '3'
+        actual_partition_path = disk_name + partition_part
+        self.mock_existence_of_partition(disk_name, partition_part)
+        self.assertEqual(
+            actual_partition_path,
+            cc_mounts.sanitize_devname('ephemeral0.3',
+                                       lambda x: disk_name,
+                                       mock.Mock()))
+
+    def test_transformer_returning_none_returns_none(self):
+        self.assertIsNone(
+            cc_mounts.sanitize_devname(
+                'ephemeral0', lambda x: None, mock.Mock()))
+
+    def test_missing_device_returns_none(self):
+        self.assertIsNone(
+            cc_mounts.sanitize_devname('/dev/sda', None, mock.Mock()))
+
+    def test_missing_sys_returns_none(self):
+        disk_path = '/dev/sda'
+        self._makedirs(disk_path)
+        self.assertIsNone(
+            cc_mounts.sanitize_devname(disk_path, None, mock.Mock()))
+
+    def test_existent_disk_but_missing_partition_returns_none(self):
+        disk_path = '/dev/sda'
+        self.mock_existence_of_disk(disk_path)
+        self.assertIsNone(
+            cc_mounts.sanitize_devname(
+                'ephemeral0.1', lambda x: disk_path, mock.Mock()))

=== added directory 'udev'
=== added file 'udev/66-azure-ephemeral.rules'
--- udev/66-azure-ephemeral.rules	1970-01-01 00:00:00 +0000
+++ udev/66-azure-ephemeral.rules	2015-07-21 12:06:21 +0000
@@ -0,0 +1,18 @@
+# Azure specific rules
+ACTION!="add|change", GOTO="cloud_init_end"
+SUBSYSTEM!="block", GOTO="cloud_init_end"
+ATTRS{ID_VENDOR}!="Msft", GOTO="cloud_init_end"
+ATTRS{ID_MODEL}!="Virtual_Disk", GOTO="cloud_init_end"
+
+# Root has a GUID of 0000 as the second value
+# The resource/resource has GUID of 0001 as the second value
+ATTRS{device_id}=="?00000000-0000-*", ENV{fabric_name}="azure_root", GOTO="ci_azure_names"
+ATTRS{device_id}=="?00000000-0001-*", ENV{fabric_name}="azure_resource", GOTO="ci_azure_names"
+GOTO="cloud_init_end"
+
+# Create the symlinks
+LABEL="ci_azure_names"
+ENV{DEVTYPE}=="disk", SYMLINK+="disk/cloud/$env{fabric_name}"
+ENV{DEVTYPE}=="partition", SYMLINK+="disk/cloud/$env{fabric_name}-part%n"
+
+LABEL="cloud_init_end"