Merge ~daniel-thewatkins/cloud-init/+git/cloud-init:dracut into cloud-init:master

Proposed by Dan Watkins on 2019-08-22
Status: Merged
Approved by: Dan Watkins on 2019-08-22
Approved revision: 2c9297467adb4c4d5d44f4a66518a25fa1b2eebf
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~daniel-thewatkins/cloud-init/+git/cloud-init:dracut
Merge into: cloud-init:master
Diff against target: 314 lines (+181/-45)
2 files modified
cloudinit/net/cmdline.py (+95/-32)
tests/unittests/test_net.py (+86/-13)
Reviewer Review Type Date Requested Status
Chad Smith Approve on 2019-08-22
Server Team CI bot continuous-integration Approve on 2019-08-22
Ryan Harper 2019-08-22 Approve on 2019-08-22
Review via email: mp+371673@code.launchpad.net

Commit message

net/cmdline: refactor to allow multiple initramfs network config sources

This refactors read_initramfs_config to support multiple different types
of initramfs network configuration. It introduces an
InitramfsNetworkConfigSource abstract base class. There is currently a
single sub-class, KlibcNetworkConfigSource, which contains the logic
which previously was directly within read_initramfs_config.

To post a comment you must log in.
Ryan Harper (raharper) :

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

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

I was expecting a second commit; but I see that's it updated.
And it sounds like you'll defer the refactor to the dracut branch. Sounds good to me.

review: Approve

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

review: Approve (continuous-integration)
Chad Smith (chad.smith) wrote :

LGTM!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cloudinit/net/cmdline.py b/cloudinit/net/cmdline.py
2index 556a10f..55166ea 100755
3--- a/cloudinit/net/cmdline.py
4+++ b/cloudinit/net/cmdline.py
5@@ -5,20 +5,95 @@
6 #
7 # This file is part of cloud-init. See LICENSE file for license information.
8
9+import abc
10 import base64
11 import glob
12 import gzip
13 import io
14 import os
15
16-from . import get_devicelist
17-from . import read_sys_net_safe
18+import six
19
20 from cloudinit import util
21
22+from . import get_devicelist
23+from . import read_sys_net_safe
24+
25 _OPEN_ISCSI_INTERFACE_FILE = "/run/initramfs/open-iscsi.interface"
26
27
28+@six.add_metaclass(abc.ABCMeta)
29+class InitramfsNetworkConfigSource(object):
30+ """ABC for net config sources that read config written by initramfses"""
31+
32+ @abc.abstractmethod
33+ def is_applicable(self):
34+ # type: () -> bool
35+ """Is this initramfs config source applicable to the current system?"""
36+ pass
37+
38+ @abc.abstractmethod
39+ def render_config(self):
40+ # type: () -> dict
41+ """Render a v1 network config from the initramfs configuration"""
42+ pass
43+
44+
45+class KlibcNetworkConfigSource(InitramfsNetworkConfigSource):
46+ """InitramfsNetworkConfigSource for klibc initramfs (i.e. Debian/Ubuntu)
47+
48+ Has three parameters, but they are intended to make testing simpler, _not_
49+ for use in production code. (This is indicated by the prepended
50+ underscores.)
51+ """
52+
53+ def __init__(self, _files=None, _mac_addrs=None, _cmdline=None):
54+ self._files = _files
55+ self._mac_addrs = _mac_addrs
56+ self._cmdline = _cmdline
57+
58+ # Set defaults here, as they require computation that we don't want to
59+ # do at method definition time
60+ if self._files is None:
61+ self._files = _get_klibc_net_cfg_files()
62+ if self._cmdline is None:
63+ self._cmdline = util.get_cmdline()
64+ if self._mac_addrs is None:
65+ self._mac_addrs = {}
66+ for k in get_devicelist():
67+ mac_addr = read_sys_net_safe(k, 'address')
68+ if mac_addr:
69+ self._mac_addrs[k] = mac_addr
70+
71+ def is_applicable(self):
72+ # type: () -> bool
73+ """
74+ Return whether this system has klibc initramfs network config or not
75+
76+ Will return True if:
77+ (a) klibc files exist in /run, AND
78+ (b) either:
79+ (i) ip= or ip6= are on the kernel cmdline, OR
80+ (ii) an open-iscsi interface file is present in the system
81+ """
82+ if self._files:
83+ if 'ip=' in self._cmdline or 'ip6=' in self._cmdline:
84+ return True
85+ if os.path.exists(_OPEN_ISCSI_INTERFACE_FILE):
86+ # iBft can configure networking without ip=
87+ return True
88+ return False
89+
90+ def render_config(self):
91+ # type: () -> dict
92+ return config_from_klibc_net_cfg(
93+ files=self._files, mac_addrs=self._mac_addrs,
94+ )
95+
96+
97+_INITRAMFS_CONFIG_SOURCES = [KlibcNetworkConfigSource]
98+
99+
100 def _klibc_to_config_entry(content, mac_addrs=None):
101 """Convert a klibc written shell content file to a 'config' entry
102 When ip= is seen on the kernel command line in debian initramfs
103@@ -137,6 +212,24 @@ def config_from_klibc_net_cfg(files=None, mac_addrs=None):
104 return {'config': entries, 'version': 1}
105
106
107+def read_initramfs_config():
108+ """
109+ Return v1 network config for initramfs-configured networking (or None)
110+
111+ This will consider each _INITRAMFS_CONFIG_SOURCES entry in turn, and return
112+ v1 network configuration for the first one that is applicable. If none are
113+ applicable, return None.
114+ """
115+ for src_cls in _INITRAMFS_CONFIG_SOURCES:
116+ cfg_source = src_cls()
117+
118+ if not cfg_source.is_applicable():
119+ continue
120+
121+ return cfg_source.render_config()
122+ return None
123+
124+
125 def _decomp_gzip(blob, strict=True):
126 # decompress blob. raise exception if not compressed unless strict=False.
127 with io.BytesIO(blob) as iobuf:
128@@ -167,36 +260,6 @@ def _b64dgz(b64str, gzipped="try"):
129 return _decomp_gzip(blob, strict=gzipped != "try")
130
131
132-def _is_initramfs_netconfig(files, cmdline):
133- if files:
134- if 'ip=' in cmdline or 'ip6=' in cmdline:
135- return True
136- if os.path.exists(_OPEN_ISCSI_INTERFACE_FILE):
137- # iBft can configure networking without ip=
138- return True
139- return False
140-
141-
142-def read_initramfs_config(files=None, mac_addrs=None, cmdline=None):
143- if cmdline is None:
144- cmdline = util.get_cmdline()
145-
146- if files is None:
147- files = _get_klibc_net_cfg_files()
148-
149- if not _is_initramfs_netconfig(files, cmdline):
150- return None
151-
152- if mac_addrs is None:
153- mac_addrs = {}
154- for k in get_devicelist():
155- mac_addr = read_sys_net_safe(k, 'address')
156- if mac_addr:
157- mac_addrs[k] = mac_addr
158-
159- return config_from_klibc_net_cfg(files=files, mac_addrs=mac_addrs)
160-
161-
162 def read_kernel_cmdline_config(cmdline=None):
163 if cmdline is None:
164 cmdline = util.get_cmdline()
165diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
166index 4f7e420..e578992 100644
167--- a/tests/unittests/test_net.py
168+++ b/tests/unittests/test_net.py
169@@ -3591,7 +3591,7 @@ class TestCmdlineConfigParsing(CiTestCase):
170 self.assertEqual(found, self.simple_cfg)
171
172
173-class TestCmdlineReadInitramfsConfig(FilesystemMockingTestCase):
174+class TestCmdlineKlibcNetworkConfigSource(FilesystemMockingTestCase):
175 macs = {
176 'eth0': '14:02:ec:42:48:00',
177 'eno1': '14:02:ec:42:48:01',
178@@ -3607,8 +3607,11 @@ class TestCmdlineReadInitramfsConfig(FilesystemMockingTestCase):
179 populate_dir(root, content)
180 self.reRoot(root)
181
182- found = cmdline.read_initramfs_config(
183- cmdline='foo root=/root/bar', mac_addrs=self.macs)
184+ src = cmdline.KlibcNetworkConfigSource(
185+ _cmdline='foo root=/root/bar', _mac_addrs=self.macs,
186+ )
187+ self.assertTrue(src.is_applicable())
188+ found = src.render_config()
189 self.assertEqual(found['version'], 1)
190 self.assertEqual(found['config'], [exp1])
191
192@@ -3621,8 +3624,11 @@ class TestCmdlineReadInitramfsConfig(FilesystemMockingTestCase):
193 populate_dir(root, content)
194 self.reRoot(root)
195
196- found = cmdline.read_initramfs_config(
197- cmdline='foo ip=dhcp', mac_addrs=self.macs)
198+ src = cmdline.KlibcNetworkConfigSource(
199+ _cmdline='foo ip=dhcp', _mac_addrs=self.macs,
200+ )
201+ self.assertTrue(src.is_applicable())
202+ found = src.render_config()
203 self.assertEqual(found['version'], 1)
204 self.assertEqual(found['config'], [exp1])
205
206@@ -3632,9 +3638,11 @@ class TestCmdlineReadInitramfsConfig(FilesystemMockingTestCase):
207 populate_dir(root, content)
208 self.reRoot(root)
209
210- found = cmdline.read_initramfs_config(
211- cmdline='foo ip6=dhcp root=/dev/sda',
212- mac_addrs=self.macs)
213+ src = cmdline.KlibcNetworkConfigSource(
214+ _cmdline='foo ip6=dhcp root=/dev/sda', _mac_addrs=self.macs,
215+ )
216+ self.assertTrue(src.is_applicable())
217+ found = src.render_config()
218 self.assertEqual(
219 found,
220 {'version': 1, 'config': [
221@@ -3648,9 +3656,10 @@ class TestCmdlineReadInitramfsConfig(FilesystemMockingTestCase):
222 # if there is no ip= or ip6= on cmdline, return value should be None
223 content = {'net6-eno1.conf': DHCP6_CONTENT_1}
224 files = sorted(populate_dir(self.tmp_dir(), content))
225- found = cmdline.read_initramfs_config(
226- files=files, cmdline='foo root=/dev/sda', mac_addrs=self.macs)
227- self.assertIsNone(found)
228+ src = cmdline.KlibcNetworkConfigSource(
229+ _files=files, _cmdline='foo root=/dev/sda', _mac_addrs=self.macs,
230+ )
231+ self.assertFalse(src.is_applicable())
232
233 def test_with_both_ip_ip6(self):
234 content = {
235@@ -3667,13 +3676,77 @@ class TestCmdlineReadInitramfsConfig(FilesystemMockingTestCase):
236 populate_dir(root, content)
237 self.reRoot(root)
238
239- found = cmdline.read_initramfs_config(
240- cmdline='foo ip=dhcp ip6=dhcp', mac_addrs=self.macs)
241+ src = cmdline.KlibcNetworkConfigSource(
242+ _cmdline='foo ip=dhcp ip6=dhcp', _mac_addrs=self.macs,
243+ )
244
245+ self.assertTrue(src.is_applicable())
246+ found = src.render_config()
247 self.assertEqual(found['version'], 1)
248 self.assertEqual(found['config'], expected)
249
250
251+class TestReadInitramfsConfig(CiTestCase):
252+
253+ def _config_source_cls_mock(self, is_applicable, render_config=None):
254+ return lambda: mock.Mock(
255+ is_applicable=lambda: is_applicable,
256+ render_config=lambda: render_config,
257+ )
258+
259+ def test_no_sources(self):
260+ with mock.patch('cloudinit.net.cmdline._INITRAMFS_CONFIG_SOURCES', []):
261+ self.assertIsNone(cmdline.read_initramfs_config())
262+
263+ def test_no_applicable_sources(self):
264+ sources = [
265+ self._config_source_cls_mock(is_applicable=False),
266+ self._config_source_cls_mock(is_applicable=False),
267+ self._config_source_cls_mock(is_applicable=False),
268+ ]
269+ with mock.patch('cloudinit.net.cmdline._INITRAMFS_CONFIG_SOURCES',
270+ sources):
271+ self.assertIsNone(cmdline.read_initramfs_config())
272+
273+ def test_one_applicable_source(self):
274+ expected_config = object()
275+ sources = [
276+ self._config_source_cls_mock(
277+ is_applicable=True, render_config=expected_config,
278+ ),
279+ ]
280+ with mock.patch('cloudinit.net.cmdline._INITRAMFS_CONFIG_SOURCES',
281+ sources):
282+ self.assertEqual(expected_config, cmdline.read_initramfs_config())
283+
284+ def test_one_applicable_source_after_inapplicable_sources(self):
285+ expected_config = object()
286+ sources = [
287+ self._config_source_cls_mock(is_applicable=False),
288+ self._config_source_cls_mock(is_applicable=False),
289+ self._config_source_cls_mock(
290+ is_applicable=True, render_config=expected_config,
291+ ),
292+ ]
293+ with mock.patch('cloudinit.net.cmdline._INITRAMFS_CONFIG_SOURCES',
294+ sources):
295+ self.assertEqual(expected_config, cmdline.read_initramfs_config())
296+
297+ def test_first_applicable_source_is_used(self):
298+ first_config, second_config = object(), object()
299+ sources = [
300+ self._config_source_cls_mock(
301+ is_applicable=True, render_config=first_config,
302+ ),
303+ self._config_source_cls_mock(
304+ is_applicable=True, render_config=second_config,
305+ ),
306+ ]
307+ with mock.patch('cloudinit.net.cmdline._INITRAMFS_CONFIG_SOURCES',
308+ sources):
309+ self.assertEqual(first_config, cmdline.read_initramfs_config())
310+
311+
312 class TestNetplanRoundTrip(CiTestCase):
313 def _render_and_read(self, network_config=None, state=None,
314 netplan_path=None, target=None):

Subscribers

People subscribed via source and target branches