← Back to team overview

cloud-init-dev team mailing list archive

Re: [Merge] ~chad.smith/cloud-init:feature/maintain-network-on-boot into cloud-init:master

 


Diff comments:

> diff --git a/cloudinit/sources/DataSourceAltCloud.py b/cloudinit/sources/DataSourceAltCloud.py
> index 24fd65f..cd6ab17 100644
> --- a/cloudinit/sources/DataSourceAltCloud.py
> +++ b/cloudinit/sources/DataSourceAltCloud.py
> @@ -114,7 +114,7 @@ class DataSourceAltCloud(sources.DataSource):
>  
>          return 'UNKNOWN'
>  
> -    def _get_data(self):
> +    def _get_data(self, clear_cache=False):

+1 on this suggestion, I've started to distill crawl_data from get_data in other Ec2 and OpenStack datasources.It's going to be a quite large branch to move all datasources to those distinct operations (read(crawl_metadata) vs persist(get_data)). So I don't want to make that move in this branch if we can avoid it.

>          '''
>          Description:
>              User Data is passed to the launching instance which
> diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py
> index 90d7457..af876ad 100644
> --- a/cloudinit/sources/__init__.py
> +++ b/cloudinit/sources/__init__.py
> @@ -134,12 +142,25 @@ class DataSource(object):
>              'region': self.region,
>              'availability-zone': self.availability_zone}}
>  
> -    def get_data(self):
> +    def get_data(self, clear_cache=False):
>          """Datasources implement _get_data to setup metadata and userdata_raw.
>  
>          Minimally, the datasource should return a boolean True on success.
> +        @param use_cache: Boolean set true to re-use data cache if present.
> +           Value of False, will clear any cached data, re-crawling all
> +           instance metadata.
>          """
> -        return_value = self._get_data()
> +        if clear_cache:
> +            if hasattr(self, '_network_config'):

Ryan I thought about this too, though those datasources might also have to provide default values expected on clear as well. Maybe the datasource.cached_attribute_defaults could be an n-tuple ('attr-name', <default_value>)  and the DataSource.clear_cached_data would set values back to their datasource default.

> +                # Clear network config property so it is regenerated from md.
> +                setattr(self, '_network_config', None)
> +            self.userdata = None
> +            self.metadata = {}
> +            self.userdata_raw = None
> +            self.vendordata = None
> +            self.vendordata_raw = None
> +
> +        return_value = self._get_data(clear_cache=clear_cache)
>          json_file = os.path.join(self.paths.run_dir, INSTANCE_JSON_FILE)
>          if not return_value:
>              return return_value
> @@ -416,6 +440,32 @@ class DataSource(object):
>      def get_package_mirror_info(self):
>          return self.distro.get_package_mirror_info(data_source=self)
>  
> +    def maintain_metadata(self, maintenance_event):

+1, changing

> +        """Refresh cached metadata if the datasource handles this event.
> +
> +        The datasource defines a network_maintenance_mask attribute which
> +        authorizes refreshing all cached metadata due to any number of
> +        supported MaintenenanceEvent types.
> +
> +        @param maintenance_event: The source MaintenanceEvent type
> +            observed to which the datasource may react.
> +
> +        @return True if the datasource has updated cached metadata due to the
> +           the provided maintenance_event type. MaintenanceEvents will be
> +           something like boot, configchange, device_add, device_remove etc.
> +        """
> +        if bool(maintenance_event & self.network_maintenance_mask):
> +            LOG.debug(
> +                "Re-crawling datasource metadata due to maintenance event: '%s'",
> +                MAINTENANCE_EVENT_STR.get(maintenance_event, maintenance_event))

fair, was trying to catch a bad traceback if for some-reason we have an unexpected caller. for an datasource which extends for events not defined in the stock MaintenanceEvent class. A corner case we shouldn't worry about I suppose.

> +            result = self.get_data(clear_cache=True)
> +            if result:
> +                return True
> +            else:
> +                LOG.warning(
> +                    'Re-crawling metadata reported invalid datasource type')
> +        return False
> +
>      def check_instance_id(self, sys_cfg):
>          # quickly (local check only) if self.instance_id is still
>          return False
> @@ -442,7 +492,7 @@ class DataSource(object):
>          return default
>  
>      @property
> -    def network_config(self):
> +    def network_config(self, regenerate=False):

Hrm, not sure how this got in here, I was probably going at this earlier and then relized it was a @property and as such we can't provide additional params.

>          return None
>  
>      @property


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


References