curtin-dev team mailing list archive

[Michael Hudson-Doyle <mp+482870@xxxxxxxxxxxxxxxxxx>]

Michael Hudson-Doyle has proposed merging ~mwhudson/curtin:no-explicit-update-initramfs-at-all into curtin:master.

Commit message:
DO NOT SQUASH

Requested reviews:
  curtin developers (curtin-dev)

For more details, see:
https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/482870

so this one probably isn't a change to land at this point of the cycle. but it makes sense I think?
-- 
Your team curtin developers is requested to review the proposed merge of ~mwhudson/curtin:no-explicit-update-initramfs-at-all into curtin:master.

diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py
index 5939ab0..d4524a7 100644
--- a/curtin/commands/curthooks.py
+++ b/curtin/commands/curthooks.py
@@ -915,81 +915,6 @@ def setup_boot(
             run_zipl(cfg, target)
 
 
-def update_initramfs(target=None, all_kernels=False):
-    """ Invoke update-initramfs in the target path.
-
-    Look up the installed kernel versions in the target
-    to ensure that an initrd get created or updated as needed.
-    This allows curtin to invoke update-initramfs exactly once
-    at the end of the install instead of multiple calls.
-    """
-    if update_initramfs_is_disabled(target):
-        return
-
-    # Ensure target is resolved even if it's None
-    target = paths.target_path(target)
-
-    if util.which('update-initramfs', target=target):
-        # We keep the all_kernels flag for callers, the implementation
-        # now will operate correctly on all kernels present in the image
-        # which is almost always exactly one.
-        #
-        # Ideally curtin should be able to use update-initramfs -k all
-        # however, update-initramfs expects to be able to find out which
-        # versions of kernels are installed by using values from the
-        # kernel package invoking update-initramfs -c <kernel version>.
-        # With update-initramfs diverted, nothing captures the kernel
-        # version strings in the place where update-initramfs expects
-        # to find this information.  Instead, curtin will examine
-        # /boot to see what kernels and initramfs are installed and
-        # either create or update as needed.
-        #
-        # This loop below will examine the contents of target's
-        # /boot and pattern match for kernel files. On Ubuntu this
-        # is in the form of /boot/vmlinu[xz]-<uname -r version>.
-        #
-        # For each kernel, we extract the version string and then
-        # construct the name of of the initrd file that *would*
-        # have been created when the kernel package was installed
-        # if curtin had not diverted update-initramfs to prevent
-        # duplicate initrd creation.
-        #
-        # if the initrd file exists, then we only need to invoke
-        # update-initramfs's -u (update) method.  If the file does
-        # not exist, then we need to run the -c (create) method.
-        for _, initrd, version in paths.get_kernel_list(target):
-            # -u == update, -c == create
-            mode = '-u' if os.path.exists(initrd) else '-c'
-            cmd = ['update-initramfs', mode, '-k', version]
-            with util.ChrootableTarget(target) as in_chroot:
-                in_chroot.subp(cmd)
-                if not os.path.exists(initrd):
-                    files = os.listdir(target + '/boot')
-                    LOG.debug('Failed to find initrd %s', initrd)
-                    LOG.debug('Files in target /boot: %s', files)
-
-    elif util.which('dracut', target=target):
-        # This check is specifically intended for the Ubuntu NVMe/TCP POC.
-        # When running the POC, we install dracut and remove initramfs-tools
-        # (the packages are mutually exclusive). Therefore, trying to call
-        # update-initramfs would fail.
-        # If we were using a dpkg-divert to disable dracut (we don't do that
-        # currently), we would need to explicitly invoke dracut here instead of
-        # just returning.
-        pass
-    else:
-        # Curtin only knows update-initramfs (provided by initramfs-tools) and
-        # dracut.
-        if not list(paths.get_kernel_list(target)):
-            LOG.debug("neither update-initramfs or dracut found in target %s"
-                      " but there is no initramfs to generate, so ignoring",
-                      target)
-        else:
-            raise RuntimeError(
-                    "cannot update the initramfs: neither update-initramfs or"
-                    f" dracut found in target {target}")
-
-
 def copy_fstab(fstab, target):
     if not fstab:
         LOG.warn("fstab variable not in state, not copying fstab")
@@ -1331,11 +1256,7 @@ def detect_and_handle_multipath(cfg, target, osfamily=DISTROS.debian):
     else:
         LOG.warn("Not sure how this will boot")
 
-    if osfamily == DISTROS.debian:
-        # Initrams needs to be updated to include /etc/multipath.cfg
-        # and /etc/multipath/bindings files.
-        update_initramfs(target, all_kernels=True)
-    elif osfamily in [DISTROS.redhat, DISTROS.suse]:
+    if osfamily in [DISTROS.redhat, DISTROS.suse]:
         # Write out initramfs/dracut config for multipath
         dracut_conf_multipath = os.path.sep.join(
             [target, '/etc/dracut.conf.d/10-curtin-multipath.conf'])
@@ -2075,17 +1996,6 @@ def builtin_curthooks(cfg, target, state):
                                     osfamily=osfamily)
             copy_zkey_repository(zkey_repository, target)
 
-        # If a crypttab file was created by block_meta than it needs to be
-        # copied onto the target system, and update_initramfs() needs to be
-        # run, so that the cryptsetup hooks are properly configured on the
-        # installed system and it will be able to open encrypted volumes
-        # at boot.
-        crypttab_location = os.path.join(os.path.split(state['fstab'])[0],
-                                         "crypttab")
-        if os.path.exists(crypttab_location):
-            copy_crypttab(crypttab_location, target)
-            update_initramfs(target)
-
     # If udev dname rules were created, copy them to target
     udev_rules_d = os.path.join(state['scratch'], "rules.d")
     if os.path.isdir(udev_rules_d):
@@ -2100,22 +2010,21 @@ def builtin_curthooks(cfg, target, state):
     ):
         configure_kernel_crash_dumps(cfg, pathlib.Path(target))
 
-    with events.ReportEventStack(
-            name=stack_prefix + '/updating-initramfs-configuration',
-            reporting_enabled=True, level="INFO",
-            description="updating initramfs configuration"):
-        if osfamily == DISTROS.debian:
-            # re-enable update_initramfs
-            enable_update_initramfs(cfg, target, machine)
-            update_initramfs(target, all_kernels=True)
-        elif osfamily == DISTROS.redhat:
+    if osfamily == DISTROS.redhat:
+        with events.ReportEventStack(
+                name=stack_prefix + '/updating-initramfs-configuration',
+                reporting_enabled=True, level="INFO",
+                description="updating initramfs configuration"):
             redhat_update_initramfs(target, cfg)
 
-    with events.ReportEventStack(
-            name=stack_prefix + '/kernel-postinstall',
-            reporting_enabled=True, level="INFO",
-            description="running kernel postinstall hooks"):
-        if osfamily == DISTROS.debian:
+    if osfamily == DISTROS.debian:
+        # The kernel postinstall hooks finally create the initrd,
+        # among other things to prepare for boot into the target
+        # system (e.g. running zipl on s390x)
+        with events.ReportEventStack(
+                name=stack_prefix + '/kernel-postinstall',
+                reporting_enabled=True, level="INFO",
+                description="running kernel postinstall hooks"):
             reconfigure_kernel(target)
 
     with events.ReportEventStack(
diff --git a/tests/unittests/test_curthooks.py b/tests/unittests/test_curthooks.py
index f1bccdb..3df85c8 100644
--- a/tests/unittests/test_curthooks.py
+++ b/tests/unittests/test_curthooks.py
@@ -379,149 +379,6 @@ class TestEnableDisableUpdateInitramfs(CiTestCase):
         self._test_disable_on_machine(machine, tools)
 
 
-class TestUpdateInitramfs(CiTestCase):
-    def setUp(self):
-        super(TestUpdateInitramfs, self).setUp()
-        self.add_patch('curtin.util.subp', 'mock_subp')
-        self.add_patch('curtin.util.which', 'mock_which')
-        self.add_patch('curtin.util.is_uefi_bootable', 'mock_uefi')
-        self.mock_which.return_value = self.random_string()
-        self.mock_uefi.return_value = False
-        self.target = self.tmp_dir()
-        self.boot = os.path.join(self.target, 'boot')
-        os.makedirs(self.boot)
-        self.kversion = '5.3.0-generic'
-        # create an installed kernel file
-        with open(os.path.join(self.boot, 'vmlinuz-' + self.kversion), 'w'):
-            pass
-        self.mounts = ['dev', 'proc', 'run', 'sys']
-
-    def _mnt_call(self, point):
-        target = os.path.join(self.target, point)
-        return call(['mount', '--bind', '/%s' % point, target])
-
-    def _side_eff(self, cmd_out=None, cmd_err=None):
-        if cmd_out is None:
-            cmd_out = ''
-        if cmd_err is None:
-            cmd_err = ''
-        effects = ([('mount', '')] * len(self.mounts) +
-                   [(cmd_out, cmd_err)] + [('settle', '')])
-        return effects
-
-    def _subp_calls(self, mycall):
-        pre = [self._mnt_call(point) for point in self.mounts]
-        post = [call(['udevadm', 'settle'])]
-        return pre + [mycall] + post
-
-    @patch("curtin.commands.curthooks.util.which",
-           Mock(return_value=True))
-    def test_does_nothing_if_binary_diverted(self):
-        self.mock_which.return_value = None
-        binary = 'update-initramfs'
-        dpkg_divert_output = "\n".join([
-            'diversion of foobar to wark',
-            ('local diversion of %s to %s.curtin-disabled' % (binary, binary))
-        ])
-        self.mock_subp.side_effect = (
-            iter(self._side_eff(cmd_out=dpkg_divert_output)))
-        curthooks.update_initramfs(self.target)
-        dcall = call(['dpkg-divert', '--list'], capture=True,
-                     target=self.target)
-        calls = self._subp_calls(dcall)
-        self.mock_subp.assert_has_calls(calls)
-        self.assertEqual(6, self.mock_subp.call_count)
-
-    @patch("curtin.commands.curthooks.update_initramfs_is_disabled",
-           Mock(return_value=False))
-    @patch("curtin.commands.curthooks.util.which",
-           Mock(side_effect=[False, True]))
-    def test_does_nothing_if_dracut_installed(self):
-        curthooks.update_initramfs(self.target)
-        self.mock_subp.assert_not_called()
-
-    @patch("curtin.commands.curthooks.update_initramfs_is_disabled",
-           Mock(return_value=False))
-    @patch("curtin.commands.curthooks.util.which",
-           Mock(return_value=False))
-    def test_fails_if_no_tool_to_update_initramfs(self):
-        with patch("curtin.commands.curthooks.glob.glob",
-                   return_value=["/boot/vmlinuz"]):
-            with self.assertRaises(ValueError):
-                curthooks.update_initramfs(self.target)
-
-        with patch("curtin.commands.curthooks.glob.glob", return_value=[]):
-            # Failure is ignored if there's no initramfs to generate.
-            curthooks.update_initramfs(self.target)
-
-        self.mock_subp.assert_not_called()
-
-    @patch("curtin.commands.curthooks.util.which",
-           Mock(return_value=True))
-    def test_mounts_and_runs(self):
-        # in_chroot calls to dpkg-divert, update-initramfs
-        effects = self._side_eff() * 2
-        self.mock_subp.side_effect = iter(effects)
-        curthooks.update_initramfs(self.target)
-        subp_calls = self._subp_calls(
-            call(['dpkg-divert', '--list'], capture=True, target=self.target))
-        subp_calls += self._subp_calls(
-            call(['update-initramfs', '-c', '-k', self.kversion],
-                 target=self.target))
-        self.mock_subp.assert_has_calls(subp_calls)
-        self.assertEqual(12, self.mock_subp.call_count)
-
-    @patch("curtin.commands.curthooks.util.which",
-           Mock(return_value=True))
-    def test_mounts_and_runs_for_all_kernels(self):
-        kversion2 = '5.4.0-generic'
-        with open(os.path.join(self.boot, 'vmlinuz-' + kversion2), 'w'):
-            pass
-        kversion3 = '5.4.1-ppc64le'
-        with open(os.path.join(self.boot, 'vmlinux-' + kversion3), 'w'):
-            pass
-        effects = self._side_eff() * 4
-        self.mock_subp.side_effect = iter(effects)
-        curthooks.update_initramfs(self.target, True)
-        subp_calls = self._subp_calls(
-            call(['dpkg-divert', '--list'], capture=True, target=self.target))
-        subp_calls += self._subp_calls(
-            call(['update-initramfs', '-c', '-k', kversion3],
-                 target=self.target))
-        subp_calls += self._subp_calls(
-            call(['update-initramfs', '-c', '-k', kversion2],
-                 target=self.target))
-        subp_calls += self._subp_calls(
-            call(['update-initramfs', '-c', '-k', self.kversion],
-                 target=self.target))
-        self.mock_subp.assert_has_calls(subp_calls)
-        self.assertEqual(24, self.mock_subp.call_count)
-
-    @patch("curtin.commands.curthooks.util.which",
-           Mock(return_value=True))
-    def test_calls_update_if_initrd_exists_else_create(self):
-        kversion2 = '5.2.0-generic'
-        with open(os.path.join(self.boot, 'vmlinuz-' + kversion2), 'w'):
-            pass
-        # an existing initrd
-        with open(os.path.join(self.boot, 'initrd.img-' + kversion2), 'w'):
-            pass
-
-        effects = self._side_eff() * 3
-        self.mock_subp.side_effect = iter(effects)
-        curthooks.update_initramfs(self.target, True)
-        subp_calls = self._subp_calls(
-            call(['dpkg-divert', '--list'], capture=True, target=self.target))
-        subp_calls += self._subp_calls(
-            call(['update-initramfs', '-c', '-k', self.kversion],
-                 target=self.target))
-        subp_calls += self._subp_calls(
-            call(['update-initramfs', '-u', '-k', kversion2],
-                 target=self.target))
-        self.mock_subp.assert_has_calls(subp_calls)
-        self.assertEqual(18, self.mock_subp.call_count)
-
-
 class TestSetupKernelImgConf(CiTestCase):
 
     def setUp(self):