cloud-init-dev team mailing list archive

[Chad Smith <chad.smith@xxxxxxxxxxxxx>]

Chad Smith has proposed merging ~chad.smith/cloud-init:bug/1840080-ubuntu-drivers-emit-latelink into cloud-init:master.

Commit message:
ubuntu-drivers: emit latelink=true to /etc/default/ to accept nvidia eula

To accept NVIDIA EULA, cloud-init needs to emit latelink=true to the INI
file /etc/default/linux-modules-nvidia prior to installing nvidia drivers
with the ubuntu-drivers command. This will allow NVIDIA modules
prior to installing drivers enabled for linking to the running kernel.

LP: #1840080

Requested reviews:
  cloud-init commiters (cloud-init-dev)
Related bugs:
  Bug #1840080 in cloud-init (Ubuntu): "cloud-init cc_ubuntu_drivers does not set up /etc/default/linux-modules-nvidia"
  https://bugs.launchpad.net/ubuntu/+source/cloud-init/+bug/1840080

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

diff --git a/cloudinit/config/cc_ubuntu_drivers.py b/cloudinit/config/cc_ubuntu_drivers.py
index 91feb60..593b1b0 100644
--- a/cloudinit/config/cc_ubuntu_drivers.py
+++ b/cloudinit/config/cc_ubuntu_drivers.py
@@ -2,6 +2,7 @@
 
 """Ubuntu Drivers: Interact with third party drivers in Ubuntu."""
 
+import os
 from textwrap import dedent
 
 from cloudinit.config.schema import (
@@ -61,9 +62,48 @@ schema = {
 OLD_UBUNTU_DRIVERS_STDERR_NEEDLE = (
     "ubuntu-drivers: error: argument <command>: invalid choice: 'install'")
 
+ETC_DEFAULT_FILE_NVIDIA='/etc/default/linux-modules-nvidia'
+
 __doc__ = get_schema_doc(schema)  # Supplement python help()
 
 
+def ammend_driver_defaults(config, config_file):
+    """Update INI-type config_file with config values provided.
+
+    Create config_file if it doesn't exist.
+
+    Other tools in  linux-restricted-modules cloud have been executed to write
+    the config_file. So, preserve any pre-existing content and updating
+    config keys to new values if already present.
+
+    Append config key=value lines if key is not yet in
+    config_file.
+
+    @param config: Dict of key value pairs to write or update in config_file
+    @param config_file: path to defaults file.
+    """
+    if not config_file:
+        config_file = ETC_DEFAULT_FILE_NVIDIA
+    if os.path.exists(config_file):
+        lines = util.load_file(config_file).splitlines()
+        replaced_keys = set()
+        for idx, line in enumerate(lines):
+            for key in config.keys():
+                if line.startswith('{k}='.format(k=key)):
+                    lines[idx] = '{k}={v}'.format(k=key, v=config[key])
+                    replaced_keys.update([key])
+                    break
+        new_keys = set(config.keys()).difference(replaced_keys)
+        lines.extend(
+            ['{k}={v}'.format(k=k, v=config[k]) for k in new_keys] + [''])
+    else:
+        lines = [
+            '{k}={v}'.format(k=k, v=v) for (k, v) in sorted(config.items())]
+        lines.insert(0, '# Written by cloud-init #cloud-config')
+        lines.append('')
+    util.write_file(config_file, '\n'.join(lines))
+
+
 def install_drivers(cfg, pkg_install_func):
     if not isinstance(cfg, dict):
         raise TypeError(
@@ -92,6 +132,8 @@ def install_drivers(cfg, pkg_install_func):
     LOG.debug("Installing NVIDIA drivers (%s=%s, version=%s)",
               cfgpath, nv_acc, version_cfg if version_cfg else 'latest')
 
+    ammend_driver_defaults({'latelink':'true'}, ETC_DEFAULT_FILE_NVIDIA)
+
     try:
         util.subp(['ubuntu-drivers', 'install', '--gpgpu', driver_arg])
     except util.ProcessExecutionError as exc:
diff --git a/cloudinit/config/tests/test_ubuntu_drivers.py b/cloudinit/config/tests/test_ubuntu_drivers.py
index efba4ce..73f6fda 100644
--- a/cloudinit/config/tests/test_ubuntu_drivers.py
+++ b/cloudinit/config/tests/test_ubuntu_drivers.py
@@ -6,7 +6,7 @@ from cloudinit.tests.helpers import CiTestCase, skipUnlessJsonSchema, mock
 from cloudinit.config.schema import (
     SchemaValidationError, validate_cloudconfig_schema)
 from cloudinit.config import cc_ubuntu_drivers as drivers
-from cloudinit.util import ProcessExecutionError
+from cloudinit.util import ProcessExecutionError, load_file, write_file
 
 MPATH = "cloudinit.config.cc_ubuntu_drivers."
 OLD_UBUNTU_DRIVERS_ERROR_STDERR = (
@@ -14,6 +14,26 @@ OLD_UBUNTU_DRIVERS_ERROR_STDERR = (
     "(choose from 'list', 'autoinstall', 'devices', 'debug')\n")
 
 
+class TestAmmendDriverDefaults(CiTestCase):
+
+    def test_write_new_file_sorted_if_absent(self):
+        """Create config_file with cloud-init header comment if absent."""
+        outfile = self.tmp_path('driver-config')
+        drivers.ammend_driver_defaults({'k2': 'v2', 'k1': 'v1'}, outfile)
+        expected = '# Written by cloud-init #cloud-config\nk1=v1\nk2=v2\n'
+        self.assertEqual(expected, load_file(outfile))
+
+    def test_ammend_keys_if_present(self):
+        """If config keys are already present, ammend the key value."""
+        existing = '# preexisting file content\nk2=oldv2\nk1=oldv1\n'
+        outfile = self.tmp_path('driver-config')
+        write_file(outfile, existing)
+        drivers.ammend_driver_defaults({'k2': 'newv2', 'k3': 'v3'}, outfile)
+        expected = existing.replace('k2=oldv2', 'k2=newv2')
+        expected += 'k3=v3\n'
+        self.assertEqual(expected, load_file(outfile))
+
+
 class TestUbuntuDrivers(CiTestCase):
     cfg_accepted = {'drivers': {'nvidia': {'license-accepted': True}}}
     install_gpgpu = ['ubuntu-drivers', 'install', '--gpgpu', 'nvidia']
@@ -28,9 +48,10 @@ class TestUbuntuDrivers(CiTestCase):
                 {'drivers': {'nvidia': {'license-accepted': "TRUE"}}},
                 schema=drivers.schema, strict=True)
 
+    @mock.patch(MPATH + "ammend_driver_defaults")
     @mock.patch(MPATH + "util.subp", return_value=('', ''))
     @mock.patch(MPATH + "util.which", return_value=False)
-    def _assert_happy_path_taken(self, config, m_which, m_subp):
+    def _assert_happy_path_taken(self, config, m_which, m_subp, m_ammend_cfg):
         """Positive path test through handle. Package should be installed."""
         myCloud = mock.MagicMock()
         drivers.handle('ubuntu_drivers', config, myCloud, None, None)
@@ -38,6 +59,9 @@ class TestUbuntuDrivers(CiTestCase):
                          myCloud.distro.install_packages.call_args_list)
         self.assertEqual([mock.call(self.install_gpgpu)],
                          m_subp.call_args_list)
+        self.assertEqual(
+            [mock.call({'latelink': 'true'}, drivers.ETC_DEFAULT_FILE_NVIDIA)],
+          m_ammend_cfg.call_args_list)
 
     def test_handle_does_package_install(self):
         self._assert_happy_path_taken(self.cfg_accepted)
@@ -48,10 +72,11 @@ class TestUbuntuDrivers(CiTestCase):
             new_config['drivers']['nvidia']['license-accepted'] = true_value
             self._assert_happy_path_taken(new_config)
 
+    @mock.patch(MPATH + "ammend_driver_defaults")
     @mock.patch(MPATH + "util.subp", side_effect=ProcessExecutionError(
         stdout='No drivers found for installation.\n', exit_code=1))
     @mock.patch(MPATH + "util.which", return_value=False)
-    def test_handle_raises_error_if_no_drivers_found(self, m_which, m_subp):
+    def test_handle_raises_error_if_no_drivers_found(self, m_which, m_subp, _):
         """If ubuntu-drivers doesn't install any drivers, raise an error."""
         myCloud = mock.MagicMock()
         with self.assertRaises(Exception):
@@ -108,9 +133,10 @@ class TestUbuntuDrivers(CiTestCase):
                       myLog.debug.call_args_list[0][0][0])
         self.assertEqual(0, m_install_drivers.call_count)
 
+    @mock.patch(MPATH + "ammend_driver_defaults")
     @mock.patch(MPATH + "util.subp", return_value=('', ''))
     @mock.patch(MPATH + "util.which", return_value=True)
-    def test_install_drivers_no_install_if_present(self, m_which, m_subp):
+    def test_install_drivers_no_install_if_present(self, m_which, m_subp, _):
         """If 'ubuntu-drivers' is present, no package install should occur."""
         pkg_install = mock.MagicMock()
         drivers.install_drivers(self.cfg_accepted['drivers'],
@@ -128,11 +154,12 @@ class TestUbuntuDrivers(CiTestCase):
             drivers.install_drivers("mystring", pkg_install_func=pkg_install)
         self.assertEqual(0, pkg_install.call_count)
 
+    @mock.patch(MPATH + "ammend_driver_defaults")
     @mock.patch(MPATH + "util.subp", side_effect=ProcessExecutionError(
         stderr=OLD_UBUNTU_DRIVERS_ERROR_STDERR, exit_code=2))
     @mock.patch(MPATH + "util.which", return_value=False)
     def test_install_drivers_handles_old_ubuntu_drivers_gracefully(
-            self, m_which, m_subp):
+            self, m_which, m_subp, _):
         """Older ubuntu-drivers versions should emit message and raise error"""
         myCloud = mock.MagicMock()
         with self.assertRaises(Exception):
@@ -153,9 +180,10 @@ class TestUbuntuDriversWithVersion(TestUbuntuDrivers):
         'drivers': {'nvidia': {'license-accepted': True, 'version': '123'}}}
     install_gpgpu = ['ubuntu-drivers', 'install', '--gpgpu', 'nvidia:123']
 
+    @mock.patch(MPATH + "ammend_driver_defaults")
     @mock.patch(MPATH + "util.subp", return_value=('', ''))
     @mock.patch(MPATH + "util.which", return_value=False)
-    def test_version_none_uses_latest(self, m_which, m_subp):
+    def test_version_none_uses_latest(self, m_which, m_subp, _):
         myCloud = mock.MagicMock()
         version_none_cfg = {
             'drivers': {'nvidia': {'license-accepted': True, 'version': None}}}