cloud-init-dev team mailing list archive

[Ryan Harper <ryan.harper@xxxxxxxxxxxxx>]

Ryan Harper has proposed merging ~raharper/cloud-init:fix/ds_update_events into cloud-init:master.

Commit message:
DataSource: use class property to allow subclasses to override update_events
    
Subclasses of DataSource, like DataSourceAzure would update the .update_events
attribute of the base-class; this value is propagated to any future subclass
that gets instantiated.  Therefore cloud-init would *leak* these additional
update_events into subclasses which do not use them.

Convert the class attribute into a property and allow subclasses to
override the property to append/modify the attribute but only for
the subclass.
    
LP: #1819913


Requested reviews:
  cloud-init commiters (cloud-init-dev)
Related bugs:
  Bug #1819913 in cloud-init: "cloud-init on xenial may generate network config on every boot"
  https://bugs.launchpad.net/cloud-init/+bug/1819913

For more details, see:
https://code.launchpad.net/~raharper/cloud-init/+git/cloud-init/+merge/364403
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~raharper/cloud-init:fix/ds_update_events into cloud-init:master.

diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
index eccbee5..cfe2d69 100644
--- a/cloudinit/sources/DataSourceAzure.py
+++ b/cloudinit/sources/DataSourceAzure.py
@@ -6,6 +6,7 @@
 
 import base64
 import contextlib
+import copy
 import crypt
 from functools import partial
 import json
@@ -279,8 +280,6 @@ class DataSourceAzure(sources.DataSource):
             BUILTIN_DS_CONFIG])
         self.dhclient_lease_file = self.ds_cfg.get('dhclient_lease_file')
         self._network_config = None
-        # Regenerate network config new_instance boot and every boot
-        self.update_events['network'].add(EventType.BOOT)
         self._ephemeral_dhcp_ctx = None
 
     def __str__(self):
@@ -656,6 +655,15 @@ class DataSourceAzure(sources.DataSource):
         return
 
     @property
+    def update_events(self):
+        if not self._update_events:
+            events = copy.deepcopy(super().update_events)
+            # Regenerate network_config on each boot
+            events['network'].add(EventType.BOOT)
+            self._update_events = events
+        return self._update_events
+
+    @property
     def network_config(self):
         """Generate a network config like net.generate_fallback_network() with
            the following exceptions.
diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py
index ac28f1d..3bbfcc3 100644
--- a/cloudinit/sources/DataSourceEc2.py
+++ b/cloudinit/sources/DataSourceEc2.py
@@ -8,6 +8,7 @@
 #
 # This file is part of cloud-init. See LICENSE file for license information.
 
+import copy
 import os
 import time
 
@@ -283,6 +284,21 @@ class DataSourceEc2(sources.DataSource):
             return None
 
     @property
+    def update_events(self):
+        if not self._update_events:
+            events = copy.deepcopy(super().update_events)
+            # Non-VPC (aka Classic) Ec2 instances need to rewrite the
+            # network config file every boot due to MAC address change.
+            # RELEASE_BLOCKER: Xenial debian/postinst needs to add
+            # EventType.BOOT on upgrade path for classic.
+            if self.is_classic_instance():
+                LOG.debug('Ec2: Classic Instance, adding update_event: %s',
+                          EventType.BOOT)
+                events['network'].add(EventType.BOOT)
+            self._update_events = events
+        return self._update_events
+
+    @property
     def region(self):
         if self.cloud_name == CloudNames.AWS:
             region = self.identity.get('region')
@@ -334,6 +350,7 @@ class DataSourceEc2(sources.DataSource):
         if isinstance(net_md, dict):
             result = convert_ec2_metadata_network_config(
                 net_md, macs_to_nics=macs_to_nics, fallback_nic=iface)
+<<<<<<< cloudinit/sources/DataSourceEc2.py
 
             # RELEASE_BLOCKER: xenial should drop the below if statement,
             # because the issue being addressed doesn't exist pre-netplan.
@@ -345,6 +362,8 @@ class DataSourceEc2(sources.DataSource):
             # network config file every boot due to MAC address change.
             if self.is_classic_instance():
                 self.update_events['network'].add(EventType.BOOT)
+=======
+>>>>>>> cloudinit/sources/DataSourceEc2.py
         else:
             LOG.warning("Metadata 'network' key not valid: %s.", net_md)
         self._network_config = result
diff --git a/cloudinit/sources/DataSourceScaleway.py b/cloudinit/sources/DataSourceScaleway.py
index b573b38..57cad13 100644
--- a/cloudinit/sources/DataSourceScaleway.py
+++ b/cloudinit/sources/DataSourceScaleway.py
@@ -5,6 +5,7 @@
 # Scaleway API:
 # https://developer.scaleway.com/#metadata
 
+import copy
 import json
 import os
 import socket
@@ -171,7 +172,6 @@ def query_data_api(api_type, api_address, retries, timeout):
 
 class DataSourceScaleway(sources.DataSource):
     dsname = "Scaleway"
-    update_events = {'network': [EventType.BOOT_NEW_INSTANCE, EventType.BOOT]}
 
     def __init__(self, sys_cfg, distro, paths):
         super(DataSourceScaleway, self).__init__(sys_cfg, distro, paths)
@@ -222,6 +222,15 @@ class DataSourceScaleway(sources.DataSource):
         return True
 
     @property
+    def update_events(self):
+        if not self._update_events:
+            events = copy.deepcopy(super().update_events)
+            # Regenerate network_config on each boot
+            events['network'].add(EventType.BOOT)
+            self._update_events = events
+        return self._update_events
+
+    @property
     def network_config(self):
         """
         Configure networking according to data received from the
diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py
index e6966b3..75f2d2a 100644
--- a/cloudinit/sources/__init__.py
+++ b/cloudinit/sources/__init__.py
@@ -42,6 +42,8 @@ EXPERIMENTAL_TEXT = (
     "EXPERIMENTAL: The structure and format of content scoped under the 'ds'"
     " key may change in subsequent releases of cloud-init.")
 
+# Default: generate network config on new instance id (first boot).
+DEFAULT_UPDATE_EVENTS = {'network': set([EventType.BOOT_NEW_INSTANCE])}
 
 # File in which public available instance meta-data is written
 # security-sensitive key values are redacted from this world-readable file
@@ -164,8 +166,7 @@ class DataSource(object):
     # A datasource which supports writing network config on each system boot
     # would call update_events['network'].add(EventType.BOOT).
 
-    # Default: generate network config on new instance id (first boot).
-    update_events = {'network': set([EventType.BOOT_NEW_INSTANCE])}
+    _update_events = None
 
     # N-tuple listing default values for any metadata-related class
     # attributes cached on an instance by a process_data runs. These attribute
@@ -589,6 +590,12 @@ class DataSource(object):
     def get_package_mirror_info(self):
         return self.distro.get_package_mirror_info(data_source=self)
 
+    @property
+    def update_events(self):
+        if not self._update_events:
+            self._update_events = dict(DEFAULT_UPDATE_EVENTS)
+        return self._update_events
+
     def update_metadata(self, source_event_types):
         """Refresh cached metadata if the datasource supports this event.
 
diff --git a/cloudinit/tests/test_stages.py b/cloudinit/tests/test_stages.py
index 94b6b25..61eb5c8 100644
--- a/cloudinit/tests/test_stages.py
+++ b/cloudinit/tests/test_stages.py
@@ -222,7 +222,7 @@ class TestInit(CiTestCase):
 
         self.init._find_networking_config = fake_network_config
         self.init.datasource = FakeDataSource(paths=self.init.paths)
-        self.init.datasource.update_events = {'network': [EventType.BOOT]}
+        self.init.datasource._update_events = {'network': [EventType.BOOT]}
         self.init.apply_network_config(True)
         self.init.distro.apply_network_config_names.assert_called_with(net_cfg)
         self.init.distro.apply_network_config.assert_called_with(