Made some changes locally, still thinking about one other.  (Not pushing them 
up until all comments are addressed, so I don't have to go digging to find the 
current set of comments.)

Diff comments:

> diff --git a/cloudinit/sources/DataSourceOracle.py 
> b/cloudinit/sources/DataSourceOracle.py
> index 76cfa38..f18e5a0 100644
> --- a/cloudinit/sources/DataSourceOracle.py
> +++ b/cloudinit/sources/DataSourceOracle.py
> @@ -28,8 +28,70 @@ 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/'
> +
> +
> +def _network_config_from_opc_imds(network_config):
> +    """
> +    Fetch data from Oracle's IMDS and generate secondary NIC config.
> +
> +    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.
> +
> +    :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).
> +
> +    :return:
> +        The ``network_config`` dict with secondary NICs added.
> +    """
> +    resp = readurl(VNIC_METADATA_URL)
> +    vnics = json.loads(str(resp))
> +
> +    if 'nicIndex' in vnics[0]:
> +        LOG.debug('VNIC metadata indicates this is a bare metal machine;'

I'm now wondering if _network_config_from_opc_imds should return something 
specific (None?) in this case, so that DataSourceOracle.network_config can 
decide what to do with this information (rather than encoding the logging logic 
in here).

(I'm not a huge fan of the mutate-and-return thing that's going on ATM anyway, 
so changing the signature wouldn't be the worst thing in the world.)

I'm going to sleep on this and decide what to do in the morning.

> +                  ' skipping secondary VNIC configuration.')
> +        return network_config
> +
> +    interfaces_by_mac = get_interfaces_by_mac()
> +
> +    for vnic_dict in vnics[1:]:
> +        mac_address = vnic_dict['macAddr'].lower()
> +        if mac_address not in interfaces_by_mac:
> +            LOG.info('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': 9000,

Done, locally.

> +            'subnets': [subnet],
> +        })
> +
> +    return network_config
>  
>  
>  class DataSourceOracle(sources.DataSource):
> @@ -121,6 +190,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'):

Done locally (for this MP, not that bug).

> +                try:
> +                    self._network_config = _network_config_from_opc_imds(
> +                        self._network_config)
> +                except Exception:
> +                    util.logexc(
> +                        LOG,
> +                        "Failed to fetch secondary network configuration!")
>          return self._network_config
>  
>  


-- 
https://code.launchpad.net/~daniel-thewatkins/cloud-init/+git/cloud-init/+merge/371053
Your team cloud-init commiters is requested to review the proposed merge of 
~daniel-thewatkins/cloud-init/+git/cloud-init:oci-vnic into cloud-init:master.

_______________________________________________
Mailing list: https://launchpad.net/~cloud-init-dev
Post to     : cloud-init-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~cloud-init-dev
More help   : https://help.launchpad.net/ListHelp

Reply via email to