DataSourceOracle: configure secondary NICs on Virtual Machines

Oracle Cloud Infrastructure's Instance Metadata Service provides network
configuration information for non-primary NICs.  This commit introduces
support, on Virtual Machines[0], for fetching that network metadata,
converting it to v1 network-config[1] and combining it into the network
configuration generated for the primary interface.

By default, this behaviour is not enabled.  Configuring the Oracle
datasource to `configure_secondary_nics` enables it:

    datasource:
      Oracle:
        configure_secondary_nics: true

Failures to fetch and generate secondary NIC configuration will log a
warning, but otherwise will not affect boot.

[0] The expected use of the IMDS-provided network configuration is
    substantially different on Bare Metal Machines, so support for that
    will be addressed separately.
[1] This is v1 config, because cloudinit.net.cmdline generates v1 config
    and we need to integrate the secondary NICs into that configuration.

3 files changed, 324 insertions, 4 deletions

diff --git a/cloudinit/sources/DataSourceOracle.py b/cloudinit/sources/DataSourceOracle.py
index 76cfa38c..086af799 100644
--- a/cloudinit/sources/DataSourceOracle.py
+++ b/cloudinit/sources/DataSourceOracle.py
@@ -16,7 +16,7 @@ Notes:
 """
 
 from cloudinit.url_helper import combine_url, readurl, UrlError
-from cloudinit.net import dhcp
+from cloudinit.net import dhcp, get_interfaces_by_mac
 from cloudinit import net
 from cloudinit import sources
 from cloudinit import util
@@ -28,8 +28,80 @@ import re
 
 LOG = logging.getLogger(__name__)
 
+BUILTIN_DS_CONFIG = {
+    # Don't use IMDS to configure secondary NICs by default
+    'configure_secondary_nics': False,
+}
 CHASSIS_ASSET_TAG = "OracleCloud.com"
 METADATA_ENDPOINT = "http://169.254.169.254/openstack/"
+VNIC_METADATA_URL = 'http://169.254.169.254/opc/v1/vnics/'
+# https://docs.cloud.oracle.com/iaas/Content/Network/Troubleshoot/connectionhang.htm#Overview,
+# indicates that an MTU of 9000 is used within OCI
+MTU = 9000
+
+
+def _add_network_config_from_opc_imds(network_config):
+    """
+    Fetch data from Oracle's IMDS, generate secondary NIC config, merge it.
+
+    The primary NIC configuration should not be modified based on the IMDS
+    values, as it should continue to be configured for DHCP.  As such, this
+    takes an existing network_config dict which is expected to have the primary
+    NIC configuration already present.  It will mutate the given dict to
+    include the secondary VNICs.
+
+    :param network_config:
+        A v1 network config dict with the primary NIC already configured.  This
+        dict will be mutated.
+
+    :raises:
+        Exceptions are not handled within this function.  Likely exceptions are
+        those raised by url_helper.readurl (if communicating with the IMDS
+        fails), ValueError/JSONDecodeError (if the IMDS returns invalid JSON),
+        and KeyError/IndexError (if the IMDS returns valid JSON with unexpected
+        contents).
+    """
+    resp = readurl(VNIC_METADATA_URL)
+    vnics = json.loads(str(resp))
+
+    if 'nicIndex' in vnics[0]:
+        # TODO: Once configure_secondary_nics defaults to True, lower the level
+        # of this log message.  (Currently, if we're running this code at all,
+        # someone has explicitly opted-in to secondary VNIC configuration, so
+        # we should warn them that it didn't happen.  Once it's default, this
+        # would be emitted on every Bare Metal Machine launch, which means INFO
+        # or DEBUG would be more appropriate.)
+        LOG.warning(
+            'VNIC metadata indicates this is a bare metal machine; skipping'
+            ' secondary VNIC configuration.'
+        )
+        return
+
+    interfaces_by_mac = get_interfaces_by_mac()
+
+    for vnic_dict in vnics[1:]:
+        # We skip the first entry in the response because the primary interface
+        # is already configured by iSCSI boot; applying configuration from the
+        # IMDS is not required.
+        mac_address = vnic_dict['macAddr'].lower()
+        if mac_address not in interfaces_by_mac:
+            LOG.debug('Interface with MAC %s not found; skipping', mac_address)
+            continue
+        name = interfaces_by_mac[mac_address]
+        subnet = {
+            'type': 'static',
+            'address': vnic_dict['privateIp'],
+            'netmask': vnic_dict['subnetCidrBlock'].split('/')[1],
+            'gateway': vnic_dict['virtualRouterIp'],
+            'control': 'manual',
+        }
+        network_config['config'].append({
+            'name': name,
+            'type': 'physical',
+            'mac_address': mac_address,
+            'mtu': MTU,
+            'subnets': [subnet],
+        })
 
 
 class DataSourceOracle(sources.DataSource):
@@ -39,6 +111,13 @@ class DataSourceOracle(sources.DataSource):
     vendordata_pure = None
     _network_config = sources.UNSET
 
+    def __init__(self, sys_cfg, *args, **kwargs):
+        super(DataSourceOracle, self).__init__(sys_cfg, *args, **kwargs)
+
+        self.ds_cfg = util.mergemanydict([
+            util.get_cfg_by_path(sys_cfg, ['datasource', self.dsname], {}),
+            BUILTIN_DS_CONFIG])
+
     def _is_platform_viable(self):
         """Check platform environment to report if this datasource may run."""
         return _is_platform_viable()
@@ -121,6 +200,14 @@ class DataSourceOracle(sources.DataSource):
             self._network_config = cmdline.read_initramfs_config()
             if not self._network_config:
                 self._network_config = self.distro.generate_fallback_config()
+            if self.ds_cfg.get('configure_secondary_nics'):
+                try:
+                    # Mutate self._network_config to include secondary VNICs
+                    _add_network_config_from_opc_imds(self._network_config)
+                except Exception:
+                    util.logexc(
+                        LOG,
+                        "Failed to fetch secondary network configuration!")
         return self._network_config
 
 
diff --git a/cloudinit/sources/tests/test_oracle.py b/cloudinit/sources/tests/test_oracle.py
index 282382c5..3e146776 100644
--- a/cloudinit/sources/tests/test_oracle.py
+++ b/cloudinit/sources/tests/test_oracle.py
@@ -18,10 +18,52 @@ import uuid
 DS_PATH = "cloudinit.sources.DataSourceOracle"
 MD_VER = "2013-10-17"
 
+# `curl -L http://169.254.169.254/opc/v1/vnics/` on a Oracle Bare Metal Machine
+# with a secondary VNIC attached (vnicId truncated for Python line length)
+OPC_BM_SECONDARY_VNIC_RESPONSE = """\
+[ {
+  "vnicId" : "ocid1.vnic.oc1.phx.abyhqljtyvcucqkhdqmgjszebxe4hrb!!TRUNCATED||",
+  "privateIp" : "10.0.0.8",
+  "vlanTag" : 0,
+  "macAddr" : "90:e2:ba:d4:f1:68",
+  "virtualRouterIp" : "10.0.0.1",
+  "subnetCidrBlock" : "10.0.0.0/24",
+  "nicIndex" : 0
+}, {
+  "vnicId" : "ocid1.vnic.oc1.phx.abyhqljtfmkxjdy2sqidndiwrsg63zf!!TRUNCATED||",
+  "privateIp" : "10.0.4.5",
+  "vlanTag" : 1,
+  "macAddr" : "02:00:17:05:CF:51",
+  "virtualRouterIp" : "10.0.4.1",
+  "subnetCidrBlock" : "10.0.4.0/24",
+  "nicIndex" : 0
+} ]"""
+
+# `curl -L http://169.254.169.254/opc/v1/vnics/` on a Oracle Virtual Machine
+# with a secondary VNIC attached
+OPC_VM_SECONDARY_VNIC_RESPONSE = """\
+[ {
+  "vnicId" : "ocid1.vnic.oc1.phx.abyhqljtch72z5pd76cc2636qeqh7z_truncated",
+  "privateIp" : "10.0.0.230",
+  "vlanTag" : 1039,
+  "macAddr" : "02:00:17:05:D1:DB",
+  "virtualRouterIp" : "10.0.0.1",
+  "subnetCidrBlock" : "10.0.0.0/24"
+}, {
+  "vnicId" : "ocid1.vnic.oc1.phx.abyhqljt4iew3gwmvrwrhhf3bp5drj_truncated",
+  "privateIp" : "10.0.0.231",
+  "vlanTag" : 1041,
+  "macAddr" : "00:00:17:02:2B:B1",
+  "virtualRouterIp" : "10.0.0.1",
+  "subnetCidrBlock" : "10.0.0.0/24"
+} ]"""
+
 
 class TestDataSourceOracle(test_helpers.CiTestCase):
     """Test datasource DataSourceOracle."""
 
+    with_logs = True
+
     ds_class = oracle.DataSourceOracle
 
     my_uuid = str(uuid.uuid4())
@@ -79,6 +121,16 @@ class TestDataSourceOracle(test_helpers.CiTestCase):
         self.assertEqual(
             'metadata (http://169.254.169.254/openstack/)', ds.subplatform)
 
+    def test_sys_cfg_can_enable_configure_secondary_nics(self):
+        # Confirm that behaviour is toggled by sys_cfg
+        ds, _mocks = self._get_ds()
+        self.assertFalse(ds.ds_cfg['configure_secondary_nics'])
+
+        sys_cfg = {
+            'datasource': {'Oracle': {'configure_secondary_nics': True}}}
+        ds, _mocks = self._get_ds(sys_cfg=sys_cfg)
+        self.assertTrue(ds.ds_cfg['configure_secondary_nics'])
+
     @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True)
     def test_without_userdata(self, m_is_iscsi_root):
         """If no user-data is provided, it should not be in return dict."""
@@ -133,9 +185,12 @@ 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 + "._add_network_config_from_opc_imds",
+                side_effect=lambda network_config: network_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_initramfs_config):
+    def test_network_cmdline(self, m_is_iscsi_root, m_initramfs_config,
+                             _m_add_network_config_from_opc_imds):
         """network_config should read kernel cmdline."""
         distro = mock.MagicMock()
         ds, _ = self._get_ds(distro=distro, patches={
@@ -151,9 +206,12 @@ class TestDataSourceOracle(test_helpers.CiTestCase):
         self.assertEqual([mock.call()], m_initramfs_config.call_args_list)
         self.assertFalse(distro.generate_fallback_config.called)
 
+    @mock.patch(DS_PATH + "._add_network_config_from_opc_imds",
+                side_effect=lambda network_config: network_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_initramfs_config):
+    def test_network_fallback(self, m_is_iscsi_root, m_initramfs_config,
+                              _m_add_network_config_from_opc_imds):
         """test that fallback network is generated if no kernel cmdline."""
         distro = mock.MagicMock()
         ds, _ = self._get_ds(distro=distro, patches={
@@ -175,6 +233,76 @@ class TestDataSourceOracle(test_helpers.CiTestCase):
         self.assertEqual(ncfg, ds.network_config)
         self.assertEqual(1, m_initramfs_config.call_count)
 
+    @mock.patch(DS_PATH + "._add_network_config_from_opc_imds")
+    @mock.patch(DS_PATH + ".cmdline.read_initramfs_config",
+                return_value={'some': 'config'})
+    @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True)
+    def test_secondary_nics_added_to_network_config_if_enabled(
+            self, _m_is_iscsi_root, _m_initramfs_config,
+            m_add_network_config_from_opc_imds):
+
+        needle = object()
+
+        def network_config_side_effect(network_config):
+            network_config['secondary_added'] = needle
+
+        m_add_network_config_from_opc_imds.side_effect = (
+            network_config_side_effect)
+
+        distro = mock.MagicMock()
+        ds, _ = self._get_ds(distro=distro, patches={
+            '_is_platform_viable': {'return_value': True},
+            'crawl_metadata': {
+                'return_value': {
+                    MD_VER: {'system_uuid': self.my_uuid,
+                             'meta_data': self.my_md}}}})
+        ds.ds_cfg['configure_secondary_nics'] = True
+        self.assertEqual(needle, ds.network_config['secondary_added'])
+
+    @mock.patch(DS_PATH + "._add_network_config_from_opc_imds")
+    @mock.patch(DS_PATH + ".cmdline.read_initramfs_config",
+                return_value={'some': 'config'})
+    @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True)
+    def test_secondary_nics_not_added_to_network_config_by_default(
+            self, _m_is_iscsi_root, _m_initramfs_config,
+            m_add_network_config_from_opc_imds):
+
+        def network_config_side_effect(network_config):
+            network_config['secondary_added'] = True
+
+        m_add_network_config_from_opc_imds.side_effect = (
+            network_config_side_effect)
+
+        distro = mock.MagicMock()
+        ds, _ = self._get_ds(distro=distro, patches={
+            '_is_platform_viable': {'return_value': True},
+            'crawl_metadata': {
+                'return_value': {
+                    MD_VER: {'system_uuid': self.my_uuid,
+                             'meta_data': self.my_md}}}})
+        self.assertNotIn('secondary_added', ds.network_config)
+
+    @mock.patch(DS_PATH + "._add_network_config_from_opc_imds")
+    @mock.patch(DS_PATH + ".cmdline.read_initramfs_config")
+    @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True)
+    def test_secondary_nic_failure_isnt_blocking(
+            self, _m_is_iscsi_root, m_initramfs_config,
+            m_add_network_config_from_opc_imds):
+
+        m_add_network_config_from_opc_imds.side_effect = Exception()
+
+        distro = mock.MagicMock()
+        ds, _ = self._get_ds(distro=distro, patches={
+            '_is_platform_viable': {'return_value': True},
+            'crawl_metadata': {
+                'return_value': {
+                    MD_VER: {'system_uuid': self.my_uuid,
+                             'meta_data': self.my_md}}}})
+        ds.ds_cfg['configure_secondary_nics'] = True
+        self.assertEqual(ds.network_config, m_initramfs_config.return_value)
+        self.assertIn('Failed to fetch secondary network configuration',
+                      self.logs.getvalue())
+
 
 @mock.patch(DS_PATH + "._read_system_uuid", return_value=str(uuid.uuid4()))
 class TestReadMetaData(test_helpers.HttprettyTestCase):
@@ -335,4 +463,86 @@ class TestLoadIndex(test_helpers.CiTestCase):
             oracle._load_index("\n".join(["meta_data.json", "user_data"])))
 
 
+class TestNetworkConfigFromOpcImds(test_helpers.CiTestCase):
+
+    with_logs = True
+
+    def setUp(self):
+        super(TestNetworkConfigFromOpcImds, self).setUp()
+        self.add_patch(DS_PATH + '.readurl', 'm_readurl')
+        self.add_patch(DS_PATH + '.get_interfaces_by_mac',
+                       'm_get_interfaces_by_mac')
+
+    def test_failure_to_readurl(self):
+        # readurl failures should just bubble out to the caller
+        self.m_readurl.side_effect = Exception('oh no')
+        with self.assertRaises(Exception) as excinfo:
+            oracle._add_network_config_from_opc_imds({})
+        self.assertEqual(str(excinfo.exception), 'oh no')
+
+    def test_empty_response(self):
+        # empty response error should just bubble out to the caller
+        self.m_readurl.return_value = ''
+        with self.assertRaises(Exception):
+            oracle._add_network_config_from_opc_imds([])
+
+    def test_invalid_json(self):
+        # invalid JSON error should just bubble out to the caller
+        self.m_readurl.return_value = '{'
+        with self.assertRaises(Exception):
+            oracle._add_network_config_from_opc_imds([])
+
+    def test_no_secondary_nics_does_not_mutate_input(self):
+        self.m_readurl.return_value = json.dumps([{}])
+        # We test this by passing in a non-dict to ensure that no dict
+        # operations are used; failure would be seen as exceptions
+        oracle._add_network_config_from_opc_imds(object())
+
+    def test_bare_metal_machine_skipped(self):
+        # nicIndex in the first entry indicates a bare metal machine
+        self.m_readurl.return_value = OPC_BM_SECONDARY_VNIC_RESPONSE
+        # We test this by passing in a non-dict to ensure that no dict
+        # operations are used
+        self.assertFalse(oracle._add_network_config_from_opc_imds(object()))
+        self.assertIn('bare metal machine', self.logs.getvalue())
+
+    def test_missing_mac_skipped(self):
+        self.m_readurl.return_value = OPC_VM_SECONDARY_VNIC_RESPONSE
+        self.m_get_interfaces_by_mac.return_value = {}
+
+        network_config = {'version': 1, 'config': [{'primary': 'nic'}]}
+        oracle._add_network_config_from_opc_imds(network_config)
+
+        self.assertEqual(1, len(network_config['config']))
+        self.assertIn(
+            'Interface with MAC 00:00:17:02:2b:b1 not found; skipping',
+            self.logs.getvalue())
+
+    def test_secondary_nic(self):
+        self.m_readurl.return_value = OPC_VM_SECONDARY_VNIC_RESPONSE
+        mac_addr, nic_name = '00:00:17:02:2b:b1', 'ens3'
+        self.m_get_interfaces_by_mac.return_value = {
+            mac_addr: nic_name,
+        }
+
+        network_config = {'version': 1, 'config': [{'primary': 'nic'}]}
+        oracle._add_network_config_from_opc_imds(network_config)
+
+        # The input is mutated
+        self.assertEqual(2, len(network_config['config']))
+
+        secondary_nic_cfg = network_config['config'][1]
+        self.assertEqual(nic_name, secondary_nic_cfg['name'])
+        self.assertEqual('physical', secondary_nic_cfg['type'])
+        self.assertEqual(mac_addr, secondary_nic_cfg['mac_address'])
+        self.assertEqual(9000, secondary_nic_cfg['mtu'])
+
+        self.assertEqual(1, len(secondary_nic_cfg['subnets']))
+        subnet_cfg = secondary_nic_cfg['subnets'][0]
+        # These values are hard-coded in OPC_VM_SECONDARY_VNIC_RESPONSE
+        self.assertEqual('10.0.0.231', subnet_cfg['address'])
+        self.assertEqual('24', subnet_cfg['netmask'])
+        self.assertEqual('10.0.0.1', subnet_cfg['gateway'])
+        self.assertEqual('manual', subnet_cfg['control'])
+
 # vi: ts=4 expandtab
diff --git a/doc/rtd/topics/datasources/oracle.rst b/doc/rtd/topics/datasources/oracle.rst
index f2383cee..98c4657c 100644
--- a/doc/rtd/topics/datasources/oracle.rst
+++ b/doc/rtd/topics/datasources/oracle.rst
@@ -8,7 +8,7 @@ This datasource reads metadata, vendor-data and user-data from
 
 Oracle Platform
 ---------------
-OCI provides bare metal and virtual machines.  In both cases, 
+OCI provides bare metal and virtual machines.  In both cases,
 the platform identifies itself via DMI data in the chassis asset tag
 with the string 'OracleCloud.com'.
 
@@ -22,5 +22,28 @@ Cloud-init has a specific datasource for Oracle in order to:
     implementation.
 
 
+Configuration
+-------------
+
+The following configuration can be set for the datasource in system
+configuration (in ``/etc/cloud/cloud.cfg`` or ``/etc/cloud/cloud.cfg.d/``).
+
+The settings that may be configured are:
+
+* **configure_secondary_nics**: A boolean, defaulting to False.  If set
+  to True on an OCI Virtual Machine, cloud-init will fetch networking
+  metadata from Oracle's IMDS and use it to configure the non-primary
+  network interface controllers in the system.  If set to True on an
+  OCI Bare Metal Machine, it will have no effect (though this may
+  change in the future).
+
+An example configuration with the default values is provided below:
+
+.. sourcecode:: yaml
+
+  datasource:
+   Oracle:
+    configure_secondary_nics: false
+
 .. _Oracle Compute Infrastructure: https://cloud.oracle.com/
 .. vi: textwidth=78