cc_grub_dpkg: Added UEFI support (#2029)

On Debian and Ubuntu based systems the cc_grub_dpkg module
handles the needed change of the disk device name / path between
the pre created image and the real hardware system.

Currently it seems only BIOS mode is supported. This adds UEFI support as
well to change the configuration keys for UEFI.

3 files changed, 150 insertions, 49 deletions

diff --git a/cloudinit/config/cc_grub_dpkg.py b/cloudinit/config/cc_grub_dpkg.py
index 893204fa..bf8f6b65 100644
--- a/cloudinit/config/cc_grub_dpkg.py
+++ b/cloudinit/config/cc_grub_dpkg.py
@@ -46,8 +46,11 @@ meta: MetaSchema = {
             """\
             grub_dpkg:
               enabled: true
+              # BIOS mode (install_devices needs disk)
               grub-pc/install_devices: /dev/sda
               grub-pc/install_devices_empty: false
+              # EFI mode (install_devices needs partition)
+              grub-efi/install_devices: /dev/sda
             """
         )
     ],
@@ -57,7 +60,7 @@ meta: MetaSchema = {
 __doc__ = get_meta_doc(meta)
 
 
-def fetch_idevs(log):
+def fetch_idevs(log: Logger):
     """
     Fetches the /dev/disk/by-id device grub is installed to.
     Falls back to plain disk name if no by-id entry is present.
@@ -65,11 +68,19 @@ def fetch_idevs(log):
     disk = ""
     devices = []
 
+    # BIOS mode systems use /boot and the disk path,
+    # EFI mode systems use /boot/efi and the partition path.
+    probe_target = "disk"
+    probe_mount = "/boot"
+    if is_efi_booted(log):
+        probe_target = "device"
+        probe_mount = "/boot/efi"
+
     try:
         # get the root disk where the /boot directory resides.
-        disk = subp.subp(["grub-probe", "-t", "disk", "/boot"], capture=True)[
-            0
-        ].strip()
+        disk = subp.subp(
+            ["grub-probe", "-t", probe_target, probe_mount], capture=True
+        ).stdout.strip()
     except ProcessExecutionError as e:
         # grub-common may not be installed, especially on containers
         # FileNotFoundError is a nested exception of ProcessExecutionError
@@ -97,8 +108,8 @@ def fetch_idevs(log):
             subp.subp(
                 ["udevadm", "info", "--root", "--query=symlink", disk],
                 capture=True,
-            )[0]
-            .strip()
+            )
+            .stdout.strip()
             .split()
         )
     except Exception:
@@ -117,10 +128,21 @@ def fetch_idevs(log):
     return idevs
 
 
+def is_efi_booted(log: Logger) -> bool:
+    """
+    Check if the system is booted in EFI mode.
+    """
+    try:
+        return os.path.exists("/sys/firmware/efi")
+    except OSError as e:
+        log.error("Failed to determine if system is booted in EFI mode: %s", e)
+        # If we can't determine if we're booted in EFI mode, assume we're not.
+        return False
+
+
 def handle(
     name: str, cfg: Config, cloud: Cloud, log: Logger, args: list
 ) -> None:
-
     mycfg = cfg.get("grub_dpkg", cfg.get("grub-dpkg", {}))
     if not mycfg:
         mycfg = {}
@@ -130,35 +152,47 @@ def handle(
         log.debug("%s disabled by config grub_dpkg/enabled=%s", name, enabled)
         return
 
-    idevs = util.get_cfg_option_str(mycfg, "grub-pc/install_devices", None)
-    if idevs is None:
-        idevs = fetch_idevs(log)
-
-    idevs_empty = mycfg.get("grub-pc/install_devices_empty")
-    if idevs_empty is None:
-        idevs_empty = not idevs
-    elif not isinstance(idevs_empty, bool):
-        idevs_empty = util.translate_bool(idevs_empty)
-    idevs_empty = str(idevs_empty).lower()
-
-    # now idevs and idevs_empty are set to determined values
-    # or, those set by user
-
-    dconf_sel = (
-        "grub-pc grub-pc/install_devices string %s\n"
-        "grub-pc grub-pc/install_devices_empty boolean %s\n"
-        % (idevs, idevs_empty)
-    )
-
-    log.debug(
-        "Setting grub debconf-set-selections with '%s','%s'"
-        % (idevs, idevs_empty)
-    )
+    dconf_sel = get_debconf_config(mycfg, log)
+    log.debug("Setting grub debconf-set-selections with '%s'" % dconf_sel)
 
     try:
         subp.subp(["debconf-set-selections"], dconf_sel)
-    except Exception:
-        util.logexc(log, "Failed to run debconf-set-selections for grub-dpkg")
+    except Exception as e:
+        util.logexc(
+            log, "Failed to run debconf-set-selections for grub_dpkg: %s", e
+        )
 
 
-# vi: ts=4 expandtab
+def get_debconf_config(mycfg: Config, log: Logger) -> str:
+    """
+    Returns the debconf config for grub-pc or
+    grub-efi depending on the systems boot mode.
+    """
+    if is_efi_booted(log):
+        idevs = util.get_cfg_option_str(
+            mycfg, "grub-efi/install_devices", None
+        )
+
+        if idevs is None:
+            idevs = fetch_idevs(log)
+
+        return "grub-pc grub-efi/install_devices string %s\n" % idevs
+    else:
+        idevs = util.get_cfg_option_str(mycfg, "grub-pc/install_devices", None)
+        if idevs is None:
+            idevs = fetch_idevs(log)
+
+        idevs_empty = mycfg.get("grub-pc/install_devices_empty")
+        if idevs_empty is None:
+            idevs_empty = not idevs
+        elif not isinstance(idevs_empty, bool):
+            idevs_empty = util.translate_bool(idevs_empty)
+        idevs_empty = str(idevs_empty).lower()
+
+        # now idevs and idevs_empty are set to determined values
+        # or, those set by user
+        return (
+            "grub-pc grub-pc/install_devices string %s\n"
+            "grub-pc grub-pc/install_devices_empty boolean %s\n"
+            % (idevs, idevs_empty)
+        )
diff --git a/cloudinit/config/schemas/schema-cloud-config-v1.json b/cloudinit/config/schemas/schema-cloud-config-v1.json
index 10636e6d..4af89d15 100644
--- a/cloudinit/config/schemas/schema-cloud-config-v1.json
+++ b/cloudinit/config/schemas/schema-cloud-config-v1.json
@@ -1355,6 +1355,10 @@
                   "changed_description": "Use a boolean value instead."
                 }
               ]
+            },
+            "grub-efi/install_devices": {
+              "type": "string",
+              "description": "Partition to use as target for grub installation. If unspecified, ``grub-probe`` of ``/boot/efi`` will be used to find the partition"
             }
           }
         },
diff --git a/tests/unittests/config/test_cc_grub_dpkg.py b/tests/unittests/config/test_cc_grub_dpkg.py
index aa076d19..89f58bdc 100644
--- a/tests/unittests/config/test_cc_grub_dpkg.py
+++ b/tests/unittests/config/test_cc_grub_dpkg.py
@@ -11,7 +11,7 @@ from cloudinit.config.schema import (
     get_schema,
     validate_cloudconfig_schema,
 )
-from cloudinit.subp import ProcessExecutionError
+from cloudinit.subp import ProcessExecutionError, SubpResult
 from tests.unittests.helpers import does_not_raise, skipUnlessJsonSchema
 
 
@@ -21,7 +21,7 @@ class TestFetchIdevs:
     # Note: udevadm info returns devices in a large single line string
     @pytest.mark.parametrize(
         "grub_output,path_exists,expected_log_call,udevadm_output"
-        ",expected_idevs",
+        ",expected_idevs,is_efi_boot",
         [
             # Inside a container, grub not installed
             (
@@ -30,6 +30,7 @@ class TestFetchIdevs:
                 mock.call("'grub-probe' not found in $PATH"),
                 "",
                 "",
+                False,
             ),
             # Inside a container, grub installed
             (
@@ -38,10 +39,11 @@ class TestFetchIdevs:
                 mock.call("grub-probe 'failed to get canonical path'"),
                 "",
                 "",
+                False,
             ),
             # KVM Instance
             (
-                ["/dev/vda"],
+                SubpResult("/dev/vda", ""),
                 True,
                 None,
                 (
@@ -49,18 +51,20 @@ class TestFetchIdevs:
                     "/dev/disk/by-path/virtio-pci-0000:00:00.0 ",
                 ),
                 "/dev/vda",
+                False,
             ),
             # Xen Instance
             (
-                ["/dev/xvda"],
+                SubpResult("/dev/xvda", ""),
                 True,
                 None,
                 "",
                 "/dev/xvda",
+                False,
             ),
             # NVMe Hardware Instance
             (
-                ["/dev/nvme1n1"],
+                SubpResult("/dev/nvme1n1", ""),
                 True,
                 None,
                 (
@@ -69,10 +73,11 @@ class TestFetchIdevs:
                     "/dev/disk/by-path/pci-0000:00:00.0-nvme-0 ",
                 ),
                 "/dev/disk/by-id/nvme-Company_hash000",
+                False,
             ),
             # SCSI Hardware Instance
             (
-                ["/dev/sda"],
+                SubpResult("/dev/sda", ""),
                 True,
                 None,
                 (
@@ -81,9 +86,28 @@ class TestFetchIdevs:
                     "/dev/disk/by-path/pci-0000:00:00.0-scsi-0:0:0:0 ",
                 ),
                 "/dev/disk/by-id/company-user-1",
+                False,
+            ),
+            # UEFI Hardware Instance
+            (
+                SubpResult("/dev/sda2", ""),
+                True,
+                None,
+                (
+                    "/dev/disk/by-id/scsi-3500a075116e6875a "
+                    "/dev/disk/by-id/scsi-SATA_Crucial_CT525MX3_171816E6875A "
+                    "/dev/disk/by-id/scsi-0ATA_Crucial_CT525MX3_171816E6875A "
+                    "/dev/disk/by-path/pci-0000:00:17.0-ata-1 "
+                    "/dev/disk/by-id/wwn-0x500a075116e6875a "
+                    "/dev/disk/by-id/ata-Crucial_CT525MX300SSD1_171816E6875A"
+                ),
+                "/dev/disk/by-id/ata-Crucial_CT525MX300SSD1_171816E6875A-"
+                "part1",
+                True,
             ),
         ],
     )
+    @mock.patch("cloudinit.config.cc_grub_dpkg.is_efi_booted")
     @mock.patch("cloudinit.config.cc_grub_dpkg.util.logexc")
     @mock.patch("cloudinit.config.cc_grub_dpkg.os.path.exists")
     @mock.patch("cloudinit.config.cc_grub_dpkg.subp.subp")
@@ -92,18 +116,30 @@ class TestFetchIdevs:
         m_subp,
         m_exists,
         m_logexc,
+        m_efi_booted,
         grub_output,
         path_exists,
         expected_log_call,
         udevadm_output,
         expected_idevs,
+        is_efi_boot,
     ):
-        """Tests outputs from grub-probe and udevadm info against grub-dpkg"""
-        m_subp.side_effect = [grub_output, ["".join(udevadm_output)]]
+        """Tests outputs from grub-probe and udevadm info against grub_dpkg"""
+        m_subp.side_effect = [
+            grub_output,
+            SubpResult("".join(udevadm_output), ""),
+        ]
         m_exists.return_value = path_exists
+        m_efi_booted.return_value = is_efi_boot
         log = mock.Mock(spec=Logger)
+
         idevs = fetch_idevs(log)
-        assert expected_idevs == idevs
+
+        if is_efi_boot:
+            assert expected_idevs.startswith(idevs) is True
+        else:
+            assert idevs == expected_idevs
+
         if expected_log_call is not None:
             assert expected_log_call in log.debug.call_args_list
 
@@ -112,7 +148,8 @@ class TestHandle:
     """Tests cc_grub_dpkg.handle()"""
 
     @pytest.mark.parametrize(
-        "cfg_idevs,cfg_idevs_empty,fetch_idevs_output,expected_log_output",
+        "cfg_idevs,cfg_idevs_empty,fetch_idevs_output,"
+        "expected_log_output,is_uefi",
         [
             (
                 # No configuration
@@ -121,8 +158,11 @@ class TestHandle:
                 "/dev/disk/by-id/nvme-Company_hash000",
                 (
                     "Setting grub debconf-set-selections with ",
-                    "'/dev/disk/by-id/nvme-Company_hash000','false'",
+                    "'grub-pc grub-pc/install_devices string "
+                    "/dev/disk/by-id/nvme-Company_hash000\n",
+                    "grub-pc grub-pc/install_devices_empty boolean false\n'",
                 ),
+                False,
             ),
             (
                 # idevs set, idevs_empty unset
@@ -131,8 +171,10 @@ class TestHandle:
                 "/dev/sda",
                 (
                     "Setting grub debconf-set-selections with ",
-                    "'/dev/sda','false'",
+                    "'grub-pc grub-pc/install_devices string /dev/sda\n",
+                    "grub-pc grub-pc/install_devices_empty boolean false\n'",
                 ),
+                False,
             ),
             (
                 # idevs unset, idevs_empty set
@@ -141,8 +183,10 @@ class TestHandle:
                 "/dev/xvda",
                 (
                     "Setting grub debconf-set-selections with ",
-                    "'/dev/xvda','true'",
+                    "'grub-pc grub-pc/install_devices string /dev/xvda\n",
+                    "grub-pc grub-pc/install_devices_empty boolean true\n'",
                 ),
+                False,
             ),
             (
                 # idevs set, idevs_empty set
@@ -151,8 +195,10 @@ class TestHandle:
                 "/dev/disk/by-id/company-user-1",
                 (
                     "Setting grub debconf-set-selections with ",
-                    "'/dev/vda','false'",
+                    "'grub-pc grub-pc/install_devices string /dev/vda\n",
+                    "grub-pc grub-pc/install_devices_empty boolean false\n'",
                 ),
+                False,
             ),
             (
                 # idevs set, idevs_empty set
@@ -162,16 +208,31 @@ class TestHandle:
                 "",
                 (
                     "Setting grub debconf-set-selections with ",
-                    "'/dev/nvme0n1','true'",
+                    "'grub-pc grub-pc/install_devices string /dev/nvme0n1\n",
+                    "grub-pc grub-pc/install_devices_empty boolean true\n'",
+                ),
+                False,
+            ),
+            (
+                # uefi active, idevs set
+                "/dev/sda1",
+                False,
+                "/dev/sda1",
+                (
+                    "Setting grub debconf-set-selections with ",
+                    "'grub-pc grub-efi/install_devices string /dev/sda1\n'",
                 ),
+                True,
             ),
         ],
     )
     @mock.patch("cloudinit.config.cc_grub_dpkg.fetch_idevs")
     @mock.patch("cloudinit.config.cc_grub_dpkg.util.logexc")
     @mock.patch("cloudinit.config.cc_grub_dpkg.subp.subp")
+    @mock.patch("cloudinit.config.cc_grub_dpkg.is_efi_booted")
     def test_handle(
         self,
+        m_is_efi_booted,
         m_subp,
         m_logexc,
         m_fetch_idevs,
@@ -179,8 +240,10 @@ class TestHandle:
         cfg_idevs_empty,
         fetch_idevs_output,
         expected_log_output,
+        is_uefi,
     ):
         """Test setting of correct debconf database entries"""
+        m_is_efi_booted.return_value = is_uefi
         m_fetch_idevs.return_value = fetch_idevs_output
         log = mock.Mock(spec=Logger)
         cfg = {"grub_dpkg": {}}