cloud-init-dev team mailing list archive

[Ryan Harper <ryan.harper@xxxxxxxxxxxxx>]

I'm not super happy with the Maintenance name.  Maybe SystemEvent?  Also, I think we need other classes of Event types;  DatasourceEvents (METADATA_REFRESHED) or PlatformEvents;  Maybe we'll be notified of a pending instance migration, pending shutdown, etc.  

Diff comments:

> diff --git a/cloudinit/hotplug.py b/cloudinit/hotplug.py
> new file mode 100644
> index 0000000..c5ba1af
> --- /dev/null
> +++ b/cloudinit/hotplug.py
> @@ -0,0 +1,15 @@
> +# This file is part of cloud-init. See LICENSE file for license information.
> +"""Classes and functions related to hotplug and eventing."""
> +
> +# Maintenance events describing the source generating a maintenance request.
> +class MaintenanceEvent(object):
> +    NONE = 0x0           # React to no maintenance events
> +    BOOT = 0x1           # Any system boot or reboot event
> +    DEVICE_ADD = 0x2     # Any new device added
> +    DEVICE_REMOVE = 0x4  # Any device removed
> +    DEVICE_CHANGE = 0x8  # Any device metadata change

I think we should expand the list of events to be very granular, and we can combine some flags into common groups.
For example, we may want to react to REBOOT differently than BOOT.
I think the comment 'metadata' is unclear;  Do you mean any changes to the device itself or changes to metadata about a particular device?

> +    ANY = 0xF            # Match any defined MaintenanceEvents
> +
> +MAINTENANCE_EVENT_STR = dict(
> +    (attr, getattr(MaintenanceEvent, attr))
> +    for attr in MaintenanceEvent.__dict__.keys() if attr.isupper())
> 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
> @@ -19,6 +19,7 @@ from cloudinit.atomic_helper import write_json
>  from cloudinit import importer
>  from cloudinit import log as logging
>  from cloudinit import net
> +from cloudinit.hotplug import MaintenanceEvent

cloudinit.events ?

>  from cloudinit import type_utils
>  from cloudinit import user_data as ud
>  from cloudinit import util
> @@ -102,6 +103,13 @@ class DataSource(object):
>      url_timeout = 10    # timeout for each metadata url read attempt
>      url_retries = 5     # number of times to retry url upon 404
>  
> +    # Subclasses can define a mask of supported MaintenanceEvents during
> +    # which the datasource will regenerate network_configuration. For example:
> +    # network_maintenance_mask = MEvent.BOOT|MEvent.DEVICE_ADD
> +
> +    # Default behavior, perform no network update for any maintenance event
> +    network_maintenance_mask = MaintenanceEvent.NONE

I think the default is .BOOT right?  If not how do we reconcile the current default behavior of per-instance;  maybe we need a NEW_INSTANCE event?

> +
>      def __init__(self, sys_cfg, distro, paths, ud_proc=None):
>          self.sys_cfg = sys_cfg
>          self.distro = distro
> @@ -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'):

I think we need to open this up to allow the Datasource subclasses to define a set of attrs that should be cleared; and we have a clear_cache() base method so the subclass can override implementation.

> +                # 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):

refresh_metadata?

> +        """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))
> +            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):

This looks strange.  The property is always going to be the most current; it doesn't need to regenerate; it's always returning the "current" config;  Whether the config is different is controlled by whether we've refreshed metadata from the datasource.

>          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.