cloud-init-dev team mailing list archive

[Ryan Harper <ryan.harper@xxxxxxxxxxxxx>]

Diff comments:

> diff --git a/cloudinit/cmd/main.py b/cloudinit/cmd/main.py
> index ce3c10d..35d9a55 100644
> --- a/cloudinit/cmd/main.py
> +++ b/cloudinit/cmd/main.py
> @@ -372,6 +372,7 @@ def main_init(name, args):
>              LOG.debug("[%s] %s is in local mode, will apply init modules now.",
>                        mode, init.datasource)
>  
> +    init.setup_datasource()

comment here about why:  some datasources can only complete their setup *after* networking is up

>      # update fully realizes user-data (pulling in #include if necessary)
>      init.update()
>      # Stage 7
> diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
> index 4fe0d63..d9132d0 100644
> --- a/cloudinit/sources/DataSourceAzure.py
> +++ b/cloudinit/sources/DataSourceAzure.py
> @@ -331,6 +335,7 @@ class DataSourceAzureNet(sources.DataSource):
>          if asset_tag != AZURE_CHASSIS_ASSET_TAG:
>              LOG.debug("Non-Azure DMI asset tag '%s' discovered.", asset_tag)
>              return False
> +

whitespace damage

>          ddir = self.ds_cfg['data_dir']
>  
>          candidates = [self.seed_dir]
> @@ -389,6 +395,33 @@ class DataSourceAzureNet(sources.DataSource):
>          # the directory to be protected.
>          write_files(ddir, files, dirmode=0o700)
>  
> +        self.metadata['instance-id'] = util.read_dmi_data('system-uuid')
> +
> +        return True
> +
> +    def device_name_to_device(self, name):
> +        return self.ds_cfg['disk_aliases'].get(name)
> +
> +    def get_config_obj(self):
> +        return self.cfg
> +
> +    def check_instance_id(self, sys_cfg):
> +        # quickly (local check only) if self.instance_id is still valid
> +        return sources.instance_id_matches_system_uuid(self.get_instance_id())
> +
> +    def setup(self, is_new_instance):

kinda want to bikeshed on this name a bit;  more of a register_instance_with_cloud or something.  Do we have any other datasources that do something like this?

> +        if self._negotiated is False:
> +            LOG.debug("negotiating for %s", self.get_instance_id())

negotiating for instance-id %s

Is this more like 'attempting register instance-id %s with azure network fabric' ?

> +            fabric_data = self._negotiate()
> +            LOG.debug("negotiating returned %s", fabric_data)

We should probably document what we expect this fabric_data to look like, I still don't know what it is.

> +            if fabric_data:
> +                self.metadata.update(fabric_data)
> +            self._negotiated = True
> +        else:
> +            LOG.debug("negotiating already done for %s",
> +                      self.get_instance_id())
> +
> +    def _negotiate(self):
>          if self.ds_cfg['agent_command'] == AGENT_START_BUILTIN:
>              self.bounce_network_with_azure_hostname()
>  
> @@ -398,31 +431,64 @@ class DataSourceAzureNet(sources.DataSource):
>          else:
>              metadata_func = self.get_metadata_from_agent
>  
> +        LOG.debug("negotiating with fabric via %s",

via 'agent_command' %s

> +                  self.ds_cfg['agent_command'])
>          try:
>              fabric_data = metadata_func()
>          except Exception as exc:
>              LOG.info("Error communicating with Azure fabric; assume we aren't"
>                       " on Azure.", exc_info=True)
>              return False
> -        self.metadata['instance-id'] = util.read_dmi_data('system-uuid')
> -        self.metadata.update(fabric_data)
> -
> -        return True
> -
> -    def device_name_to_device(self, name):
> -        return self.ds_cfg['disk_aliases'].get(name)
>  
> -    def get_config_obj(self):
> -        return self.cfg
> -
> -    def check_instance_id(self, sys_cfg):
> -        # quickly (local check only) if self.instance_id is still valid
> -        return sources.instance_id_matches_system_uuid(self.get_instance_id())
> +        return fabric_data
>  
>      def activate(self, cfg, is_new_instance):
>          address_ephemeral_resize(is_new_instance=is_new_instance)
>          return
>  
> +    @property
> +    def network_config(self):
> +        """Generate a network config like net.generate_fallback_network() with
> +           the following execptions.
> +
> +           1. Probe the drivers of the net-devices present and inject them in
> +              the network configuration under params: driver: <driver> value
> +           2. If the driver value is 'mlx4_core', the control mode should be
> +              set to manual.  The device will be later used to build a bond,
> +              for now we want to ensure the device gets named but does not
> +              break any network configuration
> +        """
> +        blacklist = ['mlx4_core']

we should set this as a default in the builtins ds configuration I think.

> +        if not self._network_config:
> +            LOG.debug('Azure: generating fallback configuration')
> +            # generate a network config, blacklist picking any mlx4_core devs
> +            netconfig = net.generate_fallback_config(
> +                blacklist_drivers=blacklist, config_driver=True)
> +
> +            # if we have any blacklisted devices, update the network_config to
> +            # include the device, mac, and driver values, but with no ip
> +            # config; this ensures udev rules are generated but won't affect
> +            # ip configuration
> +            bl_found = 0
> +            for bl_dev in [dev for dev in net.get_devicelist()
> +                           if net.device_driver(dev) in blacklist]:
> +                bl_found += 1
> +                cfg = {
> +                    'type': 'physical',
> +                    'name': 'vf%d' % bl_found,
> +                    'mac_address': net.get_interface_mac(bl_dev),
> +                    'params': {
> +                        'driver': net.device_driver(bl_dev),
> +                        'device_id': net.device_devid(bl_dev),
> +                    },
> +                }
> +                netconfig['config'].append(cfg)
> +
> +            # switch to network mode after generating a config

drop this comment

> +            self._network_config = netconfig
> +
> +        return self._network_config
> +
>  
>  def _partitions_on_device(devpath, maxnum=16):
>      # return a list of tuples (ptnum, path) for each part on devpath
> diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py
> index c3ce36d..952caf3 100644
> --- a/cloudinit/sources/__init__.py
> +++ b/cloudinit/sources/__init__.py
> @@ -251,10 +251,23 @@ class DataSource(object):
>      def first_instance_boot(self):
>          return
>  
> +    def setup(self, is_new_instance):
> +        """setup(is_new_instance)
> +
> +        This is called before user-data and vendor-data have been processed.
> +
> +        Unless the datasource has set mode to 'local', then networking
> +        per 'fallback' or per 'network_config' will have been written and
> +        brought up the OS at this point.
> +        """
> +        return

We could assert those requirements,  dsmode == DS_MODE_LOCAL and ds.network_config is not None

> +
>      def activate(self, cfg, is_new_instance):
>          """activate(cfg, is_new_instance)
>  
> -        This is called before the init_modules will be called.
> +        This is called before the init_modules will be called but after
> +        the user-data and vendor-data have been fully processed.
> +
>          The cfg is fully up to date config, it contains a merged view of
>             system config, datasource config, user config, vendor config.
>          It should be used rather than the sys_cfg passed to __init__.
> diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py
> index 7d33daf..2f7c1bb 100644
> --- a/tests/unittests/test_datasource/test_azure.py
> +++ b/tests/unittests/test_datasource/test_azure.py
> @@ -142,6 +142,10 @@ scbus-1 on xpt0 bus 0
>          def _invoke_agent(cmd):
>              data['agent_invoked'] = cmd
>  
> +        def _get_dsmodes(config):
> +            # sources.DSMODE_NETWORK
> +            return 'net'
> +

drop this

>          def _wait_for_files(flist, _maxwait=None, _naplen=None):
>              data['waited'] = flist
>              return []
> @@ -171,6 +175,7 @@ scbus-1 on xpt0 bus 0
>          self.apply_patches([
>              (dsaz, 'list_possible_azure_ds_devs', dsdevs),
>              (dsaz, 'invoke_agent', _invoke_agent),
> +            (dsaz, 'get_dsmodes', _get_dsmodes),

drop this

>              (dsaz, 'wait_for_files', _wait_for_files),
>              (dsaz, 'pubkeys_from_crt_files', _pubkeys_from_crt_files),
>              (dsaz, 'perform_hostname_bounce', mock.MagicMock()),
> @@ -568,6 +651,9 @@ class TestAzureBounce(TestCase):
>          self.patches.enter_context(
>              mock.patch.object(dsaz, 'get_metadata_from_fabric',
>                                mock.MagicMock(return_value={})))
> +        self.patches.enter_context(
> +            mock.patch.object(dsaz, 'get_dsmodes',
> +                              mock.MagicMock(return_value='net')))

drop this

>  
>          def _dmi_mocks(key):
>              if key == 'system-uuid':


-- 
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/326373
Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:azure_run_local into cloud-init:master.