Implementing device_aliases as described in docs (#945)

Implement missing device_aliases feature

The device_aliases key has been documented as part of disk_setup for
years, however the feature was never implemented. This implements the
feature as documented allowing usercfg (rather than dsconfig) to create
a mapping of device names.

This is not to be confused with disk_aliases, a very similar map but
existing solely for use by datasources.

LP: #1867532

6 files changed, 225 insertions, 147 deletions

diff --git a/cloudinit/config/cc_disk_setup.py b/cloudinit/config/cc_disk_setup.py
index 22af3813..3ec49ca5 100644
--- a/cloudinit/config/cc_disk_setup.py
+++ b/cloudinit/config/cc_disk_setup.py
@@ -125,9 +125,15 @@ def handle(_name, cfg, cloud, log, _args):
     See doc/examples/cloud-config-disk-setup.txt for documentation on the
     format.
     """
+    device_aliases = cfg.get("device_aliases", {})
+
+    def alias_to_device(cand):
+        name = device_aliases.get(cand)
+        return cloud.device_name_to_device(name or cand) or name
+
     disk_setup = cfg.get("disk_setup")
     if isinstance(disk_setup, dict):
-        update_disk_setup_devices(disk_setup, cloud.device_name_to_device)
+        update_disk_setup_devices(disk_setup, alias_to_device)
         log.debug("Partitioning disks: %s", str(disk_setup))
         for disk, definition in disk_setup.items():
             if not isinstance(definition, dict):
@@ -145,7 +151,7 @@ def handle(_name, cfg, cloud, log, _args):
     fs_setup = cfg.get("fs_setup")
     if isinstance(fs_setup, list):
         log.debug("setting up filesystems: %s", str(fs_setup))
-        update_fs_setup_devices(fs_setup, cloud.device_name_to_device)
+        update_fs_setup_devices(fs_setup, alias_to_device)
         for definition in fs_setup:
             if not isinstance(definition, dict):
                 log.warning("Invalid file system definition: %s" % definition)
@@ -174,7 +180,8 @@ def update_disk_setup_devices(disk_setup, tformer):
             del disk_setup[transformed]
 
         disk_setup[transformed] = disk_setup[origname]
-        disk_setup[transformed]['_origname'] = origname
+        if isinstance(disk_setup[transformed], dict):
+            disk_setup[transformed]['_origname'] = origname
         del disk_setup[origname]
         LOG.debug("updated disk_setup device entry '%s' to '%s'",
                   origname, transformed)
diff --git a/cloudinit/config/cc_mounts.py b/cloudinit/config/cc_mounts.py
index c22d1698..eeb008d2 100644
--- a/cloudinit/config/cc_mounts.py
+++ b/cloudinit/config/cc_mounts.py
@@ -123,7 +123,7 @@ def _is_block_device(device_path, partition_path=None):
     return os.path.exists(sys_path)
 
 
-def sanitize_devname(startname, transformer, log):
+def sanitize_devname(startname, transformer, log, aliases=None):
     log.debug("Attempting to determine the real name of %s", startname)
 
     # workaround, allow user to specify 'ephemeral'
@@ -137,9 +137,14 @@ def sanitize_devname(startname, transformer, log):
         return startname
 
     device_path, partition_number = util.expand_dotted_devname(devname)
+    orig = device_path
+
+    if aliases:
+        device_path = aliases.get(device_path, device_path)
+        if orig != device_path:
+            log.debug("Mapped device alias %s to %s", orig, device_path)
 
     if is_meta_device_name(device_path):
-        orig = device_path
         device_path = transformer(device_path)
         if not device_path:
             return None
@@ -394,6 +399,8 @@ def handle(_name, cfg, cloud, log, _args):
             fstab_devs[toks[0]] = line
             fstab_lines.append(line)
 
+    device_aliases = cfg.get("device_aliases", {})
+
     for i in range(len(cfgmnt)):
         # skip something that wasn't a list
         if not isinstance(cfgmnt[i], list):
@@ -402,7 +409,8 @@ def handle(_name, cfg, cloud, log, _args):
             continue
 
         start = str(cfgmnt[i][0])
-        sanitized = sanitize_devname(start, cloud.device_name_to_device, log)
+        sanitized = sanitize_devname(start, cloud.device_name_to_device, log,
+                                     aliases=device_aliases)
         if sanitized != start:
             log.debug("changed %s => %s" % (start, sanitized))
 
@@ -444,7 +452,8 @@ def handle(_name, cfg, cloud, log, _args):
     # entry has the same device name
     for defmnt in defmnts:
         start = defmnt[0]
-        sanitized = sanitize_devname(start, cloud.device_name_to_device, log)
+        sanitized = sanitize_devname(start, cloud.device_name_to_device, log,
+                                     aliases=device_aliases)
         if sanitized != start:
             log.debug("changed default device %s => %s" % (start, sanitized))
 
diff --git a/tests/integration_tests/bugs/test_lp1920939.py b/tests/integration_tests/bugs/test_lp1920939.py
deleted file mode 100644
index 408792a6..00000000
--- a/tests/integration_tests/bugs/test_lp1920939.py
+++ /dev/null
@@ -1,140 +0,0 @@
-"""
-Test that disk setup can run successfully on a mounted partition when
-partprobe is being used.
-
-lp-1920939
-"""
-import json
-import os
-import pytest
-from uuid import uuid4
-from pycloudlib.lxd.instance import LXDInstance
-
-from cloudinit.subp import subp
-from tests.integration_tests.instances import IntegrationInstance
-
-DISK_PATH = '/tmp/test_disk_setup_{}'.format(uuid4())
-
-
-def setup_and_mount_lxd_disk(instance: LXDInstance):
-    subp('lxc config device add {} test-disk-setup-disk disk source={}'.format(
-        instance.name, DISK_PATH).split())
-
-
-@pytest.yield_fixture
-def create_disk():
-    # 640k should be enough for anybody
-    subp('dd if=/dev/zero of={} bs=1k count=640'.format(DISK_PATH).split())
-    yield
-    os.remove(DISK_PATH)
-
-
-USERDATA = """\
-#cloud-config
-disk_setup:
-  /dev/sdb:
-    table_type: mbr
-    layout: [50, 50]
-    overwrite: True
-fs_setup:
-  - label: test
-    device: /dev/sdb1
-    filesystem: ext4
-  - label: test2
-    device: /dev/sdb2
-    filesystem: ext4
-mounts:
-- ["/dev/sdb1", "/mnt1"]
-- ["/dev/sdb2", "/mnt2"]
-"""
-
-UPDATED_USERDATA = """\
-#cloud-config
-disk_setup:
-  /dev/sdb:
-    table_type: mbr
-    layout: [100]
-    overwrite: True
-fs_setup:
-  - label: test3
-    device: /dev/sdb1
-    filesystem: ext4
-mounts:
-- ["/dev/sdb1", "/mnt3"]
-"""
-
-
-def _verify_first_disk_setup(client, log):
-    assert 'Traceback' not in log
-    assert 'WARN' not in log
-    lsblk = json.loads(client.execute('lsblk --json'))
-    sdb = [x for x in lsblk['blockdevices'] if x['name'] == 'sdb'][0]
-    assert len(sdb['children']) == 2
-    assert sdb['children'][0]['name'] == 'sdb1'
-    assert sdb['children'][0]['mountpoint'] == '/mnt1'
-    assert sdb['children'][1]['name'] == 'sdb2'
-    assert sdb['children'][1]['mountpoint'] == '/mnt2'
-
-
-@pytest.mark.user_data(USERDATA)
-@pytest.mark.lxd_setup.with_args(setup_and_mount_lxd_disk)
-@pytest.mark.ubuntu
-@pytest.mark.lxd_vm
-# Not bionic or xenial because the LXD agent gets in the way of us
-# changing the userdata
-@pytest.mark.not_bionic
-@pytest.mark.not_xenial
-def test_disk_setup_when_mounted(create_disk, client: IntegrationInstance):
-    """Test lp-1920939.
-
-    We insert an extra disk into our VM, format it to have two partitions,
-    modify our cloud config to mount devices before disk setup, and modify
-    our userdata to setup a single partition on the disk.
-
-    This allows cloud-init to attempt disk setup on a mounted partition.
-    When blockdev is in use, it will fail with
-    "blockdev: ioctl error on BLKRRPART: Device or resource busy" along
-    with a warning and a traceback. When partprobe is in use, everything
-    should work successfully.
-    """
-    log = client.read_from_file('/var/log/cloud-init.log')
-    _verify_first_disk_setup(client, log)
-
-    # Update our userdata and cloud.cfg to mount then perform new disk setup
-    client.write_to_file(
-        '/var/lib/cloud/seed/nocloud-net/user-data',
-        UPDATED_USERDATA
-    )
-    client.execute("sed -i 's/write-files/write-files\\n - mounts/' "
-                   "/etc/cloud/cloud.cfg")
-
-    client.execute('cloud-init clean --logs')
-    client.restart()
-
-    # Assert new setup works as expected
-    assert 'Traceback' not in log
-    assert 'WARN' not in log
-
-    lsblk = json.loads(client.execute('lsblk --json'))
-    sdb = [x for x in lsblk['blockdevices'] if x['name'] == 'sdb'][0]
-    assert len(sdb['children']) == 1
-    assert sdb['children'][0]['name'] == 'sdb1'
-    assert sdb['children'][0]['mountpoint'] == '/mnt3'
-
-
-@pytest.mark.user_data(USERDATA)
-@pytest.mark.lxd_setup.with_args(setup_and_mount_lxd_disk)
-@pytest.mark.ubuntu
-@pytest.mark.lxd_vm
-def test_disk_setup_no_partprobe(create_disk, client: IntegrationInstance):
-    """Ensure disk setup still works as expected without partprobe."""
-    # We can't do this part in a bootcmd because the path has already
-    # been found by the time we get to the bootcmd
-    client.execute('rm $(which partprobe)')
-    client.execute('cloud-init clean --logs')
-    client.restart()
-
-    log = client.read_from_file('/var/log/cloud-init.log')
-    _verify_first_disk_setup(client, log)
-
-    assert 'partprobe' not in log
diff --git a/tests/integration_tests/modules/test_disk_setup.py b/tests/integration_tests/modules/test_disk_setup.py
new file mode 100644
index 00000000..1fc96c52
--- /dev/null
+++ b/tests/integration_tests/modules/test_disk_setup.py
@@ -0,0 +1,192 @@
+import json
+import os
+import pytest
+from uuid import uuid4
+from pycloudlib.lxd.instance import LXDInstance
+
+from cloudinit.subp import subp
+from tests.integration_tests.instances import IntegrationInstance
+
+DISK_PATH = '/tmp/test_disk_setup_{}'.format(uuid4())
+
+
+def setup_and_mount_lxd_disk(instance: LXDInstance):
+    subp('lxc config device add {} test-disk-setup-disk disk source={}'.format(
+        instance.name, DISK_PATH).split())
+
+
+@pytest.yield_fixture
+def create_disk():
+    # 640k should be enough for anybody
+    subp('dd if=/dev/zero of={} bs=1k count=640'.format(DISK_PATH).split())
+    yield
+    os.remove(DISK_PATH)
+
+
+ALIAS_USERDATA = """\
+#cloud-config
+device_aliases:
+  my_alias: /dev/sdb
+disk_setup:
+  my_alias:
+    table_type: mbr
+    layout: [50, 50]
+    overwrite: True
+fs_setup:
+- label: fs1
+  device: my_alias.1
+  filesystem: ext4
+- label: fs2
+  device: my_alias.2
+  filesystem: ext4
+mounts:
+- ["my_alias.1", "/mnt1"]
+- ["my_alias.2", "/mnt2"]
+"""
+
+
+@pytest.mark.user_data(ALIAS_USERDATA)
+@pytest.mark.lxd_setup.with_args(setup_and_mount_lxd_disk)
+@pytest.mark.ubuntu
+@pytest.mark.lxd_vm
+class TestDeviceAliases:
+    """Test devices aliases work on disk setup/mount"""
+
+    def test_device_alias(self, create_disk, client: IntegrationInstance):
+        log = client.read_from_file('/var/log/cloud-init.log')
+        assert (
+            "updated disk_setup device entry 'my_alias' to '/dev/sdb'"
+        ) in log
+        assert 'changed my_alias.1 => /dev/sdb1' in log
+        assert 'changed my_alias.2 => /dev/sdb2' in log
+        assert 'WARN' not in log
+        assert 'Traceback' not in log
+
+        lsblk = json.loads(client.execute('lsblk --json'))
+        sdb = [x for x in lsblk['blockdevices'] if x['name'] == 'sdb'][0]
+        assert len(sdb['children']) == 2
+        assert sdb['children'][0]['name'] == 'sdb1'
+        assert sdb['children'][0]['mountpoint'] == '/mnt1'
+        assert sdb['children'][1]['name'] == 'sdb2'
+        assert sdb['children'][1]['mountpoint'] == '/mnt2'
+
+
+PARTPROBE_USERDATA = """\
+#cloud-config
+disk_setup:
+  /dev/sdb:
+    table_type: mbr
+    layout: [50, 50]
+    overwrite: True
+fs_setup:
+  - label: test
+    device: /dev/sdb1
+    filesystem: ext4
+  - label: test2
+    device: /dev/sdb2
+    filesystem: ext4
+mounts:
+- ["/dev/sdb1", "/mnt1"]
+- ["/dev/sdb2", "/mnt2"]
+"""
+
+UPDATED_PARTPROBE_USERDATA = """\
+#cloud-config
+disk_setup:
+  /dev/sdb:
+    table_type: mbr
+    layout: [100]
+    overwrite: True
+fs_setup:
+  - label: test3
+    device: /dev/sdb1
+    filesystem: ext4
+mounts:
+- ["/dev/sdb1", "/mnt3"]
+"""
+
+
+@pytest.mark.user_data(PARTPROBE_USERDATA)
+@pytest.mark.lxd_setup.with_args(setup_and_mount_lxd_disk)
+@pytest.mark.ubuntu
+@pytest.mark.lxd_vm
+class TestPartProbeAvailability:
+    """Test disk setup works with partprobe
+
+    Disk setup can run successfully on a mounted partition when
+    partprobe is being used.
+
+    lp-1920939
+    """
+
+    def _verify_first_disk_setup(self, client, log):
+        assert 'Traceback' not in log
+        assert 'WARN' not in log
+        lsblk = json.loads(client.execute('lsblk --json'))
+        sdb = [x for x in lsblk['blockdevices'] if x['name'] == 'sdb'][0]
+        assert len(sdb['children']) == 2
+        assert sdb['children'][0]['name'] == 'sdb1'
+        assert sdb['children'][0]['mountpoint'] == '/mnt1'
+        assert sdb['children'][1]['name'] == 'sdb2'
+        assert sdb['children'][1]['mountpoint'] == '/mnt2'
+
+    # Not bionic or xenial because the LXD agent gets in the way of us
+    # changing the userdata
+    @pytest.mark.not_bionic
+    @pytest.mark.not_xenial
+    def test_disk_setup_when_mounted(
+        self, create_disk, client: IntegrationInstance
+    ):
+        """Test lp-1920939.
+
+        We insert an extra disk into our VM, format it to have two partitions,
+        modify our cloud config to mount devices before disk setup, and modify
+        our userdata to setup a single partition on the disk.
+
+        This allows cloud-init to attempt disk setup on a mounted partition.
+        When blockdev is in use, it will fail with
+        "blockdev: ioctl error on BLKRRPART: Device or resource busy" along
+        with a warning and a traceback. When partprobe is in use, everything
+        should work successfully.
+        """
+        log = client.read_from_file('/var/log/cloud-init.log')
+        self._verify_first_disk_setup(client, log)
+
+        # Update our userdata and cloud.cfg to mount then perform new disk
+        # setup
+        client.write_to_file(
+            '/var/lib/cloud/seed/nocloud-net/user-data',
+            UPDATED_PARTPROBE_USERDATA,
+        )
+        client.execute(
+            "sed -i 's/write-files/write-files\\n - mounts/' "
+            "/etc/cloud/cloud.cfg"
+        )
+
+        client.execute('cloud-init clean --logs')
+        client.restart()
+
+        # Assert new setup works as expected
+        assert 'Traceback' not in log
+        assert 'WARN' not in log
+
+        lsblk = json.loads(client.execute('lsblk --json'))
+        sdb = [x for x in lsblk['blockdevices'] if x['name'] == 'sdb'][0]
+        assert len(sdb['children']) == 1
+        assert sdb['children'][0]['name'] == 'sdb1'
+        assert sdb['children'][0]['mountpoint'] == '/mnt3'
+
+    def test_disk_setup_no_partprobe(
+        self, create_disk, client: IntegrationInstance
+    ):
+        """Ensure disk setup still works as expected without partprobe."""
+        # We can't do this part in a bootcmd because the path has already
+        # been found by the time we get to the bootcmd
+        client.execute('rm $(which partprobe)')
+        client.execute('cloud-init clean --logs')
+        client.restart()
+
+        log = client.read_from_file('/var/log/cloud-init.log')
+        self._verify_first_disk_setup(client, log)
+
+        assert 'partprobe' not in log
diff --git a/tests/unittests/test_handler/test_handler_mounts.py b/tests/unittests/test_handler/test_handler_mounts.py
index e87069f6..69e8b30d 100644
--- a/tests/unittests/test_handler/test_handler_mounts.py
+++ b/tests/unittests/test_handler/test_handler_mounts.py
@@ -133,6 +133,15 @@ class TestSanitizeDevname(test_helpers.FilesystemMockingTestCase):
             disk_path,
             cc_mounts.sanitize_devname(disk_path, None, mock.Mock()))
 
+    def test_device_aliases_remapping(self):
+        disk_path = '/dev/sda'
+        self.mock_existence_of_disk(disk_path)
+        self.assertEqual(disk_path,
+                         cc_mounts.sanitize_devname('mydata',
+                                                    lambda x: None,
+                                                    mock.Mock(),
+                                                    {'mydata': disk_path}))
+
 
 class TestSwapFileCreation(test_helpers.FilesystemMockingTestCase):
 
diff --git a/tools/.github-cla-signers b/tools/.github-cla-signers
index e2979ed4..3c2c6d14 100644
--- a/tools/.github-cla-signers
+++ b/tools/.github-cla-signers
@@ -32,6 +32,7 @@ klausenbusk
 landon912
 lucasmoura
 lungj
+mal
 mamercad
 manuelisimo
 marlluslustosa