Merge ~oddbloke/cloud-init/+git/cloud-init:feature/driver-enablement into cloud-init:master

[Dan Watkins]

Add ubuntu_drivers config module

The ubuntu_drivers config module enables usage of the 'ubuntu-drivers'
command. At this point it only serves as a way of installing NVIDIA
drivers for general purpose graphics processing unit (GPGPU)
functionality.

Also, a small usability improvement to get_cfg_by_path to allow it to
take a string for the key path
  "toplevel/second/mykey"
in addition to the original:
  ("toplevel", "second", "mykey")

[Dan Watkins (oddbloke)]

I have tested this in a lxd container (without NVIDIA hardware available) and in a GCE instance with an NVIDIA GPGPU attached, and it behaves as expected.

(This doesn't include support for selecting versions, but I think it adds enough value without that to be worth landing by itself.)

One open question: this relies on ubuntu-drivers-common behaviour that is only present in disco currently (and is only intended to be SRU'd back as far as bionic); what should we do to avoid this running on Ubuntu systems where it can't possibly work?

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:b2515d0c1e955e2b7ec3cf61aa0d608fb48f66a7
https://jenkins.ubuntu.com/server/job/cloud-init-ci/612/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/612/rebuild

review: Approve (continuous-integration)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:b3f9b49af07505d85dbe17b7226e8222573914c3
https://jenkins.ubuntu.com/server/job/cloud-init-ci/614/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/614/rebuild

review: Approve (continuous-integration)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:9646774cc3c4bd24674087411bda86cc46e7b27d
https://jenkins.ubuntu.com/server/job/cloud-init-ci/615/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/615/rebuild

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

review: Needs Information

[Scott Moser (smoser)]

review: Needs Information

[Scott Moser (smoser)]

i approve with your judgement on the log level applied.

review: Approve

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:c9f9075bc859d0ec3bdad5165a1fa3914d7af130
https://jenkins.ubuntu.com/server/job/cloud-init-ci/642/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/642/rebuild

review: Approve (continuous-integration)

af13b69... by Dan Watkins on 2019-03-18

Modify log level per smoser's suggestion

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:af13b69f8d00652c95545c336f59aa7e22c786ed
https://jenkins.ubuntu.com/server/job/cloud-init-ci/643/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/643/rebuild

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

review: Needs Information

e8ec33e... by Dan Watkins on 2019-03-18

Add explanatory comment

[Dan Watkins (oddbloke)]

On Mon, Mar 18, 2019 at 03:13:42PM -0000, Ryan Harper wrote:
> > + nv_acc = util.translate_bool(util.get_cfg_by_path(cfg, cfgpath))
>
> Wasn't this going to be get_cfg_option_bool()?

get_cfg_option_bool doesn't support the path syntax we're using here.
(translate_bool is what get_cfg_option_bool uses itself.)

I've pushed up a comment explaning the use, but would be happy to make
more of a change if you would prefer.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:e8ec33e7d07d7be4bdc2fddd3ef164ec13d5824e
https://jenkins.ubuntu.com/server/job/cloud-init-ci/644/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/644/rebuild

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

I missed that; I just saw the config/by/path. A couple in-line nits.

review: Approve

0bdf197... by Dan Watkins on 2019-03-18

Update/add test name and docstrings

[Dan Watkins (oddbloke)]

On Mon, Mar 18, 2019 at 04:02:37PM -0000, Ryan Harper wrote:
> I missed that; I just saw the config/by/path. A couple in-line nits.

Nits addressed, I believe.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:0bdf1979bc30e45ddca71b8638e6b94e785bc302
https://jenkins.ubuntu.com/server/job/cloud-init-ci/646/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/646/rebuild

review: Approve (continuous-integration)

diff --git a/cloudinit/config/cc_ubuntu_drivers.py b/cloudinit/config/cc_ubuntu_drivers.py
new file mode 100644
index 0000000..91feb60
--- /dev/null
+++ b/cloudinit/config/cc_ubuntu_drivers.py
@@ -0,0 +1,112 @@
+# This file is part of cloud-init. See LICENSE file for license information.
+
+"""Ubuntu Drivers: Interact with third party drivers in Ubuntu."""
+
+from textwrap import dedent
+
+from cloudinit.config.schema import (
+    get_schema_doc, validate_cloudconfig_schema)
+from cloudinit import log as logging
+from cloudinit.settings import PER_INSTANCE
+from cloudinit import type_utils
+from cloudinit import util
+
+LOG = logging.getLogger(__name__)
+
+frequency = PER_INSTANCE
+distros = ['ubuntu']
+schema = {
+    'id': 'cc_ubuntu_drivers',
+    'name': 'Ubuntu Drivers',
+    'title': 'Interact with third party drivers in Ubuntu.',
+    'description': dedent("""\
+        This module interacts with the 'ubuntu-drivers' command to install
+        third party driver packages."""),
+    'distros': distros,
+    'examples': [dedent("""\
+        drivers:
+          nvidia:
+            license-accepted: true
+        """)],
+    'frequency': frequency,
+    'type': 'object',
+    'properties': {
+        'drivers': {
+            'type': 'object',
+            'additionalProperties': False,
+            'properties': {
+                'nvidia': {
+                    'type': 'object',
+                    'additionalProperties': False,
+                    'required': ['license-accepted'],
+                    'properties': {
+                        'license-accepted': {
+                            'type': 'boolean',
+                            'description': ("Do you accept the NVIDIA driver"
+                                            " license?"),
+                        },
+                        'version': {
+                            'type': 'string',
+                            'description': (
+                                'The version of the driver to install (e.g.'
+                                ' "390", "410"). Defaults to the latest'
+                                ' version.'),
+                        },
+                    },
+                },
+            },
+        },
+    },
+}
+OLD_UBUNTU_DRIVERS_STDERR_NEEDLE = (
+    "ubuntu-drivers: error: argument <command>: invalid choice: 'install'")
+
+__doc__ = get_schema_doc(schema)  # Supplement python help()
+
+
+def install_drivers(cfg, pkg_install_func):
+    if not isinstance(cfg, dict):
+        raise TypeError(
+            "'drivers' config expected dict, found '%s': %s" %
+            (type_utils.obj_name(cfg), cfg))
+
+    cfgpath = 'nvidia/license-accepted'
+    # Call translate_bool to ensure that we treat string values like "yes" as
+    # acceptance and _don't_ treat string values like "nah" as acceptance
+    # because they're True-ish
+    nv_acc = util.translate_bool(util.get_cfg_by_path(cfg, cfgpath))
+    if not nv_acc:
+        LOG.debug("Not installing NVIDIA drivers. %s=%s", cfgpath, nv_acc)
+        return
+
+    if not util.which('ubuntu-drivers'):
+        LOG.debug("'ubuntu-drivers' command not available.  "
+                  "Installing ubuntu-drivers-common")
+        pkg_install_func(['ubuntu-drivers-common'])
+
+    driver_arg = 'nvidia'
+    version_cfg = util.get_cfg_by_path(cfg, 'nvidia/version')
+    if version_cfg:
+        driver_arg += ':{}'.format(version_cfg)
+
+    LOG.debug("Installing NVIDIA drivers (%s=%s, version=%s)",
+              cfgpath, nv_acc, version_cfg if version_cfg else 'latest')
+
+    try:
+        util.subp(['ubuntu-drivers', 'install', '--gpgpu', driver_arg])
+    except util.ProcessExecutionError as exc:
+        if OLD_UBUNTU_DRIVERS_STDERR_NEEDLE in exc.stderr:
+            LOG.warning('the available version of ubuntu-drivers is'
+                        ' too old to perform requested driver installation')
+        elif 'No drivers found for installation.' in exc.stdout:
+            LOG.warning('ubuntu-drivers found no drivers for installation')
+        raise
+
+
+def handle(name, cfg, cloud, log, _args):
+    if "drivers" not in cfg:
+        log.debug("Skipping module named %s, no 'drivers' key in config", name)
+        return
+
+    validate_cloudconfig_schema(cfg, schema)
+    install_drivers(cfg['drivers'], cloud.distro.install_packages)
diff --git a/cloudinit/config/tests/test_ubuntu_drivers.py b/cloudinit/config/tests/test_ubuntu_drivers.py
new file mode 100644
index 0000000..efba4ce
--- /dev/null
+++ b/cloudinit/config/tests/test_ubuntu_drivers.py
@@ -0,0 +1,174 @@
+# This file is part of cloud-init. See LICENSE file for license information.
+
+import copy
+
+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
+
+MPATH = "cloudinit.config.cc_ubuntu_drivers."
+OLD_UBUNTU_DRIVERS_ERROR_STDERR = (
+    "ubuntu-drivers: error: argument <command>: invalid choice: 'install' "
+    "(choose from 'list', 'autoinstall', 'devices', 'debug')\n")
+
+
+class TestUbuntuDrivers(CiTestCase):
+    cfg_accepted = {'drivers': {'nvidia': {'license-accepted': True}}}
+    install_gpgpu = ['ubuntu-drivers', 'install', '--gpgpu', 'nvidia']
+
+    with_logs = True
+
+    @skipUnlessJsonSchema()
+    def test_schema_requires_boolean_for_license_accepted(self):
+        with self.assertRaisesRegex(
+                SchemaValidationError, ".*license-accepted.*TRUE.*boolean"):
+            validate_cloudconfig_schema(
+                {'drivers': {'nvidia': {'license-accepted': "TRUE"}}},
+                schema=drivers.schema, strict=True)
+
+    @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):
+        """Positive path test through handle. Package should be installed."""
+        myCloud = mock.MagicMock()
+        drivers.handle('ubuntu_drivers', config, myCloud, None, None)
+        self.assertEqual([mock.call(['ubuntu-drivers-common'])],
+                         myCloud.distro.install_packages.call_args_list)
+        self.assertEqual([mock.call(self.install_gpgpu)],
+                         m_subp.call_args_list)
+
+    def test_handle_does_package_install(self):
+        self._assert_happy_path_taken(self.cfg_accepted)
+
+    def test_trueish_strings_are_considered_approval(self):
+        for true_value in ['yes', 'true', 'on', '1']:
+            new_config = copy.deepcopy(self.cfg_accepted)
+            new_config['drivers']['nvidia']['license-accepted'] = true_value
+            self._assert_happy_path_taken(new_config)
+
+    @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):
+        """If ubuntu-drivers doesn't install any drivers, raise an error."""
+        myCloud = mock.MagicMock()
+        with self.assertRaises(Exception):
+            drivers.handle(
+                'ubuntu_drivers', self.cfg_accepted, myCloud, None, None)
+        self.assertEqual([mock.call(['ubuntu-drivers-common'])],
+                         myCloud.distro.install_packages.call_args_list)
+        self.assertEqual([mock.call(self.install_gpgpu)],
+                         m_subp.call_args_list)
+        self.assertIn('ubuntu-drivers found no drivers for installation',
+                      self.logs.getvalue())
+
+    @mock.patch(MPATH + "util.subp", return_value=('', ''))
+    @mock.patch(MPATH + "util.which", return_value=False)
+    def _assert_inert_with_config(self, config, m_which, m_subp):
+        """Helper to reduce repetition when testing negative cases"""
+        myCloud = mock.MagicMock()
+        drivers.handle('ubuntu_drivers', config, myCloud, None, None)
+        self.assertEqual(0, myCloud.distro.install_packages.call_count)
+        self.assertEqual(0, m_subp.call_count)
+
+    def test_handle_inert_if_license_not_accepted(self):
+        """Ensure we don't do anything if the license is rejected."""
+        self._assert_inert_with_config(
+            {'drivers': {'nvidia': {'license-accepted': False}}})
+
+    def test_handle_inert_if_garbage_in_license_field(self):
+        """Ensure we don't do anything if unknown text is in license field."""
+        self._assert_inert_with_config(
+            {'drivers': {'nvidia': {'license-accepted': 'garbage'}}})
+
+    def test_handle_inert_if_no_license_key(self):
+        """Ensure we don't do anything if no license key."""
+        self._assert_inert_with_config({'drivers': {'nvidia': {}}})
+
+    def test_handle_inert_if_no_nvidia_key(self):
+        """Ensure we don't do anything if other license accepted."""
+        self._assert_inert_with_config(
+            {'drivers': {'acme': {'license-accepted': True}}})
+
+    def test_handle_inert_if_string_given(self):
+        """Ensure we don't do anything if string refusal given."""
+        for false_value in ['no', 'false', 'off', '0']:
+            self._assert_inert_with_config(
+                {'drivers': {'nvidia': {'license-accepted': false_value}}})
+
+    @mock.patch(MPATH + "install_drivers")
+    def test_handle_no_drivers_does_nothing(self, m_install_drivers):
+        """If no 'drivers' key in the config, nothing should be done."""
+        myCloud = mock.MagicMock()
+        myLog = mock.MagicMock()
+        drivers.handle('ubuntu_drivers', {'foo': 'bzr'}, myCloud, myLog, None)
+        self.assertIn('Skipping module named',
+                      myLog.debug.call_args_list[0][0][0])
+        self.assertEqual(0, m_install_drivers.call_count)
+
+    @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):
+        """If 'ubuntu-drivers' is present, no package install should occur."""
+        pkg_install = mock.MagicMock()
+        drivers.install_drivers(self.cfg_accepted['drivers'],
+                                pkg_install_func=pkg_install)
+        self.assertEqual(0, pkg_install.call_count)
+        self.assertEqual([mock.call('ubuntu-drivers')],
+                         m_which.call_args_list)
+        self.assertEqual([mock.call(self.install_gpgpu)],
+                         m_subp.call_args_list)
+
+    def test_install_drivers_rejects_invalid_config(self):
+        """install_drivers should raise TypeError if not given a config dict"""
+        pkg_install = mock.MagicMock()
+        with self.assertRaisesRegex(TypeError, ".*expected dict.*"):
+            drivers.install_drivers("mystring", pkg_install_func=pkg_install)
+        self.assertEqual(0, pkg_install.call_count)
+
+    @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):
+        """Older ubuntu-drivers versions should emit message and raise error"""
+        myCloud = mock.MagicMock()
+        with self.assertRaises(Exception):
+            drivers.handle(
+                'ubuntu_drivers', self.cfg_accepted, myCloud, None, None)
+        self.assertEqual([mock.call(['ubuntu-drivers-common'])],
+                         myCloud.distro.install_packages.call_args_list)
+        self.assertEqual([mock.call(self.install_gpgpu)],
+                         m_subp.call_args_list)
+        self.assertIn('WARNING: the available version of ubuntu-drivers is'
+                      ' too old to perform requested driver installation',
+                      self.logs.getvalue())
+
+
+# Sub-class TestUbuntuDrivers to run the same test cases, but with a version
+class TestUbuntuDriversWithVersion(TestUbuntuDrivers):
+    cfg_accepted = {
+        'drivers': {'nvidia': {'license-accepted': True, 'version': '123'}}}
+    install_gpgpu = ['ubuntu-drivers', 'install', '--gpgpu', 'nvidia:123']
+
+    @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):
+        myCloud = mock.MagicMock()
+        version_none_cfg = {
+            'drivers': {'nvidia': {'license-accepted': True, 'version': None}}}
+        drivers.handle(
+            'ubuntu_drivers', version_none_cfg, myCloud, None, None)
+        self.assertEqual(
+            [mock.call(['ubuntu-drivers', 'install', '--gpgpu', 'nvidia'])],
+            m_subp.call_args_list)
+
+    def test_specifying_a_version_doesnt_override_license_acceptance(self):
+        self._assert_inert_with_config({
+            'drivers': {'nvidia': {'license-accepted': False,
+                                   'version': '123'}}
+        })
+
+# vi: ts=4 expandtab
diff --git a/cloudinit/util.py b/cloudinit/util.py
index a192091..385f231 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -703,6 +703,21 @@ def get_cfg_option_list(yobj, key, default=None):
 # get a cfg entry by its path array
 # for f['a']['b']: get_cfg_by_path(mycfg,('a','b'))
 def get_cfg_by_path(yobj, keyp, default=None):
+    """Return the value of the item at path C{keyp} in C{yobj}.
+
+    example:
+      get_cfg_by_path({'a': {'b': {'num': 4}}}, 'a/b/num') == 4
+      get_cfg_by_path({'a': {'b': {'num': 4}}}, 'c/d') == None
+
+    @param yobj: A dictionary.
+    @param keyp: A path inside yobj.  it can be a '/' delimited string,
+                 or an iterable.
+    @param default: The default to return if the path does not exist.
+    @return: The value of the item at keyp."
+        is not found."""
+
+    if isinstance(keyp, six.string_types):
+        keyp = keyp.split("/")
     cur = yobj
     for tok in keyp:
         if tok not in cur:
diff --git a/config/cloud.cfg.tmpl b/config/cloud.cfg.tmpl
index 7513176..25db43e 100644
--- a/config/cloud.cfg.tmpl
+++ b/config/cloud.cfg.tmpl
@@ -112,6 +112,9 @@ cloud_final_modules:
  - landscape
  - lxd
 {% endif %}
+{% if variant in ["ubuntu", "unknown"] %}
+ - ubuntu-drivers
+{% endif %}
 {% if variant not in ["freebsd"] %}
  - puppet
  - chef
diff --git a/doc/rtd/topics/modules.rst b/doc/rtd/topics/modules.rst
index d9720f6..3dcdd3b 100644
--- a/doc/rtd/topics/modules.rst
+++ b/doc/rtd/topics/modules.rst
@@ -54,6 +54,7 @@ Modules
 .. automodule:: cloudinit.config.cc_ssh_import_id
 .. automodule:: cloudinit.config.cc_timezone
 .. automodule:: cloudinit.config.cc_ubuntu_advantage
+.. automodule:: cloudinit.config.cc_ubuntu_drivers
 .. automodule:: cloudinit.config.cc_update_etc_hosts
 .. automodule:: cloudinit.config.cc_update_hostname
 .. automodule:: cloudinit.config.cc_users_groups
diff --git a/tests/unittests/test_handler/test_schema.py b/tests/unittests/test_handler/test_schema.py
index 1bad07f..e69a47a 100644
--- a/tests/unittests/test_handler/test_schema.py
+++ b/tests/unittests/test_handler/test_schema.py
@@ -28,6 +28,7 @@ class GetSchemaTest(CiTestCase):
                 'cc_runcmd',
                 'cc_snap',
                 'cc_ubuntu_advantage',
+                'cc_ubuntu_drivers',
                 'cc_zypper_add_repo'
             ],
             [subschema['id'] for subschema in schema['allOf']])