Merge ~chad.smith/cloud-init:bug/1840080-ubuntu-drivers-emit-latelink into cloud-init:master

Proposed by Chad Smith on 2019-08-15
Status: Merged
Approved by: Dan Watkins on 2019-08-19
Approved revision: 8b8401cd5f99e907a944b05e2a1caf1e889ecbbf
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~chad.smith/cloud-init:bug/1840080-ubuntu-drivers-emit-latelink
Merge into: cloud-init:master
Diff against target: 149 lines (+37/-6)
4 files modified
cloudinit/config/cc_apt_configure.py (+3/-1)
cloudinit/config/cc_ubuntu_drivers.py (+10/-0)
cloudinit/config/tests/test_ubuntu_drivers.py (+13/-5)
tests/unittests/test_handler/test_handler_apt_source_v3.py (+11/-0)
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Approve on 2019-08-19
Dan Watkins 2019-08-15 Approve on 2019-08-19
Ryan Harper Approve on 2019-08-19
Review via email: mp+371369@code.launchpad.net

Commit message

ubuntu-drivers: emit latelink=true debconf to accept nvidia eula

To accept NVIDIA EULA, cloud-init needs to emit latelink=true debconf
setting to the linux-restricted-modules package to allow NVIDIA
drivers to properly link to the running kernel.

LP: #1840080

To post a comment you must log in.

FAILED: Continuous integration, rev:a863ff8e78236c07d899e0db53d8df0fd29341b0
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1049/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

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

review: Needs Fixing (continuous-integration)
review: Needs Fixing
Ryan Harper (raharper) :
0e655fe... by Chad Smith on 2019-08-15

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 shell
config 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

Chad Smith (chad.smith) :

FAILED: Continuous integration, rev:72683d97f0b451bf03dfafa3d99bdfaf1fd3a9db
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1051/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

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

review: Needs Fixing (continuous-integration)

FAILED: Continuous integration, rev:511562748874462955e5465ee87cf6b7f14663e1
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1052/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

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

review: Needs Fixing (continuous-integration)

FAILED: Continuous integration, rev:51ad98bdf0d47d13dc0cc032ed7a355c7fc8342f
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1053/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

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

review: Needs Fixing (continuous-integration)

PASSED: Continuous integration, rev:0e655fe032c6d39d1f2732c0af168d8fafa8a277
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1055/
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/1055//rebuild

review: Approve (continuous-integration)
8b8401c... by Chad Smith on 2019-08-16

rebase solution to use debconf-set-selections instead

PASSED: Continuous integration, rev:cf2a84dc27c15f28e0e9bf18bb776c995f29fc32
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1056/
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/1056//rebuild

review: Approve (continuous-integration)
Ryan Harper (raharper) wrote :

Looks good to me.

review: Approve
Dan Watkins (daniel-thewatkins) wrote :

Overall LGTM; the one question I think we _need_ to answer is on diff-line 20. The other comment is a nice-to-have.

review: Needs Information

PASSED: Continuous integration, rev:d93d2b7c594fb6cafc8919031b39a4ba1dc2ed28
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1058/
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/1058//rebuild

review: Approve (continuous-integration)
review: Needs Fixing
Dan Watkins (daniel-thewatkins) wrote :

Thanks!

review: Approve

PASSED: Continuous integration, rev:8b8401cd5f99e907a944b05e2a1caf1e889ecbbf
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1059/
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/1059//rebuild

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cloudinit/config/cc_apt_configure.py b/cloudinit/config/cc_apt_configure.py
2index 919d199..f01e2aa 100644
3--- a/cloudinit/config/cc_apt_configure.py
4+++ b/cloudinit/config/cc_apt_configure.py
5@@ -332,6 +332,8 @@ def apply_apt(cfg, cloud, target):
6
7
8 def debconf_set_selections(selections, target=None):
9+ if not selections.endswith(b'\n'):
10+ selections += b'\n'
11 util.subp(['debconf-set-selections'], data=selections, target=target,
12 capture=True)
13
14@@ -374,7 +376,7 @@ def apply_debconf_selections(cfg, target=None):
15
16 selections = '\n'.join(
17 [selsets[key] for key in sorted(selsets.keys())])
18- debconf_set_selections(selections.encode() + b"\n", target=target)
19+ debconf_set_selections(selections.encode(), target=target)
20
21 # get a complete list of packages listed in input
22 pkgs_cfgd = set()
23diff --git a/cloudinit/config/cc_ubuntu_drivers.py b/cloudinit/config/cc_ubuntu_drivers.py
24index 91feb60..4da34ee 100644
25--- a/cloudinit/config/cc_ubuntu_drivers.py
26+++ b/cloudinit/config/cc_ubuntu_drivers.py
27@@ -4,6 +4,7 @@
28
29 from textwrap import dedent
30
31+from cloudinit.config import cc_apt_configure
32 from cloudinit.config.schema import (
33 get_schema_doc, validate_cloudconfig_schema)
34 from cloudinit import log as logging
35@@ -92,6 +93,15 @@ def install_drivers(cfg, pkg_install_func):
36 LOG.debug("Installing NVIDIA drivers (%s=%s, version=%s)",
37 cfgpath, nv_acc, version_cfg if version_cfg else 'latest')
38
39+ # Setting NVIDIA latelink confirms acceptance of EULA for the package
40+ # linux-restricted-modules
41+ # Reference code defining debconf variable is here
42+ # https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/
43+ # linux-restricted-modules/+git/eoan/tree/debian/templates/
44+ # nvidia.templates.in
45+ selections = b'linux-restricted-modules linux/nvidia/latelink boolean true'
46+ cc_apt_configure.debconf_set_selections(selections)
47+
48 try:
49 util.subp(['ubuntu-drivers', 'install', '--gpgpu', driver_arg])
50 except util.ProcessExecutionError as exc:
51diff --git a/cloudinit/config/tests/test_ubuntu_drivers.py b/cloudinit/config/tests/test_ubuntu_drivers.py
52index efba4ce..6a763bd 100644
53--- a/cloudinit/config/tests/test_ubuntu_drivers.py
54+++ b/cloudinit/config/tests/test_ubuntu_drivers.py
55@@ -28,9 +28,11 @@ class TestUbuntuDrivers(CiTestCase):
56 {'drivers': {'nvidia': {'license-accepted': "TRUE"}}},
57 schema=drivers.schema, strict=True)
58
59+ @mock.patch(MPATH + "cc_apt_configure.debconf_set_selections")
60 @mock.patch(MPATH + "util.subp", return_value=('', ''))
61 @mock.patch(MPATH + "util.which", return_value=False)
62- def _assert_happy_path_taken(self, config, m_which, m_subp):
63+ def _assert_happy_path_taken(
64+ self, config, m_which, m_subp, m_debconf_set_selections):
65 """Positive path test through handle. Package should be installed."""
66 myCloud = mock.MagicMock()
67 drivers.handle('ubuntu_drivers', config, myCloud, None, None)
68@@ -38,6 +40,8 @@ class TestUbuntuDrivers(CiTestCase):
69 myCloud.distro.install_packages.call_args_list)
70 self.assertEqual([mock.call(self.install_gpgpu)],
71 m_subp.call_args_list)
72+ m_debconf_set_selections.assert_called_with(
73+ b'linux-restricted-modules linux/nvidia/latelink boolean true')
74
75 def test_handle_does_package_install(self):
76 self._assert_happy_path_taken(self.cfg_accepted)
77@@ -48,10 +52,11 @@ class TestUbuntuDrivers(CiTestCase):
78 new_config['drivers']['nvidia']['license-accepted'] = true_value
79 self._assert_happy_path_taken(new_config)
80
81+ @mock.patch(MPATH + "cc_apt_configure.debconf_set_selections")
82 @mock.patch(MPATH + "util.subp", side_effect=ProcessExecutionError(
83 stdout='No drivers found for installation.\n', exit_code=1))
84 @mock.patch(MPATH + "util.which", return_value=False)
85- def test_handle_raises_error_if_no_drivers_found(self, m_which, m_subp):
86+ def test_handle_raises_error_if_no_drivers_found(self, m_which, m_subp, _):
87 """If ubuntu-drivers doesn't install any drivers, raise an error."""
88 myCloud = mock.MagicMock()
89 with self.assertRaises(Exception):
90@@ -108,9 +113,10 @@ class TestUbuntuDrivers(CiTestCase):
91 myLog.debug.call_args_list[0][0][0])
92 self.assertEqual(0, m_install_drivers.call_count)
93
94+ @mock.patch(MPATH + "cc_apt_configure.debconf_set_selections")
95 @mock.patch(MPATH + "util.subp", return_value=('', ''))
96 @mock.patch(MPATH + "util.which", return_value=True)
97- def test_install_drivers_no_install_if_present(self, m_which, m_subp):
98+ def test_install_drivers_no_install_if_present(self, m_which, m_subp, _):
99 """If 'ubuntu-drivers' is present, no package install should occur."""
100 pkg_install = mock.MagicMock()
101 drivers.install_drivers(self.cfg_accepted['drivers'],
102@@ -128,11 +134,12 @@ class TestUbuntuDrivers(CiTestCase):
103 drivers.install_drivers("mystring", pkg_install_func=pkg_install)
104 self.assertEqual(0, pkg_install.call_count)
105
106+ @mock.patch(MPATH + "cc_apt_configure.debconf_set_selections")
107 @mock.patch(MPATH + "util.subp", side_effect=ProcessExecutionError(
108 stderr=OLD_UBUNTU_DRIVERS_ERROR_STDERR, exit_code=2))
109 @mock.patch(MPATH + "util.which", return_value=False)
110 def test_install_drivers_handles_old_ubuntu_drivers_gracefully(
111- self, m_which, m_subp):
112+ self, m_which, m_subp, _):
113 """Older ubuntu-drivers versions should emit message and raise error"""
114 myCloud = mock.MagicMock()
115 with self.assertRaises(Exception):
116@@ -153,9 +160,10 @@ class TestUbuntuDriversWithVersion(TestUbuntuDrivers):
117 'drivers': {'nvidia': {'license-accepted': True, 'version': '123'}}}
118 install_gpgpu = ['ubuntu-drivers', 'install', '--gpgpu', 'nvidia:123']
119
120+ @mock.patch(MPATH + "cc_apt_configure.debconf_set_selections")
121 @mock.patch(MPATH + "util.subp", return_value=('', ''))
122 @mock.patch(MPATH + "util.which", return_value=False)
123- def test_version_none_uses_latest(self, m_which, m_subp):
124+ def test_version_none_uses_latest(self, m_which, m_subp, _):
125 myCloud = mock.MagicMock()
126 version_none_cfg = {
127 'drivers': {'nvidia': {'license-accepted': True, 'version': None}}}
128diff --git a/tests/unittests/test_handler/test_handler_apt_source_v3.py b/tests/unittests/test_handler/test_handler_apt_source_v3.py
129index 90fe6ee..2f21b6d 100644
130--- a/tests/unittests/test_handler/test_handler_apt_source_v3.py
131+++ b/tests/unittests/test_handler/test_handler_apt_source_v3.py
132@@ -998,6 +998,17 @@ deb http://ubuntu.com/ubuntu/ xenial-proposed main""")
133
134 class TestDebconfSelections(TestCase):
135
136+ @mock.patch("cloudinit.config.cc_apt_configure.util.subp")
137+ def test_set_sel_appends_newline_if_absent(self, m_subp):
138+ """Automatically append a newline to debconf-set-selections config."""
139+ selections = b'some/setting boolean true'
140+ cc_apt_configure.debconf_set_selections(selections=selections)
141+ cc_apt_configure.debconf_set_selections(selections=selections + b'\n')
142+ m_call = mock.call(
143+ ['debconf-set-selections'], data=selections + b'\n', capture=True,
144+ target=None)
145+ self.assertEqual([m_call, m_call], m_subp.call_args_list)
146+
147 @mock.patch("cloudinit.config.cc_apt_configure.debconf_set_selections")
148 def test_no_set_sel_if_none_to_set(self, m_set_sel):
149 cc_apt_configure.apply_debconf_selections({'foo': 'bar'})

Subscribers

People subscribed via source and target branches