cloud-init-dev team mailing list archive

[Ryan Harper <ryan.harper@xxxxxxxxxxxxx>]

some inline comments/suggestions.

Diff comments:

> diff --git a/cloudinit/config/cc_ubuntu_drivers.py b/cloudinit/config/cc_ubuntu_drivers.py
> index 4da34ee..44a2bfe 100644
> --- a/cloudinit/config/cc_ubuntu_drivers.py
> +++ b/cloudinit/config/cc_ubuntu_drivers.py
> @@ -65,6 +65,39 @@ OLD_UBUNTU_DRIVERS_STDERR_NEEDLE = (
>  __doc__ = get_schema_doc(schema)  # Supplement python help()
>  
>  
> +# debconf template to allow cloud-init pre-configure the global debconf
> +# variable linux/nvidia/latelink to true, allowing linux-restricted-modules
> +# to accept the NVIDIA EULA and automatically link drivers to the running
> +# kernel.
> +# EOL_XENIAL: can then drop this script and use python3-debconf which is only
> +# available in Bionic and later. Can't use python3-debconf currently as it
> +# isn't in Xenial and doesn't yet support X_LOADTEMPLATEFILE debconf command.
> +
> +NVIDIA_DRIVER_LATELINK_DEBCONF_TMPL = """\

TMPL makes me think of jinja or some other string replacement by the caller of this.  
Could we replace _TMPL with _SCRIPT ?

> +#/bin/sh
> +# Allow cloud-init to trigger EULA acceptance via registering a debconf
> +# template to set linux/nvidia/latelink true
> +
> +. /usr/share/debconf/confmodule
> +
> +SCRIPT=$(readlink -f "$0")
> +DIRNAME=$(dirname "$SCRIPT")
> +tmpfile=$(mktemp -p ${DIRNAME} -t "cloud-init-ubuntu-drivers-XXXXXX.template")
> +cat > "$tmpfile" << EOF
> +Template: linux/nvidia/latelink
> +Type: boolean
> +Default: true
> +Description: Late-link NVIDIA kernel modules?
> + Enable this to link the NVIDIA kernel modules in cloud-init and
> + make them available for use.
> +EOF
> +echo BEFORE $tmpfile

The script would be simpler if it required $1 to be the tmpfile and
then we could util.write_files() the script and the input file?

We already make one tempdir to hold the script, we could just write
the input file there as well and subp the script with the input file as argument?

#!/bin/sh
. /usr/share/debconf/confmodule
db_x_loadtemplatefile "$1" cloud-init

util.subp([script_name, input_file]) ?

> +db_x_loadtemplatefile "$tmpfile" cloud-init
> +echo AFTER $tmpfile
> +rm "$tmpfile"
> +"""
> +
> +
>  def install_drivers(cfg, pkg_install_func):
>      if not isinstance(cfg, dict):
>          raise TypeError(
> @@ -90,17 +123,22 @@ def install_drivers(cfg, pkg_install_func):
>      if version_cfg:
>          driver_arg += ':{}'.format(version_cfg)
>  
> -    LOG.debug("Installing NVIDIA drivers (%s=%s, version=%s)",
> +    LOG.debug("Installing and activating NVIDIA drivers (%s=%s, version=%s)",
>                cfgpath, nv_acc, version_cfg if version_cfg else 'latest')
>  
> -    # Setting NVIDIA latelink confirms acceptance of EULA for the package
> -    # linux-restricted-modules
> -    # Reference code defining debconf variable is here
> -    # https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/
> -    # linux-restricted-modules/+git/eoan/tree/debian/templates/
> -    # nvidia.templates.in
> -    selections = b'linux-restricted-modules linux/nvidia/latelink boolean true'
> -    cc_apt_configure.debconf_set_selections(selections)
> +    # Register and set debconf selection linux/nvidia/latelink = true
> +    with temp_utils.ExtendedTemporaryFile(
> +            suffix=".sh", needs_exe=True) as tmpf:
> +        try:
> +            tmpf.write(util.encode_text(NVIDIA_DRIVER_LATELINK_DEBCONF_TMPL))
> +            tmpf.flush()
> +            util.chmod(tmpf.name, 0o755)

Any reason not to use util.write_files(tmpf.name, content, mode=0o755) ?

> +            util.subp([tmpf.name])
> +        except Exception as e:
> +            util.logexc(
> +                LOG,
> +                "Failed to register NVIDIA debconf template: %s", str(e))
> +            raise
>  
>      try:
>          util.subp(['ubuntu-drivers', 'install', '--gpgpu', driver_arg])


-- 
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/371546
Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:bug/1840080-ubuntu-drivers-emit-latelink-v2 into cloud-init:master.