[Merge] ~cpete/ubuntu-release-upgrader:disable-kdump-tools-quirk into ubuntu-release-upgrader:ubuntu/main

Nick Rosbrook mp+474187 at code.launchpad.net
Tue Oct 1 20:01:04 UTC 2024


Review: Needs Information

This looks good overall. Thanks for including tests! Just a few minor comments inline.

Do you have a bug number to include in the changelog?

Diff comments:

> diff --git a/DistUpgrade/DistUpgradeQuirks.py b/DistUpgrade/DistUpgradeQuirks.py
> index c6f2bb6..5885ea2 100644
> --- a/DistUpgrade/DistUpgradeQuirks.py
> +++ b/DistUpgrade/DistUpgradeQuirks.py
> @@ -1117,3 +1119,31 @@ class DistUpgradeQuirks(object):
>                  ),
>              )
>              self.controller.abort()
> +
> +    def _disable_kdump_tools_on_install(self, cache):
> +        """Disable kdump-tools if installed during upgrade."""
> +        if 'kdump-tools' not in cache:
> +            # Not installed or requested, nothing to do.
> +            return
> +
> +        pkg = cache['kdump-tools']
> +
> +        if pkg.is_installed:
> +            logging.info("kdump-tools already installed. Not disabling.")
> +            return
> +        elif pkg.marked_install:
> +            logging.info("installing kdump-tools due to upgrade. Disabling.")
> +            proc = Popen(

Isn't subprocess.run() simpler here?

> +                (
> +                    'echo "kdump-tools kdump-tools/use_kdump boolean false"'
> +                    ' | debconf-set-selections'
> +                ),
> +                shell=True,
> +            )
> +            proc.wait()
> +            logging.debug(
> +                (
> +                     "kdump-tools debconf-set-selections "
> +                     f"returned: {proc.returncode}"

Could we avoid this extra log unless the return code is non-zero?

> +                )
> +            )


-- 
https://code.launchpad.net/~cpete/ubuntu-release-upgrader/+git/ubuntu-release-upgrader/+merge/474187
Your team Ubuntu Core Development Team is subscribed to branch ubuntu-release-upgrader:ubuntu/main.




More information about the Ubuntu-reviews mailing list