Merge ~oddbloke/cloud-init/+git/cloud-init:networking into cloud-init:master

[Dan Watkins]

net/cmdline: split interfaces_by_mac and init network config determination

Previously "cmdline" network configuration could be either
user-specified network-config=... configuration data, or
initramfs-provided configuration data. Before data sources could modify
the order in which network config sources were considered, this
conflation didn't matter (and, indeed, in the default data source
configuration it will continue to not matter).

However, it _is_ desirable for a data source to be able to specify that
its network configuration should be preferred over the
initramfs-provided network configuration but still allow explicit
network-config=... configuration passed to the kernel cmdline to
continue to override both of those sources.

(This also modifies the Oracle data source to use read_initramfs_config
directly, which is effectively what it was using
read_kernel_cmdline_config for previously.)

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

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

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

Nice clean-up. One question inline around the mock used, otherwise fine.

[Ryan Harper (raharper)]

review: Approve

diff --git a/cloudinit/net/cmdline.py b/cloudinit/net/cmdline.py
index f89a0f7..556a10f 100755
--- a/cloudinit/net/cmdline.py
+++ b/cloudinit/net/cmdline.py
@@ -177,21 +177,13 @@ def _is_initramfs_netconfig(files, cmdline):
     return False
 
 
-def read_kernel_cmdline_config(files=None, mac_addrs=None, cmdline=None):
+def read_initramfs_config(files=None, mac_addrs=None, cmdline=None):
     if cmdline is None:
         cmdline = util.get_cmdline()
 
     if files is None:
         files = _get_klibc_net_cfg_files()
 
-    if 'network-config=' in cmdline:
-        data64 = None
-        for tok in cmdline.split():
-            if tok.startswith("network-config="):
-                data64 = tok.split("=", 1)[1]
-        if data64:
-            return util.load_yaml(_b64dgz(data64))
-
     if not _is_initramfs_netconfig(files, cmdline):
         return None
 
@@ -204,4 +196,19 @@ def read_kernel_cmdline_config(files=None, mac_addrs=None, cmdline=None):
 
     return config_from_klibc_net_cfg(files=files, mac_addrs=mac_addrs)
 
+
+def read_kernel_cmdline_config(cmdline=None):
+    if cmdline is None:
+        cmdline = util.get_cmdline()
+
+    if 'network-config=' in cmdline:
+        data64 = None
+        for tok in cmdline.split():
+            if tok.startswith("network-config="):
+                data64 = tok.split("=", 1)[1]
+        if data64:
+            return util.load_yaml(_b64dgz(data64))
+
+    return None
+
 # vi: ts=4 expandtab
diff --git a/cloudinit/sources/DataSourceOracle.py b/cloudinit/sources/DataSourceOracle.py
index 70b9c58..76cfa38 100644
--- a/cloudinit/sources/DataSourceOracle.py
+++ b/cloudinit/sources/DataSourceOracle.py
@@ -48,7 +48,7 @@ class DataSourceOracle(sources.DataSource):
             return False
 
         # network may be configured if iscsi root.  If that is the case
-        # then read_kernel_cmdline_config will return non-None.
+        # then read_initramfs_config will return non-None.
         if _is_iscsi_root():
             data = self.crawl_metadata()
         else:
@@ -118,10 +118,8 @@ class DataSourceOracle(sources.DataSource):
         We nonetheless return cmdline provided config if present
         and fallback to generate fallback."""
         if self._network_config == sources.UNSET:
-            cmdline_cfg = cmdline.read_kernel_cmdline_config()
-            if cmdline_cfg:
-                self._network_config = cmdline_cfg
-            else:
+            self._network_config = cmdline.read_initramfs_config()
+            if not self._network_config:
                 self._network_config = self.distro.generate_fallback_config()
         return self._network_config
 
@@ -137,7 +135,7 @@ def _is_platform_viable():
 
 
 def _is_iscsi_root():
-    return bool(cmdline.read_kernel_cmdline_config())
+    return bool(cmdline.read_initramfs_config())
 
 
 def _load_index(content):
diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py
index 9d24936..c2baccd 100644
--- a/cloudinit/sources/__init__.py
+++ b/cloudinit/sources/__init__.py
@@ -69,7 +69,7 @@ CLOUD_ID_REGION_PREFIX_MAP = {
 # NetworkConfigSource represents the canonical list of network config sources
 # that cloud-init knows about.  (Python 2.7 lacks PEP 435, so use a singleton
 # namedtuple as an enum; see https://stackoverflow.com/a/6971002)
-_NETCFG_SOURCE_NAMES = ('cmdline', 'ds', 'system_cfg', 'fallback')
+_NETCFG_SOURCE_NAMES = ('cmdline', 'ds', 'system_cfg', 'fallback', 'initramfs')
 NetworkConfigSource = namedtuple('NetworkConfigSource',
                                  _NETCFG_SOURCE_NAMES)(*_NETCFG_SOURCE_NAMES)
 
@@ -166,6 +166,7 @@ class DataSource(object):
     # should always be a subset of the members of NetworkConfigSource with no
     # duplicate entries.
     network_config_sources = (NetworkConfigSource.cmdline,
+                              NetworkConfigSource.initramfs,
                               NetworkConfigSource.system_cfg,
                               NetworkConfigSource.ds)
 
diff --git a/cloudinit/sources/tests/test_oracle.py b/cloudinit/sources/tests/test_oracle.py
index 97d6294..282382c 100644
--- a/cloudinit/sources/tests/test_oracle.py
+++ b/cloudinit/sources/tests/test_oracle.py
@@ -133,9 +133,9 @@ class TestDataSourceOracle(test_helpers.CiTestCase):
         self.assertEqual(self.my_md['uuid'], ds.get_instance_id())
         self.assertEqual(my_userdata, ds.userdata_raw)
 
-    @mock.patch(DS_PATH + ".cmdline.read_kernel_cmdline_config")
+    @mock.patch(DS_PATH + ".cmdline.read_initramfs_config")
     @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True)
-    def test_network_cmdline(self, m_is_iscsi_root, m_cmdline_config):
+    def test_network_cmdline(self, m_is_iscsi_root, m_initramfs_config):
         """network_config should read kernel cmdline."""
         distro = mock.MagicMock()
         ds, _ = self._get_ds(distro=distro, patches={
@@ -145,15 +145,15 @@ class TestDataSourceOracle(test_helpers.CiTestCase):
                     MD_VER: {'system_uuid': self.my_uuid,
                              'meta_data': self.my_md}}}})
         ncfg = {'version': 1, 'config': [{'a': 'b'}]}
-        m_cmdline_config.return_value = ncfg
+        m_initramfs_config.return_value = ncfg
         self.assertTrue(ds._get_data())
         self.assertEqual(ncfg, ds.network_config)
-        m_cmdline_config.assert_called_once_with()
+        self.assertEqual([mock.call()], m_initramfs_config.call_args_list)
         self.assertFalse(distro.generate_fallback_config.called)
 
-    @mock.patch(DS_PATH + ".cmdline.read_kernel_cmdline_config")
+    @mock.patch(DS_PATH + ".cmdline.read_initramfs_config")
     @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True)
-    def test_network_fallback(self, m_is_iscsi_root, m_cmdline_config):
+    def test_network_fallback(self, m_is_iscsi_root, m_initramfs_config):
         """test that fallback network is generated if no kernel cmdline."""
         distro = mock.MagicMock()
         ds, _ = self._get_ds(distro=distro, patches={
@@ -163,18 +163,17 @@ class TestDataSourceOracle(test_helpers.CiTestCase):
                     MD_VER: {'system_uuid': self.my_uuid,
                              'meta_data': self.my_md}}}})
         ncfg = {'version': 1, 'config': [{'a': 'b'}]}
-        m_cmdline_config.return_value = None
+        m_initramfs_config.return_value = None
         self.assertTrue(ds._get_data())
         ncfg = {'version': 1, 'config': [{'distro1': 'value'}]}
         distro.generate_fallback_config.return_value = ncfg
         self.assertEqual(ncfg, ds.network_config)
-        m_cmdline_config.assert_called_once_with()
+        self.assertEqual([mock.call()], m_initramfs_config.call_args_list)
         distro.generate_fallback_config.assert_called_once_with()
-        self.assertEqual(1, m_cmdline_config.call_count)
 
         # test that the result got cached, and the methods not re-called.
         self.assertEqual(ncfg, ds.network_config)
-        self.assertEqual(1, m_cmdline_config.call_count)
+        self.assertEqual(1, m_initramfs_config.call_count)
 
 
 @mock.patch(DS_PATH + "._read_system_uuid", return_value=str(uuid.uuid4()))
diff --git a/cloudinit/stages.py b/cloudinit/stages.py
index 6bcda2d..5012988 100644
--- a/cloudinit/stages.py
+++ b/cloudinit/stages.py
@@ -633,6 +633,7 @@ class Init(object):
 
         available_cfgs = {
             NetworkConfigSource.cmdline: cmdline.read_kernel_cmdline_config(),
+            NetworkConfigSource.initramfs: cmdline.read_initramfs_config(),
             NetworkConfigSource.ds: None,
             NetworkConfigSource.system_cfg: self.cfg.get('network'),
         }
diff --git a/cloudinit/tests/test_stages.py b/cloudinit/tests/test_stages.py
index 7e13e29..d5c9c0e 100644
--- a/cloudinit/tests/test_stages.py
+++ b/cloudinit/tests/test_stages.py
@@ -59,20 +59,39 @@ class TestInit(CiTestCase):
             (None, disable_file),
             self.init._find_networking_config())
 
+    @mock.patch('cloudinit.stages.cmdline.read_initramfs_config')
     @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config')
-    def test_wb__find_networking_config_disabled_by_kernel(self, m_cmdline):
+    def test_wb__find_networking_config_disabled_by_kernel(
+            self, m_cmdline, m_initramfs):
         """find_networking_config returns when disabled by kernel cmdline."""
         m_cmdline.return_value = {'config': 'disabled'}
+        m_initramfs.return_value = {'config': ['fake_initrd']}
         self.assertEqual(
             (None, NetworkConfigSource.cmdline),
             self.init._find_networking_config())
         self.assertEqual('DEBUG: network config disabled by cmdline\n',
                          self.logs.getvalue())
 
+    @mock.patch('cloudinit.stages.cmdline.read_initramfs_config')
     @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config')
-    def test_wb__find_networking_config_disabled_by_datasrc(self, m_cmdline):
+    def test_wb__find_networking_config_disabled_by_initrd(
+            self, m_cmdline, m_initramfs):
+        """find_networking_config returns when disabled by kernel cmdline."""
+        m_cmdline.return_value = {}
+        m_initramfs.return_value = {'config': 'disabled'}
+        self.assertEqual(
+            (None, NetworkConfigSource.initramfs),
+            self.init._find_networking_config())
+        self.assertEqual('DEBUG: network config disabled by initramfs\n',
+                         self.logs.getvalue())
+
+    @mock.patch('cloudinit.stages.cmdline.read_initramfs_config')
+    @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config')
+    def test_wb__find_networking_config_disabled_by_datasrc(
+            self, m_cmdline, m_initramfs):
         """find_networking_config returns when disabled by datasource cfg."""
         m_cmdline.return_value = {}  # Kernel doesn't disable networking
+        m_initramfs.return_value = {}  # initramfs doesn't disable networking
         self.init._cfg = {'system_info': {'paths': {'cloud_dir': self.tmpdir}},
                           'network': {}}  # system config doesn't disable
 
@@ -84,10 +103,13 @@ class TestInit(CiTestCase):
         self.assertEqual('DEBUG: network config disabled by ds\n',
                          self.logs.getvalue())
 
+    @mock.patch('cloudinit.stages.cmdline.read_initramfs_config')
     @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config')
-    def test_wb__find_networking_config_disabled_by_sysconfig(self, m_cmdline):
+    def test_wb__find_networking_config_disabled_by_sysconfig(
+            self, m_cmdline, m_initramfs):
         """find_networking_config returns when disabled by system config."""
         m_cmdline.return_value = {}  # Kernel doesn't disable networking
+        m_initramfs.return_value = {}  # initramfs doesn't disable networking
         self.init._cfg = {'system_info': {'paths': {'cloud_dir': self.tmpdir}},
                           'network': {'config': 'disabled'}}
         self.assertEqual(
@@ -96,27 +118,31 @@ class TestInit(CiTestCase):
         self.assertEqual('DEBUG: network config disabled by system_cfg\n',
                          self.logs.getvalue())
 
+    @mock.patch('cloudinit.stages.cmdline.read_initramfs_config')
     @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config')
-    def test__find_networking_config_uses_datasrc_order(self, m_cmdline):
+    def test__find_networking_config_uses_datasrc_order(
+            self, m_cmdline, m_initramfs):
         """find_networking_config should check sources in DS defined order"""
-        # cmdline, which would normally be preferred over other sources,
-        # disables networking; in this case, though, the DS moves cmdline later
-        # so its own config is preferred
+        # cmdline and initramfs, which would normally be preferred over other
+        # sources, disable networking; in this case, though, the DS moves them
+        # later so its own config is preferred
         m_cmdline.return_value = {'config': 'disabled'}
+        m_initramfs.return_value = {'config': 'disabled'}
 
         ds_net_cfg = {'config': {'needle': True}}
         self.init.datasource = FakeDataSource(network_config=ds_net_cfg)
         self.init.datasource.network_config_sources = [
             NetworkConfigSource.ds, NetworkConfigSource.system_cfg,
-            NetworkConfigSource.cmdline]
+            NetworkConfigSource.cmdline, NetworkConfigSource.initramfs]
 
         self.assertEqual(
             (ds_net_cfg, NetworkConfigSource.ds),
             self.init._find_networking_config())
 
+    @mock.patch('cloudinit.stages.cmdline.read_initramfs_config')
     @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config')
     def test__find_networking_config_warns_if_datasrc_uses_invalid_src(
-            self, m_cmdline):
+            self, m_cmdline, m_initramfs):
         """find_networking_config should check sources in DS defined order"""
         ds_net_cfg = {'config': {'needle': True}}
         self.init.datasource = FakeDataSource(network_config=ds_net_cfg)
@@ -130,9 +156,10 @@ class TestInit(CiTestCase):
                       ' cfg_source: invalid_src',
                       self.logs.getvalue())
 
+    @mock.patch('cloudinit.stages.cmdline.read_initramfs_config')
     @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config')
     def test__find_networking_config_warns_if_datasrc_uses_unavailable_src(
-            self, m_cmdline):
+            self, m_cmdline, m_initramfs):
         """find_networking_config should check sources in DS defined order"""
         ds_net_cfg = {'config': {'needle': True}}
         self.init.datasource = FakeDataSource(network_config=ds_net_cfg)
@@ -146,11 +173,14 @@ class TestInit(CiTestCase):
                       ' cfg_source: fallback',
                       self.logs.getvalue())
 
+    @mock.patch('cloudinit.stages.cmdline.read_initramfs_config')
     @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config')
-    def test_wb__find_networking_config_returns_kernel(self, m_cmdline):
+    def test_wb__find_networking_config_returns_kernel(
+            self, m_cmdline, m_initramfs):
         """find_networking_config returns kernel cmdline config if present."""
         expected_cfg = {'config': ['fakekernel']}
         m_cmdline.return_value = expected_cfg
+        m_initramfs.return_value = {'config': ['fake_initrd']}
         self.init._cfg = {'system_info': {'paths': {'cloud_dir': self.tmpdir}},
                           'network': {'config': ['fakesys_config']}}
         self.init.datasource = FakeDataSource(
@@ -159,10 +189,29 @@ class TestInit(CiTestCase):
             (expected_cfg, NetworkConfigSource.cmdline),
             self.init._find_networking_config())
 
+    @mock.patch('cloudinit.stages.cmdline.read_initramfs_config')
+    @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config')
+    def test_wb__find_networking_config_returns_initramfs(
+            self, m_cmdline, m_initramfs):
+        """find_networking_config returns kernel cmdline config if present."""
+        expected_cfg = {'config': ['fake_initrd']}
+        m_cmdline.return_value = {}
+        m_initramfs.return_value = expected_cfg
+        self.init._cfg = {'system_info': {'paths': {'cloud_dir': self.tmpdir}},
+                          'network': {'config': ['fakesys_config']}}
+        self.init.datasource = FakeDataSource(
+            network_config={'config': ['fakedatasource']})
+        self.assertEqual(
+            (expected_cfg, NetworkConfigSource.initramfs),
+            self.init._find_networking_config())
+
+    @mock.patch('cloudinit.stages.cmdline.read_initramfs_config')
     @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config')
-    def test_wb__find_networking_config_returns_system_cfg(self, m_cmdline):
+    def test_wb__find_networking_config_returns_system_cfg(
+            self, m_cmdline, m_initramfs):
         """find_networking_config returns system config when present."""
         m_cmdline.return_value = {}  # No kernel network config
+        m_initramfs.return_value = {}  # no initramfs network config
         expected_cfg = {'config': ['fakesys_config']}
         self.init._cfg = {'system_info': {'paths': {'cloud_dir': self.tmpdir}},
                           'network': expected_cfg}
@@ -172,10 +221,13 @@ class TestInit(CiTestCase):
             (expected_cfg, NetworkConfigSource.system_cfg),
             self.init._find_networking_config())
 
+    @mock.patch('cloudinit.stages.cmdline.read_initramfs_config')
     @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config')
-    def test_wb__find_networking_config_returns_datasrc_cfg(self, m_cmdline):
+    def test_wb__find_networking_config_returns_datasrc_cfg(
+            self, m_cmdline, m_initramfs):
         """find_networking_config returns datasource net config if present."""
         m_cmdline.return_value = {}  # No kernel network config
+        m_initramfs.return_value = {}  # no initramfs network config
         # No system config for network in setUp
         expected_cfg = {'config': ['fakedatasource']}
         self.init.datasource = FakeDataSource(network_config=expected_cfg)
@@ -183,10 +235,13 @@ class TestInit(CiTestCase):
             (expected_cfg, NetworkConfigSource.ds),
             self.init._find_networking_config())
 
+    @mock.patch('cloudinit.stages.cmdline.read_initramfs_config')
     @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config')
-    def test_wb__find_networking_config_returns_fallback(self, m_cmdline):
+    def test_wb__find_networking_config_returns_fallback(
+            self, m_cmdline, m_initramfs):
         """find_networking_config returns fallback config if not defined."""
         m_cmdline.return_value = {}  # Kernel doesn't disable networking
+        m_initramfs.return_value = {}  # no initramfs network config
         # Neither datasource nor system_info disable or provide network
 
         fake_cfg = {'config': [{'type': 'physical', 'name': 'eth9'}],
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index e2bbb84..1840ade 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -3558,13 +3558,13 @@ class TestCmdlineConfigParsing(CiTestCase):
         self.assertEqual(found, self.simple_cfg)
 
 
-class TestCmdlineReadKernelConfig(FilesystemMockingTestCase):
+class TestCmdlineReadInitramfsConfig(FilesystemMockingTestCase):
     macs = {
         'eth0': '14:02:ec:42:48:00',
         'eno1': '14:02:ec:42:48:01',
     }
 
-    def test_ip_cmdline_without_ip(self):
+    def test_without_ip(self):
         content = {'/run/net-eth0.conf': DHCP_CONTENT_1,
                    cmdline._OPEN_ISCSI_INTERFACE_FILE: "eth0\n"}
         exp1 = copy.deepcopy(DHCP_EXPECTED_1)
@@ -3574,12 +3574,12 @@ class TestCmdlineReadKernelConfig(FilesystemMockingTestCase):
         populate_dir(root, content)
         self.reRoot(root)
 
-        found = cmdline.read_kernel_cmdline_config(
+        found = cmdline.read_initramfs_config(
             cmdline='foo root=/root/bar', mac_addrs=self.macs)
         self.assertEqual(found['version'], 1)
         self.assertEqual(found['config'], [exp1])
 
-    def test_ip_cmdline_read_kernel_cmdline_ip(self):
+    def test_with_ip(self):
         content = {'/run/net-eth0.conf': DHCP_CONTENT_1}
         exp1 = copy.deepcopy(DHCP_EXPECTED_1)
         exp1['mac_address'] = self.macs['eth0']
@@ -3588,18 +3588,18 @@ class TestCmdlineReadKernelConfig(FilesystemMockingTestCase):
         populate_dir(root, content)
         self.reRoot(root)
 
-        found = cmdline.read_kernel_cmdline_config(
+        found = cmdline.read_initramfs_config(
             cmdline='foo ip=dhcp', mac_addrs=self.macs)
         self.assertEqual(found['version'], 1)
         self.assertEqual(found['config'], [exp1])
 
-    def test_ip_cmdline_read_kernel_cmdline_ip6(self):
+    def test_with_ip6(self):
         content = {'/run/net6-eno1.conf': DHCP6_CONTENT_1}
         root = self.tmp_dir()
         populate_dir(root, content)
         self.reRoot(root)
 
-        found = cmdline.read_kernel_cmdline_config(
+        found = cmdline.read_initramfs_config(
             cmdline='foo ip6=dhcp root=/dev/sda',
             mac_addrs=self.macs)
         self.assertEqual(
@@ -3611,15 +3611,15 @@ class TestCmdlineReadKernelConfig(FilesystemMockingTestCase):
                   {'dns_nameservers': ['2001:67c:1562:8010::2:1'],
                    'control': 'manual', 'type': 'dhcp6', 'netmask': '64'}]}]})
 
-    def test_ip_cmdline_read_kernel_cmdline_none(self):
+    def test_with_no_ip_or_ip6(self):
         # if there is no ip= or ip6= on cmdline, return value should be None
         content = {'net6-eno1.conf': DHCP6_CONTENT_1}
         files = sorted(populate_dir(self.tmp_dir(), content))
-        found = cmdline.read_kernel_cmdline_config(
+        found = cmdline.read_initramfs_config(
             files=files, cmdline='foo root=/dev/sda', mac_addrs=self.macs)
         self.assertIsNone(found)
 
-    def test_ip_cmdline_both_ip_ip6(self):
+    def test_with_both_ip_ip6(self):
         content = {
             '/run/net-eth0.conf': DHCP_CONTENT_1,
             '/run/net6-eth0.conf': DHCP6_CONTENT_1.replace('eno1', 'eth0')}
@@ -3634,7 +3634,7 @@ class TestCmdlineReadKernelConfig(FilesystemMockingTestCase):
         populate_dir(root, content)
         self.reRoot(root)
 
-        found = cmdline.read_kernel_cmdline_config(
+        found = cmdline.read_initramfs_config(
             cmdline='foo ip=dhcp ip6=dhcp', mac_addrs=self.macs)
 
         self.assertEqual(found['version'], 1)