curtin-dev team mailing list archive

[Dan Bungert <mp+482870@xxxxxxxxxxxxxxxxxx>]

Nice proposal.  Yes, I feel better about this in the future 25.10 branch than 25.04.  A few comments inline.

Diff comments:

> diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py
> index 5939ab0..6369e88 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):

So it seems we've picked up a dracut implementation of ensuring that initrd is updated for free.  It sounds more and more like we're on the right path.

> -        # 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:

I guess we'd want to explain why we don't have a `if debian` path here, it sounds like it would be an oversight.

> -        # 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

I wonder about this part.  Do you think the copy step is redundant?  I get the argument for not doing `update_initramfs()` here.

> -        # 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):


-- 
https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/482870
Your team curtin developers is requested to review the proposed merge of ~mwhudson/curtin:no-explicit-update-initramfs-at-all into curtin:master.