cloud-init-dev team mailing list archive
-
cloud-init-dev team
-
Mailing list archive
-
Message #05109
[Merge] ~chad.smith/cloud-init:feature/maintain-network-on-boot into cloud-init:master
Chad Smith has proposed merging ~chad.smith/cloud-init:feature/maintain-network-on-boot into cloud-init:master.
Commit message:
work-in-progress Strawman for MaintenanceEvent discussion.
Base-level MaintenanceEvent class and DataSource.maintain_metadata behavior to allow clearing cached instance data and re-crawling all metadata sources.
I'll build this out today and review any comments/suggestions folks want to attach while I'm developing it.
Requested reviews:
Server Team CI bot (server-team-bot): continuous-integration
cloud-init commiters (cloud-init-dev)
For more details, see:
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.
diff --git a/cloudinit/event.py b/cloudinit/event.py
new file mode 100644
index 0000000..f7b311f
--- /dev/null
+++ b/cloudinit/event.py
@@ -0,0 +1,17 @@
+# This file is part of cloud-init. See LICENSE file for license information.
+
+"""Classes and functions related to event handling."""
+
+
+# Event types which can generate maintenance requests for cloud-init.
+class EventType(object):
+ BOOT = "System boot"
+ BOOT_NEW_INSTANCE = "New instance first boot"
+
+ # TODO: Cloud-init will grow support for the follow event types:
+ # UDEV
+ # METADATA_CHANGE
+ # USER_REQUEST
+
+
+# vi: ts=4 expandtab
diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py
index 90d7457..02be572 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.event import EventType
from cloudinit import type_utils
from cloudinit import user_data as ud
from cloudinit import util
@@ -102,6 +103,26 @@ class DataSource(object):
url_timeout = 10 # timeout for each metadata url read attempt
url_retries = 5 # number of times to retry url upon 404
+ # The datasource defines a list of supported EventTypes during which
+ # the datasource can react to changes in metadata and regenerate
+ # network configuration on metadata changes.
+ # A datasource which supports writing network config on each system boot
+ # would set maintenance_events = [EventType.BOOT].
+
+ # Default: generate network config on new instance id (first boot).
+ maintenance_events = [EventType.BOOT_NEW_INSTANCE]
+
+ # N-tuple listing default values for any metadata-related class
+ # attributes cached on an instance by a get_data runs. These attribute
+ # values are reset via clear_cached_data during any of the supported
+ # maintenance_events.
+ cached_attr_defaults = (
+ ('ec2_metadata', UNSET), ('network_json', UNSET),
+ ('metadata', {}), ('userdata', None), ('userdata_raw', None),
+ ('vendordata', None), ('vendordata_raw', None))
+
+ _dirty_cache = False
+
def __init__(self, sys_cfg, distro, paths, ud_proc=None):
self.sys_cfg = sys_cfg
self.distro = distro
@@ -134,11 +155,21 @@ class DataSource(object):
'region': self.region,
'availability-zone': self.availability_zone}}
+ def clear_cached_data(self):
+ """Reset any cached metadata attributes to datasource defaults."""
+ if self._dirty_cache:
+ for attribute, value in self.cached_attr_defaults:
+ if hasattr(self, attribute):
+ setattr(self, attribute, value)
+ self._dirty_cache = False
+
def get_data(self):
"""Datasources implement _get_data to setup metadata and userdata_raw.
Minimally, the datasource should return a boolean True on success.
"""
+ self.clear_cached_data()
+ self._dirty_cache = True
return_value = self._get_data()
json_file = os.path.join(self.paths.run_dir, INSTANCE_JSON_FILE)
if not return_value:
@@ -416,6 +447,30 @@ class DataSource(object):
def get_package_mirror_info(self):
return self.distro.get_package_mirror_info(data_source=self)
+ def update_metadata(self, source_event_types):
+ """Refresh cached metadata if the datasource supports this event.
+
+ The datasource has a list of maintenance_events which
+ trigger refreshing all cached metadata.
+
+ @param source_event_types: List of EventTypes which may trigger a
+ metadata update.
+
+ @return True if the datasource did successfully update cached metadata
+ due to source_event_type.
+ """
+ supported_events = [
+ evt for evt in source_event_types
+ if evt in self.maintenance_events]
+ if supported_events:
+ LOG.debug(
+ "Update datasource metadata due to maintenance events: '%s'",
+ ','.join(supported_events))
+ result = self.get_data()
+ if result:
+ return True
+ return False
+
def check_instance_id(self, sys_cfg):
# quickly (local check only) if self.instance_id is still
return False
@@ -520,7 +575,7 @@ def find_source(sys_cfg, distro, paths, ds_deps, cfg_list, pkg_list, reporter):
with myrep:
LOG.debug("Seeing if we can get any data from %s", cls)
s = cls(sys_cfg, distro, paths)
- if s.get_data():
+ if s.update_metadata([EventType.BOOT_NEW_INSTANCE]):
myrep.message = "found %s data from %s" % (mode, name)
return (s, type_utils.obj_name(cls))
except Exception:
diff --git a/cloudinit/sources/tests/test_init.py b/cloudinit/sources/tests/test_init.py
index d5bc98a..fe40110 100644
--- a/cloudinit/sources/tests/test_init.py
+++ b/cloudinit/sources/tests/test_init.py
@@ -5,6 +5,7 @@ import os
import six
import stat
+from cloudinit.event import EventType
from cloudinit.helpers import Paths
from cloudinit import importer
from cloudinit.sources import (
@@ -381,3 +382,69 @@ class TestDataSource(CiTestCase):
get_args(grandchild.get_hostname), # pylint: disable=W1505
'%s does not implement DataSource.get_hostname params'
% grandchild)
+
+ def test_clear_cached_data_resets_cached_attr_class_attributes(self):
+ """Class attributes listed in cached_attr_defaults are reset."""
+ count = 0
+ # Setup values for all cached class attributes
+ for attr, value in self.datasource.cached_attr_defaults:
+ setattr(self.datasource, attr, count)
+ count += 1
+ self.datasource._dirty_cache = True
+ self.datasource.clear_cached_data()
+ for attr, value in self.datasource.cached_attr_defaults:
+ self.assertEqual(value, getattr(self.datasource, attr))
+
+ def test_clear_cached_data_noops_on_clean_cache(self):
+ """Class attributes listed in cached_attr_defaults are reset."""
+ count = 0
+ # Setup values for all cached class attributes
+ for attr, _ in self.datasource.cached_attr_defaults:
+ setattr(self.datasource, attr, count)
+ count += 1
+ self.datasource._dirty_cache = False # Fake clean cache
+ self.datasource.clear_cached_data()
+ count = 0
+ for attr, value in self.datasource.cached_attr_defaults:
+ self.assertEqual(count, getattr(self.datasource, attr))
+ count += 1
+
+ def test_clear_cached_data_skips_non_attr_class_attributes(self):
+ """Skip any cached_attr_defaults which aren't class attributes."""
+ self.datasource._dirty_cache = True
+ self.datasource.clear_cached_data()
+ for attr in ('ec2_metadata', 'network_json'):
+ self.assertFalse(hasattr(self.datasource, attr))
+
+ def test_update_metadata_only_acts_on_supported_maintenance_events(self):
+ """update_metadata won't get_data on unsupported maintenance events."""
+ self.assertEqual(
+ [EventType.BOOT_NEW_INSTANCE],
+ self.datasource.maintenance_events)
+
+ def fake_get_data():
+ raise Exception('get_data should not be called')
+
+ self.datasource.get_data = fake_get_data
+ self.assertFalse(
+ self.datasource.update_metadata(
+ source_event_types=[EventType.BOOT]))
+
+ def test_update_metadata_returns_true_on_supported_maintenance_event(self):
+ """update_metadata returns get_data response on supported events."""
+
+ def fake_get_data():
+ return True
+
+ self.datasource.get_data = fake_get_data
+ self.assertTrue(
+ self.datasource.update_metadata(
+ source_event_types=[
+ EventType.BOOT, EventType.BOOT_NEW_INSTANCE]))
+ self.assertIn(
+ "DEBUG: Update datasource metadata due to maintenance events:"
+ " 'New instance first boot'",
+ self.logs.getvalue())
+
+
+# vi: ts=4 expandtab
diff --git a/cloudinit/stages.py b/cloudinit/stages.py
index 286607b..c132b57 100644
--- a/cloudinit/stages.py
+++ b/cloudinit/stages.py
@@ -22,6 +22,8 @@ from cloudinit.handlers import cloud_config as cc_part
from cloudinit.handlers import shell_script as ss_part
from cloudinit.handlers import upstart_job as up_part
+from cloudinit.event import EventType
+
from cloudinit import cloud
from cloudinit import config
from cloudinit import distros
@@ -648,10 +650,14 @@ class Init(object):
except Exception as e:
LOG.warning("Failed to rename devices: %s", e)
- if (self.datasource is not NULL_DATA_SOURCE and
- not self.is_new_instance()):
- LOG.debug("not a new instance. network config is not applied.")
- return
+ if self.datasource is not NULL_DATA_SOURCE:
+ if not self.is_new_instance():
+ if not self.datasource.update_metadata([EventType.BOOT]):
+ LOG.debug(
+ "No network config applied. Neither a new instance"
+ " nor datasource network update on '%s' event",
+ EventType.BOOT)
+ return
LOG.info("Applying network configuration from %s bringup=%s: %s",
src, bring_up, netcfg)
diff --git a/cloudinit/tests/test_stages.py b/cloudinit/tests/test_stages.py
new file mode 100644
index 0000000..2c0b9fc
--- /dev/null
+++ b/cloudinit/tests/test_stages.py
@@ -0,0 +1,231 @@
+# This file is part of cloud-init. See LICENSE file for license information.
+
+"""Tests related to cloudinit.stages module."""
+
+import os
+
+from cloudinit import stages
+from cloudinit import sources
+
+from cloudinit.event import EventType
+from cloudinit.util import write_file
+
+from cloudinit.tests.helpers import CiTestCase, mock
+
+TEST_INSTANCE_ID = 'i-testing'
+
+
+class FakeDataSource(sources.DataSource):
+
+ def __init__(self, paths=None, userdata=None, vendordata=None,
+ network_config=''):
+ super(FakeDataSource, self).__init__({}, None, paths=paths)
+ self.metadata = {'instance-id': TEST_INSTANCE_ID}
+ self.userdata_raw = userdata
+ self.vendordata_raw = vendordata
+ self._network_config = None
+ if network_config: # Permit for None value to setup attribute
+ self._network_config = network_config
+
+ @property
+ def network_config(self):
+ return self._network_config
+
+ def _get_data(self):
+ return True
+
+
+class TestInit(CiTestCase):
+ with_logs = True
+
+ def setUp(self):
+ super(TestInit, self).setUp()
+ self.tmpdir = self.tmp_dir()
+ self.init = stages.Init()
+ # Setup fake Paths for Init to reference
+ self.init._cfg = {'system_info': {
+ 'distro': 'ubuntu', 'paths': {'cloud_dir': self.tmpdir,
+ 'run_dir': self.tmpdir}}}
+ self.init.datasource = FakeDataSource(paths=self.init.paths)
+
+ def test_wb__find_networking_config_disabled(self):
+ """find_networking_config returns no config when disabled."""
+ disable_file = os.path.join(
+ self.init.paths.get_cpath('data'), 'upgraded-network')
+ write_file(disable_file, '')
+ self.assertEqual(
+ (None, disable_file),
+ self.init._find_networking_config())
+
+ @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config')
+ def test_wb__find_networking_config_disabled_by_kernel(self, m_cmdline):
+ """find_networking_config returns when disabled by kernel cmdline."""
+ m_cmdline.return_value = {'config': 'disabled'}
+ self.assertEqual(
+ (None, 'cmdline'),
+ self.init._find_networking_config())
+ self.assertEqual('DEBUG: network config disabled by cmdline\n',
+ self.logs.getvalue())
+
+ @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config')
+ def test_wb__find_networking_config_disabled_by_datasrc(self, m_cmdline):
+ """find_networking_config returns when disabled by datasource cfg."""
+ m_cmdline.return_value = {} # Kernel doesn't disable networking
+ self.init._cfg = {'system_info': {'paths': {'cloud_dir': self.tmpdir}},
+ 'network': {}} # system config doesn't disable
+
+ self.init.datasource = FakeDataSource(
+ network_config={'config': 'disabled'})
+ self.assertEqual(
+ (None, 'ds'),
+ self.init._find_networking_config())
+ self.assertEqual('DEBUG: network config disabled by ds\n',
+ self.logs.getvalue())
+
+ @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config')
+ def test_wb__find_networking_config_disabled_by_sysconfig(self, m_cmdline):
+ """find_networking_config returns when disabled by system config."""
+ m_cmdline.return_value = {} # Kernel doesn't disable networking
+ self.init._cfg = {'system_info': {'paths': {'cloud_dir': self.tmpdir}},
+ 'network': {'config': 'disabled'}}
+ self.assertEqual(
+ (None, 'system_cfg'),
+ self.init._find_networking_config())
+ self.assertEqual('DEBUG: network config disabled by system_cfg\n',
+ self.logs.getvalue())
+
+ @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config')
+ def test_wb__find_networking_config_returns_kernel(self, m_cmdline):
+ """find_networking_config returns kernel cmdline config if present."""
+ expected_cfg = {'config': ['fakekernel']}
+ m_cmdline.return_value = expected_cfg
+ self.init._cfg = {'system_info': {'paths': {'cloud_dir': self.tmpdir}},
+ 'network': {'config': ['fakesys_config']}}
+ self.init.datasource = FakeDataSource(
+ network_config={'config': ['fakedatasource']})
+ self.assertEqual(
+ (expected_cfg, 'cmdline'),
+ self.init._find_networking_config())
+
+ @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config')
+ def test_wb__find_networking_config_returns_system_cfg(self, m_cmdline):
+ """find_networking_config returns system config when present."""
+ m_cmdline.return_value = {} # No kernel network config
+ expected_cfg = {'config': ['fakesys_config']}
+ self.init._cfg = {'system_info': {'paths': {'cloud_dir': self.tmpdir}},
+ 'network': expected_cfg}
+ self.init.datasource = FakeDataSource(
+ network_config={'config': ['fakedatasource']})
+ self.assertEqual(
+ (expected_cfg, 'system_cfg'),
+ self.init._find_networking_config())
+
+ @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config')
+ def test_wb__find_networking_config_returns_datasrc_cfg(self, m_cmdline):
+ """find_networking_config returns datasource net config if present."""
+ m_cmdline.return_value = {} # No kernel network config
+ # No system config for network in setUp
+ expected_cfg = {'config': ['fakedatasource']}
+ self.init.datasource = FakeDataSource(network_config=expected_cfg)
+ self.assertEqual(
+ (expected_cfg, 'ds'),
+ self.init._find_networking_config())
+
+ @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config')
+ def test_wb__find_networking_config_returns_fallback(self, m_cmdline):
+ """find_networking_config returns fallback config if not defined."""
+ m_cmdline.return_value = {} # Kernel doesn't disable networking
+ # Neither datasource nor system_info disable or provide network
+
+ fake_cfg = {'config': [{'type': 'physical', 'name': 'eth9'}],
+ 'version': 1}
+
+ def fake_generate_fallback():
+ return fake_cfg
+
+ # Monkey patch distro which gets cached on self.init
+ distro = self.init.distro
+ distro.generate_fallback_config = fake_generate_fallback
+ self.assertEqual(
+ (fake_cfg, 'fallback'),
+ self.init._find_networking_config())
+ self.assertNotIn('network config disabled', self.logs.getvalue())
+
+ def test_apply_network_config_disabled(self):
+ """Log when network is disabled by upgraded-network."""
+ disable_file = os.path.join(
+ self.init.paths.get_cpath('data'), 'upgraded-network')
+
+ def fake_network_config():
+ return (None, disable_file)
+
+ self.init._find_networking_config = fake_network_config
+
+ self.init.apply_network_config(True)
+ self.assertIn(
+ 'INFO: network config is disabled by %s' % disable_file,
+ self.logs.getvalue())
+
+ @mock.patch('cloudinit.distros.ubuntu.Distro')
+ def test_apply_network_on_new_instance(self, m_ubuntu):
+ """Call distro apply_network_config methods on is_new_instance."""
+ net_cfg = {
+ 'version': 1, 'config': [
+ {'subnets': [{'type': 'dhcp'}], 'type': 'physical',
+ 'name': 'eth9', 'mac_address': '42:42:42:42:42:42'}]}
+
+ def fake_network_config():
+ return net_cfg, 'fallback'
+
+ self.init._find_networking_config = fake_network_config
+ 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(
+ net_cfg, bring_up=True)
+
+ @mock.patch('cloudinit.distros.ubuntu.Distro')
+ def test_apply_network_on_same_instance_id(self, m_ubuntu):
+ """Only call distro.apply_network_config_names on same instance id."""
+ old_instance_id = os.path.join(
+ self.init.paths.get_cpath('data'), 'instance-id')
+ write_file(old_instance_id, TEST_INSTANCE_ID)
+ net_cfg = {
+ 'version': 1, 'config': [
+ {'subnets': [{'type': 'dhcp'}], 'type': 'physical',
+ 'name': 'eth9', 'mac_address': '42:42:42:42:42:42'}]}
+
+ def fake_network_config():
+ return net_cfg, 'fallback'
+
+ self.init._find_networking_config = fake_network_config
+ 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_not_called()
+ self.assertIn(
+ 'No network config applied. Neither a new instance'
+ " nor datasource network update on '%s' event" % EventType.BOOT,
+ self.logs.getvalue())
+
+ @mock.patch('cloudinit.distros.ubuntu.Distro')
+ def test_apply_network_on_datasource_allowed_event(self, m_ubuntu):
+ """Apply network if datasource.update_metadata permits BOOT event."""
+ old_instance_id = os.path.join(
+ self.init.paths.get_cpath('data'), 'instance-id')
+ write_file(old_instance_id, TEST_INSTANCE_ID)
+ net_cfg = {
+ 'version': 1, 'config': [
+ {'subnets': [{'type': 'dhcp'}], 'type': 'physical',
+ 'name': 'eth9', 'mac_address': '42:42:42:42:42:42'}]}
+
+ def fake_network_config():
+ return net_cfg, 'fallback'
+
+ self.init._find_networking_config = fake_network_config
+ self.init.datasource = FakeDataSource(paths=self.init.paths)
+ self.init.datasource.maintenance_events = [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(
+ net_cfg, bring_up=True)
+
+# vi: ts=4 expandtab
Follow ups
-
Re: [Merge] ~chad.smith/cloud-init:feature/maintain-network-on-boot into cloud-init:master
From: Server Team CI bot, 2018-06-29
-
[Merge] ~chad.smith/cloud-init:feature/maintain-network-on-boot into cloud-init:master
From: Chad Smith, 2018-06-29
-
[Merge] ~chad.smith/cloud-init:feature/maintain-network-on-boot into cloud-init:master
From: Chad Smith, 2018-06-29
-
[Merge] ~chad.smith/cloud-init:feature/maintain-network-on-boot into cloud-init:master
From: Chad Smith, 2018-06-29
-
Re: [Merge] ~chad.smith/cloud-init:feature/maintain-network-on-boot into cloud-init:master
From: Server Team CI bot, 2018-06-29
-
Re: [Merge] ~chad.smith/cloud-init:feature/maintain-network-on-boot into cloud-init:master
From: Server Team CI bot, 2018-06-28
-
Re: [Merge] ~chad.smith/cloud-init:feature/maintain-network-on-boot into cloud-init:master
From: Chad Smith, 2018-06-28
-
[Merge] ~chad.smith/cloud-init:feature/maintain-network-on-boot into cloud-init:master
From: Chad Smith, 2018-06-27
-
Re: [Merge] ~chad.smith/cloud-init:feature/maintain-network-on-boot into cloud-init:master
From: Server Team CI bot, 2018-06-26
-
Re: [Merge] ~chad.smith/cloud-init:feature/maintain-network-on-boot into cloud-init:master
From: Server Team CI bot, 2018-06-26
-
Re: [Merge] ~chad.smith/cloud-init:feature/maintain-network-on-boot into cloud-init:master
From: Server Team CI bot, 2018-06-26