Merge ~chad.smith/cloud-init:bug/1840080-ubuntu-drivers-emit-latelink into cloud-init:master
- Git
- lp:~chad.smith/cloud-init
- bug/1840080-ubuntu-drivers-emit-latelink
- Merge into master
Status: | Merged |
---|---|
Approved by: | Dan Watkins |
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) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Server Team CI bot | continuous-integration | Approve | |
Dan Watkins | Approve | ||
Ryan Harper | Approve | ||
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-restricte
drivers to properly link to the running kernel.
LP: #1840080
Description of the change
Server Team CI bot (server-team-bot) wrote : | # |
Dan Watkins (oddbloke) : | # |
Ryan Harper (raharper) : | # |
Dan Watkins (oddbloke) : | # |
- 0e655fe... by Chad Smith
-
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) : | # |
Server Team CI bot (server-team-bot) wrote : | # |
FAILED: Continuous integration, rev:72683d97f0b
https:/
Executed test runs:
SUCCESS: Checkout
FAILED: Unit & Style Tests
Click here to trigger a rebuild:
https:/
Server Team CI bot (server-team-bot) wrote : | # |
FAILED: Continuous integration, rev:51156274887
https:/
Executed test runs:
SUCCESS: Checkout
FAILED: Unit & Style Tests
Click here to trigger a rebuild:
https:/
Server Team CI bot (server-team-bot) wrote : | # |
FAILED: Continuous integration, rev:51ad98bdf0d
https:/
Executed test runs:
SUCCESS: Checkout
FAILED: Unit & Style Tests
Click here to trigger a rebuild:
https:/
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:0e655fe032c
https:/
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:/
- 8b8401c... by Chad Smith
-
rebase solution to use debconf-
set-selections instead
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:cf2a84dc27c
https:/
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:/
Ryan Harper (raharper) wrote : | # |
Looks good to me.
Dan Watkins (oddbloke) 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.
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:d93d2b7c594
https:/
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:/
Dan Watkins (oddbloke) : | # |
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:8b8401cd5f9
https:/
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:/
Preview Diff
1 | diff --git a/cloudinit/config/cc_apt_configure.py b/cloudinit/config/cc_apt_configure.py | |||
2 | index 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 | 332 | 332 | ||
7 | 333 | 333 | ||
8 | 334 | def debconf_set_selections(selections, target=None): | 334 | def debconf_set_selections(selections, target=None): |
9 | 335 | if not selections.endswith(b'\n'): | ||
10 | 336 | selections += b'\n' | ||
11 | 335 | util.subp(['debconf-set-selections'], data=selections, target=target, | 337 | util.subp(['debconf-set-selections'], data=selections, target=target, |
12 | 336 | capture=True) | 338 | capture=True) |
13 | 337 | 339 | ||
14 | @@ -374,7 +376,7 @@ def apply_debconf_selections(cfg, target=None): | |||
15 | 374 | 376 | ||
16 | 375 | selections = '\n'.join( | 377 | selections = '\n'.join( |
17 | 376 | [selsets[key] for key in sorted(selsets.keys())]) | 378 | [selsets[key] for key in sorted(selsets.keys())]) |
19 | 377 | debconf_set_selections(selections.encode() + b"\n", target=target) | 379 | debconf_set_selections(selections.encode(), target=target) |
20 | 378 | 380 | ||
21 | 379 | # get a complete list of packages listed in input | 381 | # get a complete list of packages listed in input |
22 | 380 | pkgs_cfgd = set() | 382 | pkgs_cfgd = set() |
23 | diff --git a/cloudinit/config/cc_ubuntu_drivers.py b/cloudinit/config/cc_ubuntu_drivers.py | |||
24 | index 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 | 4 | 4 | ||
29 | 5 | from textwrap import dedent | 5 | from textwrap import dedent |
30 | 6 | 6 | ||
31 | 7 | from cloudinit.config import cc_apt_configure | ||
32 | 7 | from cloudinit.config.schema import ( | 8 | from cloudinit.config.schema import ( |
33 | 8 | get_schema_doc, validate_cloudconfig_schema) | 9 | get_schema_doc, validate_cloudconfig_schema) |
34 | 9 | from cloudinit import log as logging | 10 | from cloudinit import log as logging |
35 | @@ -92,6 +93,15 @@ def install_drivers(cfg, pkg_install_func): | |||
36 | 92 | LOG.debug("Installing NVIDIA drivers (%s=%s, version=%s)", | 93 | LOG.debug("Installing NVIDIA drivers (%s=%s, version=%s)", |
37 | 93 | cfgpath, nv_acc, version_cfg if version_cfg else 'latest') | 94 | cfgpath, nv_acc, version_cfg if version_cfg else 'latest') |
38 | 94 | 95 | ||
39 | 96 | # Setting NVIDIA latelink confirms acceptance of EULA for the package | ||
40 | 97 | # linux-restricted-modules | ||
41 | 98 | # Reference code defining debconf variable is here | ||
42 | 99 | # https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/ | ||
43 | 100 | # linux-restricted-modules/+git/eoan/tree/debian/templates/ | ||
44 | 101 | # nvidia.templates.in | ||
45 | 102 | selections = b'linux-restricted-modules linux/nvidia/latelink boolean true' | ||
46 | 103 | cc_apt_configure.debconf_set_selections(selections) | ||
47 | 104 | |||
48 | 95 | try: | 105 | try: |
49 | 96 | util.subp(['ubuntu-drivers', 'install', '--gpgpu', driver_arg]) | 106 | util.subp(['ubuntu-drivers', 'install', '--gpgpu', driver_arg]) |
50 | 97 | except util.ProcessExecutionError as exc: | 107 | except util.ProcessExecutionError as exc: |
51 | diff --git a/cloudinit/config/tests/test_ubuntu_drivers.py b/cloudinit/config/tests/test_ubuntu_drivers.py | |||
52 | index 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 | 28 | {'drivers': {'nvidia': {'license-accepted': "TRUE"}}}, | 28 | {'drivers': {'nvidia': {'license-accepted': "TRUE"}}}, |
57 | 29 | schema=drivers.schema, strict=True) | 29 | schema=drivers.schema, strict=True) |
58 | 30 | 30 | ||
59 | 31 | @mock.patch(MPATH + "cc_apt_configure.debconf_set_selections") | ||
60 | 31 | @mock.patch(MPATH + "util.subp", return_value=('', '')) | 32 | @mock.patch(MPATH + "util.subp", return_value=('', '')) |
61 | 32 | @mock.patch(MPATH + "util.which", return_value=False) | 33 | @mock.patch(MPATH + "util.which", return_value=False) |
63 | 33 | def _assert_happy_path_taken(self, config, m_which, m_subp): | 34 | def _assert_happy_path_taken( |
64 | 35 | self, config, m_which, m_subp, m_debconf_set_selections): | ||
65 | 34 | """Positive path test through handle. Package should be installed.""" | 36 | """Positive path test through handle. Package should be installed.""" |
66 | 35 | myCloud = mock.MagicMock() | 37 | myCloud = mock.MagicMock() |
67 | 36 | drivers.handle('ubuntu_drivers', config, myCloud, None, None) | 38 | drivers.handle('ubuntu_drivers', config, myCloud, None, None) |
68 | @@ -38,6 +40,8 @@ class TestUbuntuDrivers(CiTestCase): | |||
69 | 38 | myCloud.distro.install_packages.call_args_list) | 40 | myCloud.distro.install_packages.call_args_list) |
70 | 39 | self.assertEqual([mock.call(self.install_gpgpu)], | 41 | self.assertEqual([mock.call(self.install_gpgpu)], |
71 | 40 | m_subp.call_args_list) | 42 | m_subp.call_args_list) |
72 | 43 | m_debconf_set_selections.assert_called_with( | ||
73 | 44 | b'linux-restricted-modules linux/nvidia/latelink boolean true') | ||
74 | 41 | 45 | ||
75 | 42 | def test_handle_does_package_install(self): | 46 | def test_handle_does_package_install(self): |
76 | 43 | self._assert_happy_path_taken(self.cfg_accepted) | 47 | self._assert_happy_path_taken(self.cfg_accepted) |
77 | @@ -48,10 +52,11 @@ class TestUbuntuDrivers(CiTestCase): | |||
78 | 48 | new_config['drivers']['nvidia']['license-accepted'] = true_value | 52 | new_config['drivers']['nvidia']['license-accepted'] = true_value |
79 | 49 | self._assert_happy_path_taken(new_config) | 53 | self._assert_happy_path_taken(new_config) |
80 | 50 | 54 | ||
81 | 55 | @mock.patch(MPATH + "cc_apt_configure.debconf_set_selections") | ||
82 | 51 | @mock.patch(MPATH + "util.subp", side_effect=ProcessExecutionError( | 56 | @mock.patch(MPATH + "util.subp", side_effect=ProcessExecutionError( |
83 | 52 | stdout='No drivers found for installation.\n', exit_code=1)) | 57 | stdout='No drivers found for installation.\n', exit_code=1)) |
84 | 53 | @mock.patch(MPATH + "util.which", return_value=False) | 58 | @mock.patch(MPATH + "util.which", return_value=False) |
86 | 54 | def test_handle_raises_error_if_no_drivers_found(self, m_which, m_subp): | 59 | def test_handle_raises_error_if_no_drivers_found(self, m_which, m_subp, _): |
87 | 55 | """If ubuntu-drivers doesn't install any drivers, raise an error.""" | 60 | """If ubuntu-drivers doesn't install any drivers, raise an error.""" |
88 | 56 | myCloud = mock.MagicMock() | 61 | myCloud = mock.MagicMock() |
89 | 57 | with self.assertRaises(Exception): | 62 | with self.assertRaises(Exception): |
90 | @@ -108,9 +113,10 @@ class TestUbuntuDrivers(CiTestCase): | |||
91 | 108 | myLog.debug.call_args_list[0][0][0]) | 113 | myLog.debug.call_args_list[0][0][0]) |
92 | 109 | self.assertEqual(0, m_install_drivers.call_count) | 114 | self.assertEqual(0, m_install_drivers.call_count) |
93 | 110 | 115 | ||
94 | 116 | @mock.patch(MPATH + "cc_apt_configure.debconf_set_selections") | ||
95 | 111 | @mock.patch(MPATH + "util.subp", return_value=('', '')) | 117 | @mock.patch(MPATH + "util.subp", return_value=('', '')) |
96 | 112 | @mock.patch(MPATH + "util.which", return_value=True) | 118 | @mock.patch(MPATH + "util.which", return_value=True) |
98 | 113 | def test_install_drivers_no_install_if_present(self, m_which, m_subp): | 119 | def test_install_drivers_no_install_if_present(self, m_which, m_subp, _): |
99 | 114 | """If 'ubuntu-drivers' is present, no package install should occur.""" | 120 | """If 'ubuntu-drivers' is present, no package install should occur.""" |
100 | 115 | pkg_install = mock.MagicMock() | 121 | pkg_install = mock.MagicMock() |
101 | 116 | drivers.install_drivers(self.cfg_accepted['drivers'], | 122 | drivers.install_drivers(self.cfg_accepted['drivers'], |
102 | @@ -128,11 +134,12 @@ class TestUbuntuDrivers(CiTestCase): | |||
103 | 128 | drivers.install_drivers("mystring", pkg_install_func=pkg_install) | 134 | drivers.install_drivers("mystring", pkg_install_func=pkg_install) |
104 | 129 | self.assertEqual(0, pkg_install.call_count) | 135 | self.assertEqual(0, pkg_install.call_count) |
105 | 130 | 136 | ||
106 | 137 | @mock.patch(MPATH + "cc_apt_configure.debconf_set_selections") | ||
107 | 131 | @mock.patch(MPATH + "util.subp", side_effect=ProcessExecutionError( | 138 | @mock.patch(MPATH + "util.subp", side_effect=ProcessExecutionError( |
108 | 132 | stderr=OLD_UBUNTU_DRIVERS_ERROR_STDERR, exit_code=2)) | 139 | stderr=OLD_UBUNTU_DRIVERS_ERROR_STDERR, exit_code=2)) |
109 | 133 | @mock.patch(MPATH + "util.which", return_value=False) | 140 | @mock.patch(MPATH + "util.which", return_value=False) |
110 | 134 | def test_install_drivers_handles_old_ubuntu_drivers_gracefully( | 141 | def test_install_drivers_handles_old_ubuntu_drivers_gracefully( |
112 | 135 | self, m_which, m_subp): | 142 | self, m_which, m_subp, _): |
113 | 136 | """Older ubuntu-drivers versions should emit message and raise error""" | 143 | """Older ubuntu-drivers versions should emit message and raise error""" |
114 | 137 | myCloud = mock.MagicMock() | 144 | myCloud = mock.MagicMock() |
115 | 138 | with self.assertRaises(Exception): | 145 | with self.assertRaises(Exception): |
116 | @@ -153,9 +160,10 @@ class TestUbuntuDriversWithVersion(TestUbuntuDrivers): | |||
117 | 153 | 'drivers': {'nvidia': {'license-accepted': True, 'version': '123'}}} | 160 | 'drivers': {'nvidia': {'license-accepted': True, 'version': '123'}}} |
118 | 154 | install_gpgpu = ['ubuntu-drivers', 'install', '--gpgpu', 'nvidia:123'] | 161 | install_gpgpu = ['ubuntu-drivers', 'install', '--gpgpu', 'nvidia:123'] |
119 | 155 | 162 | ||
120 | 163 | @mock.patch(MPATH + "cc_apt_configure.debconf_set_selections") | ||
121 | 156 | @mock.patch(MPATH + "util.subp", return_value=('', '')) | 164 | @mock.patch(MPATH + "util.subp", return_value=('', '')) |
122 | 157 | @mock.patch(MPATH + "util.which", return_value=False) | 165 | @mock.patch(MPATH + "util.which", return_value=False) |
124 | 158 | def test_version_none_uses_latest(self, m_which, m_subp): | 166 | def test_version_none_uses_latest(self, m_which, m_subp, _): |
125 | 159 | myCloud = mock.MagicMock() | 167 | myCloud = mock.MagicMock() |
126 | 160 | version_none_cfg = { | 168 | version_none_cfg = { |
127 | 161 | 'drivers': {'nvidia': {'license-accepted': True, 'version': None}}} | 169 | 'drivers': {'nvidia': {'license-accepted': True, 'version': None}}} |
128 | diff --git a/tests/unittests/test_handler/test_handler_apt_source_v3.py b/tests/unittests/test_handler/test_handler_apt_source_v3.py | |||
129 | index 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 | 998 | 998 | ||
134 | 999 | class TestDebconfSelections(TestCase): | 999 | class TestDebconfSelections(TestCase): |
135 | 1000 | 1000 | ||
136 | 1001 | @mock.patch("cloudinit.config.cc_apt_configure.util.subp") | ||
137 | 1002 | def test_set_sel_appends_newline_if_absent(self, m_subp): | ||
138 | 1003 | """Automatically append a newline to debconf-set-selections config.""" | ||
139 | 1004 | selections = b'some/setting boolean true' | ||
140 | 1005 | cc_apt_configure.debconf_set_selections(selections=selections) | ||
141 | 1006 | cc_apt_configure.debconf_set_selections(selections=selections + b'\n') | ||
142 | 1007 | m_call = mock.call( | ||
143 | 1008 | ['debconf-set-selections'], data=selections + b'\n', capture=True, | ||
144 | 1009 | target=None) | ||
145 | 1010 | self.assertEqual([m_call, m_call], m_subp.call_args_list) | ||
146 | 1011 | |||
147 | 1001 | @mock.patch("cloudinit.config.cc_apt_configure.debconf_set_selections") | 1012 | @mock.patch("cloudinit.config.cc_apt_configure.debconf_set_selections") |
148 | 1002 | def test_no_set_sel_if_none_to_set(self, m_set_sel): | 1013 | def test_no_set_sel_if_none_to_set(self, m_set_sel): |
149 | 1003 | cc_apt_configure.apply_debconf_selections({'foo': 'bar'}) | 1014 | cc_apt_configure.apply_debconf_selections({'foo': 'bar'}) |
FAILED: Continuous integration, rev:a863ff8e782 36c07d899e0db53 d8df0fd29341b0 /jenkins. ubuntu. com/server/ job/cloud- init-ci/ 1049/
https:/
Executed test runs:
SUCCESS: Checkout
FAILED: Unit & Style Tests
Click here to trigger a rebuild: /jenkins. ubuntu. com/server/ job/cloud- init-ci/ 1049//rebuild
https:/