cloud-init-dev team mailing list archive

[Scott Moser <ssmoser2+ubuntu@xxxxxxxxx>]

Diff comments:

> diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
> index 7007d9e..d8170c3 100644
> --- a/cloudinit/sources/DataSourceAzure.py
> +++ b/cloudinit/sources/DataSourceAzure.py
> @@ -373,41 +391,84 @@ class DataSourceAzure(sources.DataSource):
>              except NonAzureDataSource:
>                  continue
>              except BrokenAzureDataSource as exc:
> -                raise exc
> +                msg = 'BrokenAzureDataSource: %s' % exc
> +                raise sources.InvalidMetaDataException(msg)
>              except util.MountFailedError:
>                  LOG.warning("%s was not mountable", cdev)
>                  continue
>  
>              if reprovision or self._should_reprovision(ret):
>                  ret = self._reprovision()
> -            (md, self.userdata_raw, cfg, files) = ret
> +            imds_md = get_metadata_from_imds(
> +                self.fallback_interface, retries=3)
> +            (md, userdata_raw, cfg, files) = ret
>              self.seed = cdev
> -            self.metadata = util.mergemanydict([md, DEFAULT_METADATA])
> -            self.cfg = util.mergemanydict([cfg, BUILTIN_CLOUD_CONFIG])
> +            crawled_data.update({
> +                'cfg': cfg,
> +                'files': files,
> +                'metadata': util.mergemanydict(
> +                    [md, {'imds': imds_md}]),
> +                'userdata_raw': userdata_raw})
>              found = cdev
>  
>              LOG.debug("found datasource in %s", cdev)
>              break
>  
>          if not found:
> -            return False
> +            raise sources.InvalidMetaDataException('No Azure metadata found')
>  
>          if found == ddir:
>              LOG.debug("using files cached in %s", ddir)
>  
>          seed = _get_random_seed()
>          if seed:
> -            self.metadata['random_seed'] = seed
> +            crawled_data['metadata']['random_seed'] = seed
> +        crawled_data['metadata']['instance-id'] = util.read_dmi_data(
> +            'system-uuid')
> +        return crawled_data
> +
> +    def _is_platform_viable(self):
> +        """Check platform environment to report if this datasource may run."""
> +        return _is_platform_viable(self.seed_dir)
> +
> +    def clear_cached_attrs(self, attr_defaults=()):
> +        """Reset any cached class attributes to defaults."""
> +        super(DataSourceAzure, self).clear_cached_attrs(attr_defaults)
> +        self._metadata_imds = sources.UNSET
> +
> +    def _get_data(self):
> +        """Crawl and process datasource metadata caching metadata as attrs.
> +
> +        @return: True on success, False on error, invalid or disabled
> +            datasource.
> +        """
> +        if not self._is_platform_viable():
> +            return False
> +        if self.distro and self.distro.name == 'ubuntu':
> +            maybe_remove_ubuntu_network_config_scripts()

can we move the abov e 2 lines down after the crawl_metadata?
that way we don't delete files if there is invalid metadata?

> +        try:
> +            crawled_data = util.log_time(
> +                        logfunc=LOG.debug, msg='Crawl of metadata service',
> +                        func=self.crawl_metadata)
> +        except sources.InvalidMetaDataException as e:
> +            LOG.warning('Could not crawl Azure metadata: %s', e)
> +            return False
> +
> +        # Process crawled data and augment with various config defaults
> +        self.cfg = util.mergemanydict(
> +            [crawled_data['cfg'], BUILTIN_CLOUD_CONFIG])
> +        self._metadata_imds = crawled_data['metadata']['imds']
> +        self.metadata = util.mergemanydict(
> +            [crawled_data['metadata'], DEFAULT_METADATA])
> +        self.userdata_raw = crawled_data['userdata_raw']
>  
>          user_ds_cfg = util.get_cfg_by_path(self.cfg, DS_CFG_PATH, {})
>          self.ds_cfg = util.mergemanydict([user_ds_cfg, self.ds_cfg])
>  
>          # walinux agent writes files world readable, but expects
>          # the directory to be protected.
> -        write_files(ddir, files, dirmode=0o700)
> -
> -        self.metadata['instance-id'] = util.read_dmi_data('system-uuid')
> -
> +        write_files(
> +            self.ds_cfg['data_dir'], crawled_data['files'], dirmode=0o700)
>          return True
>  
>      def device_name_to_device(self, name):
> @@ -1025,6 +1079,162 @@ def load_azure_ds_dir(source_dir):
>      return (md, ud, cfg, {'ovf-env.xml': contents})
>  
>  
> +def parse_network_config(imds_metadata):
> +    """Convert imds_metadata dictionary to network v2 configuration.
> +
> +    Parses network configuration from imds metadata if present or generate
> +    fallback network config excluding mlx4_core devices.
> +
> +    @param: imds_metadata: Dict of content read from IMDS network service.
> +    @return: Dictionary containing network version 2 standard configuration.

i

> +    """
> +    if imds_metadata != sources.UNSET and imds_metadata:
> +        netconfig = {'version': 2, 'ethernets': {}}
> +        LOG.debug('Azure: generating network configuration from IMDS')
> +        network_metadata = imds_metadata['network']
> +        for idx, intf in enumerate(network_metadata['interface']):
> +            nicname = 'eth{idx}'.format(idx=idx)
> +            dev_config = {}
> +            for addr4 in intf['ipv4']['ipAddress']:
> +                privateIpv4 = addr4['privateIpAddress']
> +                if privateIpv4:
> +                    if dev_config.get('dhcp4', False):
> +                        # Append static address config for nic > 1
> +                        netPrefix = intf['ipv4']['subnet'][0].get(
> +                            'prefix', '24')
> +                        if not dev_config.get('addresses'):
> +                            dev_config['addresses'] = []
> +                        dev_config['addresses'].append(
> +                            '{ip}/{prefix}'.format(
> +                                ip=privateIpv4, prefix=netPrefix))
> +                    else:
> +                        dev_config['dhcp4'] = True
> +            for addr6 in intf['ipv6']['ipAddress']:
> +                privateIpv6 = addr6['privateIpAddress']
> +                if privateIpv6:
> +                    dev_config['dhcp6'] = True
> +                    break
> +            if dev_config:
> +                mac = ':'.join(re.findall(r'..', intf['macAddress']))
> +                dev_config.update(
> +                    {'match': {'macaddress': mac.lower()},
> +                     'set-name': nicname})
> +                netconfig['ethernets'][nicname] = dev_config
> +    else:
> +        blacklist = ['mlx4_core']
> +        LOG.debug('Azure: generating fallback configuration')
> +        # generate a network config, blacklist picking mlx4_core devs
> +        netconfig = net.generate_fallback_config(
> +            blacklist_drivers=blacklist, config_driver=True)
> +    return netconfig
> +
> +
> +def get_metadata_from_imds(fallback_nic, retries):
> +    """Query Azure's network metadata service, returning a dictionary.
> +
> +    If network is not up, setup ephemeral dhcp on fallback_nic to talk to the
> +    IMDS. For more info on IMDS:
> +        https://docs.microsoft.com/en-us/azure/virtual-machines/windows/instance-metadata-service
> +
> +    @param fallback_nic: String. The name of the nic which requires active
> +        networ in order to query IMDS.
> +    @param retries: The number of retries of the IMDS_URL.
> +
> +    @return: A dict of instance metadata containing compute and network
> +        info.
> +    """
> +    if net.is_up(fallback_nic):
> +        return util.log_time(
> +            logfunc=LOG.debug,
> +            msg='Crawl of Azure Instance Metadata Service (IMDS)',
> +            func=_get_metadata_from_imds, args=(retries,))
> +    else:
> +        with EphemeralDHCPv4(fallback_nic):
> +            return util.log_time(
> +                logfunc=LOG.debug,
> +                msg='Crawl of Azure Instance Metadata Service (IMDS)',
> +                func=_get_metadata_from_imds, args=(retries,))
> +
> +
> +def _get_metadata_from_imds(retries):
> +
> +    def retry_on_url_error(msg, exception):
> +        if isinstance(exception, UrlError) and exception.code == 404:
> +            return True  # Continue retries
> +        return False  # Stop retries on all other exceptions, including 404s
> +
> +    url = IMDS_URL + "instance?api-version=2017-12-01"
> +    headers = {"Metadata": "true"}
> +    try:
> +        response = readurl(
> +            url, timeout=1, headers=headers, retries=retries,
> +            exception_cb=retry_on_url_error)
> +    except Exception as e:
> +        LOG.debug('Ignoring IMDS instance metadata: %s', e)
> +        return {}
> +    try:
> +        return util.load_json(str(response))
> +    except json.decoder.JSONDecodeError:
> +        LOG.warning(
> +            'Ignoring non-json IMDS instance metadata: %s', str(response))
> +    return {}
> +
> +
> +def maybe_remove_ubuntu_network_config_scripts(paths=None):
> +    """Remove Azure-specific ubuntu network config for non-primary nics.
> +
> +    @param paths: List of networking scripts or directories to remove when
> +        present.
> +
> +    In certain supported ubuntu images, static udev rules or netplan yaml
> +    config is delivered in the base ubuntu image to support dhcp on any
> +    additional interfaces which get attached by a customer at some point
> +    after initial boot. Since the Azure datasource can now regenerate
> +    network configuration as metadata reports these new devices, we no longer
> +    want the udev rules or netplan's 90-azure-hotplug.yaml to configure
> +    networking on eth1 or greater as it might collide with cloud-init's
> +    configuration.
> +
> +    Remove the any existing extended network scripts if the datasource is
> +    enabled to write network per-boot.
> +    """
> +    if not paths:
> +        paths = UBUNTU_EXTENDED_NETWORK_SCRIPTS
> +    logged = False
> +    for path in paths:
> +        if os.path.exists(path):
> +            if not logged:
> +                LOG.info(
> +                    'Removing Ubuntu extended network scripts because'
> +                    ' cloud-init updates Azure network configuration on the'
> +                    ' following event: %s.',
> +                    EventType.BOOT)
> +                logged = True
> +            if os.path.isdir(path):
> +                util.del_dir(path)
> +            else:
> +                util.del_file(path)
> +
> +
> +def _is_platform_viable(seed_dir):
> +    """Check platform environment to report if this datasource may run."""

i dont think this function lines up exactly with behavior of dscheck_Azure

> +    asset_tag = util.read_dmi_data('chassis-asset-tag')
> +    if asset_tag == AZURE_CHASSIS_ASSET_TAG:
> +        return True

this part is good, lining up with:
    is_azure_chassis && return $DS_FOUND

> +    LOG.debug("Non-Azure DMI asset tag '%s' discovered.", asset_tag)
> +    for path in (AGENT_SEED_DIR, seed_dir):
> +        if os.path.exists(os.path.join(path, 'ovf-env.xml')):
> +            return True

Why do we check the AGENT_SEED_DIR ?
ds_identify does not do that.

> +    if util.which('systemd-detect-virt'):
> +        (virt_type, _err) = util.subp(
> +            ['systemd-detect-virt'], rcs=[0, 1], capture=True)
> +        if virt_type.strip() == 'microsoft':
> +            return True

ds_dentify returns NOT_FOUND (False) unless DI_VIRT == microsoft.
Thats differnt than returning true if DI_VIRT == microsoft 
in that ds_identify does not continue on and look at label if virt is not microsoft.

> +    if util.find_devs_with(criteria='LABEL=rd_rdfe_*'):
> +        return True

does this actually work? I dont think it does.
you can write it yourself failrly easily though using recently added 'blkid':

rdfe_disks = [k for k, v in util.blkid().items() 
              if v.get("LABEL", "").startswith("rd_rdfe_")]
if rdfe_disks:
    return True

> +    return False
> +
> +
>  class BrokenAzureDataSource(Exception):
>      pass
>  


-- 
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/352660
Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:feature/azure-network-per-boot into cloud-init:master.