Review: Needs Fixing
The OEM matcher is far too broad and must be fixed before we can proceed.
I want to see a plan that ensure that the things curthooks handles today are
handled in the OEM case.
Also, when we inevitably add more things to curthooks, how will OEM adapt to
that for these preinstalled images?
Diff comments:
> diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py
> index 4bc3614..46298bd 100644
> --- a/curtin/commands/curthooks.py
> +++ b/curtin/commands/curthooks.py
> @@ -2091,6 +2091,10 @@ def curthooks(args):
> description="Configuring Ubuntu-Core for first boot"):
> ubuntu_core_curthooks(cfg, target)
> sys.exit(0)
> + # OEM is special too, handle it.
> + elif distro.is_ubuntu_oem(target):
Please review all existing curthooks and elaborate on how you're handling those
cases in the preinstalled case. I expect some of them aren't handled, such as
the new-to-24.10 case of kdump support, and some earlier ones may also not be
handled.
> + LOG.info('Detected Ubuntu OEM image, skip hooks')
> + sys.exit(0)
>
> # user asked for target, or auto mode
> if curthooks_mode in ['auto', 'target']:
> diff --git a/curtin/distro.py b/curtin/distro.py
> index e6e13d5..c7639ad 100644
> --- a/curtin/distro.py
> +++ b/curtin/distro.py
> @@ -163,6 +163,11 @@ def is_ubuntu_core_20(target=None):
> return os.path.exists(target_path(target, 'snaps'))
>
>
> +def is_ubuntu_oem(target=None):
> + """Check if Ubuntu OEM specific file is present at target"""
> + return os.path.exists(target_path(target, '.disk/info'))
This matches all ISO!s This is far too broad and would have broken installs if
this was merged. This must be made narrow to OEM.
> +
> +
> def is_centos(target=None):
> """Check if CentOS specific file is present at target"""
> return os.path.exists(target_path(target, 'etc/centos-release'))
--
https://code.launchpad.net/~laiderlai/curtin/+git/curtin/+merge/475096
Your team curtin developers is subscribed to branch curtin:master.
--
Mailing list: https://launchpad.net/~curtin-dev
Post to : curtin-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~curtin-dev
More help : https://help.launchpad.net/ListHelp
