From 5b329408e8c73f7ee4efb57fe1bf4eb68c3a22f5 Mon Sep 17 00:00:00 2001 From: James Falcon Date: Fri, 15 Apr 2022 08:08:54 -0500 Subject: [PATCH 1/9] Type hints to make life easier --- cloudinit/cloud.py | 14 +++++++++++++- cloudinit/cmd/main.py | 6 +++++- cloudinit/distros/__init__.py | 2 +- cloudinit/helpers.py | 2 +- cloudinit/sources/__init__.py | 6 ++++-- cloudinit/stages.py | 25 ++++++++++++------------- cloudinit/util.py | 2 +- 7 files changed, 37 insertions(+), 20 deletions(-) diff --git a/cloudinit/cloud.py b/cloudinit/cloud.py index 91e48103e94..cbc5d0dbd76 100644 --- a/cloudinit/cloud.py +++ b/cloudinit/cloud.py @@ -6,9 +6,13 @@ import copy import os +from typing import Optional from cloudinit import log as logging +from cloudinit.distros import Distro +from cloudinit.helpers import Paths, Runners from cloudinit.reporting import events +from cloudinit.sources import DataSource LOG = logging.getLogger(__name__) @@ -25,7 +29,15 @@ class Cloud(object): - def __init__(self, datasource, paths, cfg, distro, runners, reporter=None): + def __init__( + self, + datasource: DataSource, + paths: Paths, + cfg: dict, + distro: Distro, + runners: Runners, + reporter: Optional[events.ReportEventStack] = None, + ): self.datasource = datasource self.paths = paths self.distro = distro diff --git a/cloudinit/cmd/main.py b/cloudinit/cmd/main.py index c9be41b3496..7a70f50e87c 100644 --- a/cloudinit/cmd/main.py +++ b/cloudinit/cmd/main.py @@ -11,6 +11,9 @@ # # This file is part of cloud-init. See LICENSE file for license information. +# Skip isort on this file because of the patch that comes between imports +# isort: skip_file + import argparse import json import os @@ -19,6 +22,7 @@ import traceback from cloudinit import patcher +from cloudinit.config.modules import Modules patcher.patch_logging() @@ -105,7 +109,7 @@ def extract_fns(args): return fn_cfgs -def run_module_section(mods, action_name, section): +def run_module_section(mods: Modules, action_name, section): full_section_name = MOD_SECTION_TPL % (section) (which_ran, failures) = mods.run_section(full_section_name) total_attempted = len(which_ran) + len(failures) diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index f2d9de107ed..4ff34381411 100755 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -1064,7 +1064,7 @@ def _get_arch_package_mirror_info(package_mirrors, arch): return default -def fetch(name): +def fetch(name) -> Type[Distro]: locs, looked_locs = importer.find_module(name, ["", __name__], ["Distro"]) if not locs: raise ImportError( diff --git a/cloudinit/helpers.py b/cloudinit/helpers.py index c2c9e5842bc..442bb11a9d3 100644 --- a/cloudinit/helpers.py +++ b/cloudinit/helpers.py @@ -260,7 +260,7 @@ def _get_instance_configs(self): ) return i_cfgs - def _read_cfg(self): + def _read_cfg(self) -> dict: # Input config files override # env config files which # override instance configs diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py index 88028cfa6e9..c5d51af7698 100644 --- a/cloudinit/sources/__init__.py +++ b/cloudinit/sources/__init__.py @@ -13,7 +13,7 @@ import json import os from collections import namedtuple -from typing import Dict, List # noqa: F401 +from typing import Dict, List, Tuple # noqa: F401 from cloudinit import dmi, importer from cloudinit import log as logging @@ -892,7 +892,9 @@ def normalize_pubkey_data(pubkey_data): return keys -def find_source(sys_cfg, distro, paths, ds_deps, cfg_list, pkg_list, reporter): +def find_source( + sys_cfg, distro, paths, ds_deps, cfg_list, pkg_list, reporter +) -> Tuple[DataSource, str]: ds_list = list_sources(cfg_list, ds_deps, pkg_list) ds_names = [type_utils.obj_name(f) for f in ds_list] mode = "network" if DEP_NETWORK in ds_deps else "local" diff --git a/cloudinit/stages.py b/cloudinit/stages.py index e3f8b3ef781..2d55daa47c8 100644 --- a/cloudinit/stages.py +++ b/cloudinit/stages.py @@ -9,7 +9,7 @@ import pickle import sys from collections import namedtuple -from typing import Dict, Set # noqa: F401 +from typing import Dict, List, Optional, Set # noqa: F401 from cloudinit import cloud, config, distros, handlers, helpers, importer from cloudinit import log as logging @@ -39,7 +39,6 @@ LOG = logging.getLogger(__name__) -NULL_DATA_SOURCE = None NO_PREVIOUS_INSTANCE_ID = "NO_PREVIOUS_INSTANCE_ID" @@ -98,17 +97,17 @@ def update_event_enabled( class Init(object): - def __init__(self, ds_deps=None, reporter=None): + def __init__(self, ds_deps: Optional[List[str]] = None, reporter=None): if ds_deps is not None: self.ds_deps = ds_deps else: self.ds_deps = [sources.DEP_FILESYSTEM, sources.DEP_NETWORK] # Created on first use - self._cfg = None - self._paths = None - self._distro = None + self._cfg: Optional[dict] = None + self._paths: Optional[helpers.Paths] = None + self._distro: Optional[distros.Distro] = None # Changed only when a fetch occurs - self.datasource = NULL_DATA_SOURCE + self.datasource: Optional[sources.DataSource] = None self.ds_restored = False self._previous_iid = None @@ -126,7 +125,7 @@ def _reset(self, reset_ds=False): self._paths = None self._distro = None if reset_ds: - self.datasource = NULL_DATA_SOURCE + self.datasource = None self.ds_restored = False @property @@ -141,7 +140,7 @@ def distro(self): # If we have an active datasource we need to adjust # said datasource and move its distro/system config # from whatever it was to a new set... - if self.datasource is not NULL_DATA_SOURCE: + if self.datasource is not None: self.datasource.distro = self._distro self.datasource.sys_cfg = self.cfg return self._distro @@ -252,7 +251,7 @@ def _restore_from_cache(self): return _pkl_load(self.paths.get_ipath_cur("obj_pkl")) def _write_to_cache(self): - if self.datasource is NULL_DATA_SOURCE: + if self.datasource is None: return False if util.get_cfg_option_bool(self.cfg, "manual_cache_clean", False): # The empty file in instance/ dir indicates manual cleaning, @@ -301,7 +300,7 @@ def _restore_from_checked_cache(self, existing): return (None, "cache invalid in datasource: %s" % ds) def _get_data_source(self, existing) -> sources.DataSource: - if self.datasource is not NULL_DATA_SOURCE: + if self.datasource is not None: return self.datasource with events.ReportEventStack( @@ -330,7 +329,7 @@ def _get_data_source(self, existing) -> sources.DataSource: self.reporter, ) LOG.info("Loaded datasource %s - %s", dsname, ds) - self.datasource = ds # type: sources.DataSource + self.datasource = ds # Ensure we adjust our path members datasource # now that we have one (thus allowing ipath to be used) self._reset() @@ -892,7 +891,7 @@ def should_run_on_boot_event(): ) if ( - self.datasource is not NULL_DATA_SOURCE + self.datasource is not None and not self.is_new_instance() and not should_run_on_boot_event() and not event_enabled_and_metadata_updated(EventType.BOOT_LEGACY) diff --git a/cloudinit/util.py b/cloudinit/util.py index 0e8f2f58e5d..444f17d6325 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -821,7 +821,7 @@ def make_url( return parse.urlunparse(pieces) -def mergemanydict(srcs, reverse=False): +def mergemanydict(srcs, reverse=False) -> dict: if reverse: srcs = reversed(srcs) merged_cfg = {} From bd2b4a6211d30d14c417a45d6f244c941e74e826 Mon Sep 17 00:00:00 2001 From: James Falcon Date: Fri, 15 Apr 2022 08:20:18 -0500 Subject: [PATCH 2/9] Move module operations into its own file --- cloudinit/cmd/main.py | 6 +- cloudinit/config/__init__.py | 47 ------- cloudinit/config/modules.py | 260 +++++++++++++++++++++++++++++++++++ cloudinit/stages.py | 214 +--------------------------- 4 files changed, 264 insertions(+), 263 deletions(-) create mode 100644 cloudinit/config/modules.py diff --git a/cloudinit/cmd/main.py b/cloudinit/cmd/main.py index 7a70f50e87c..f745df2befc 100644 --- a/cloudinit/cmd/main.py +++ b/cloudinit/cmd/main.py @@ -488,7 +488,7 @@ def main_init(name, args): apply_reporting_cfg(init.cfg) # Stage 8 - re-read and apply relevant cloud-config to include user-data - mods = stages.Modules(init, extract_fns(args), reporter=args.reporter) + mods = Modules(init, extract_fns(args), reporter=args.reporter) # Stage 9 try: outfmt_orig = outfmt @@ -591,7 +591,7 @@ def main_modules(action_name, args): return [(msg)] _maybe_persist_instance_data(init) # Stage 3 - mods = stages.Modules(init, extract_fns(args), reporter=args.reporter) + mods = Modules(init, extract_fns(args), reporter=args.reporter) # Stage 4 try: LOG.debug("Closing stdin") @@ -646,7 +646,7 @@ def main_single(name, args): return 1 _maybe_persist_instance_data(init) # Stage 3 - mods = stages.Modules(init, extract_fns(args), reporter=args.reporter) + mods = Modules(init, extract_fns(args), reporter=args.reporter) mod_args = args.module_args if mod_args: LOG.debug("Using passed in arguments %s", mod_args) diff --git a/cloudinit/config/__init__.py b/cloudinit/config/__init__.py index ed124180579..e69de29bb2d 100644 --- a/cloudinit/config/__init__.py +++ b/cloudinit/config/__init__.py @@ -1,47 +0,0 @@ -# Copyright (C) 2008-2010 Canonical Ltd. -# Copyright (C) 2012 Hewlett-Packard Development Company, L.P. -# -# Author: Chuck Short -# Author: Juerg Haefliger -# -# This file is part of cloud-init. See LICENSE file for license information. - -from cloudinit import log as logging -from cloudinit.settings import FREQUENCIES, PER_INSTANCE - -LOG = logging.getLogger(__name__) - -# This prefix is used to make it less -# of a chance that when importing -# we will not find something else with the same -# name in the lookup path... -MOD_PREFIX = "cc_" - - -def form_module_name(name): - canon_name = name.replace("-", "_") - if canon_name.lower().endswith(".py"): - canon_name = canon_name[0 : (len(canon_name) - 3)] - canon_name = canon_name.strip() - if not canon_name: - return None - if not canon_name.startswith(MOD_PREFIX): - canon_name = "%s%s" % (MOD_PREFIX, canon_name) - return canon_name - - -def fixup_module(mod, def_freq=PER_INSTANCE): - if not hasattr(mod, "frequency"): - setattr(mod, "frequency", def_freq) - else: - freq = mod.frequency - if freq and freq not in FREQUENCIES: - LOG.warning("Module %s has an unknown frequency %s", mod, freq) - if not hasattr(mod, "distros"): - setattr(mod, "distros", []) - if not hasattr(mod, "osfamilies"): - setattr(mod, "osfamilies", []) - return mod - - -# vi: ts=4 expandtab diff --git a/cloudinit/config/modules.py b/cloudinit/config/modules.py new file mode 100644 index 00000000000..9cf957da666 --- /dev/null +++ b/cloudinit/config/modules.py @@ -0,0 +1,260 @@ +# Copyright (C) 2008-2022 Canonical Ltd. +# Copyright (C) 2012 Hewlett-Packard Development Company, L.P. +# +# Author: Chuck Short +# Author: Juerg Haefliger +# +# This file is part of cloud-init. See LICENSE file for license information. + +import copy + +from cloudinit import config, distros, helpers, importer +from cloudinit import log as logging +from cloudinit import type_utils, util +from cloudinit.reporting import events +from cloudinit.settings import FREQUENCIES, PER_INSTANCE + +LOG = logging.getLogger(__name__) + +# This prefix is used to make it less +# of a chance that when importing +# we will not find something else with the same +# name in the lookup path... +MOD_PREFIX = "cc_" + + +def form_module_name(name): + canon_name = name.replace("-", "_") + if canon_name.lower().endswith(".py"): + canon_name = canon_name[0 : (len(canon_name) - 3)] + canon_name = canon_name.strip() + if not canon_name: + return None + if not canon_name.startswith(MOD_PREFIX): + canon_name = "%s%s" % (MOD_PREFIX, canon_name) + return canon_name + + +def fixup_module(mod, def_freq=PER_INSTANCE): + if not hasattr(mod, "frequency"): + setattr(mod, "frequency", def_freq) + else: + freq = mod.frequency + if freq and freq not in FREQUENCIES: + LOG.warning("Module %s has an unknown frequency %s", mod, freq) + if not hasattr(mod, "distros"): + setattr(mod, "distros", []) + if not hasattr(mod, "osfamilies"): + setattr(mod, "osfamilies", []) + return mod + + +class Modules(object): + def __init__(self, init, cfg_files=None, reporter=None): + self.init = init + self.cfg_files = cfg_files + # Created on first use + self._cached_cfg = None + if reporter is None: + reporter = events.ReportEventStack( + name="module-reporter", + description="module-desc", + reporting_enabled=False, + ) + self.reporter = reporter + + @property + def cfg(self): + # None check to avoid empty case causing re-reading + if self._cached_cfg is None: + merger = helpers.ConfigMerger( + paths=self.init.paths, + datasource=self.init.datasource, + additional_fns=self.cfg_files, + base_cfg=self.init.cfg, + ) + self._cached_cfg = merger.cfg + # LOG.debug("Loading 'module' config %s", self._cached_cfg) + # Only give out a copy so that others can't modify this... + return copy.deepcopy(self._cached_cfg) + + def _read_modules(self, name): + module_list = [] + if name not in self.cfg: + return module_list + cfg_mods = self.cfg.get(name) + if not cfg_mods: + return module_list + # Create 'module_list', an array of hashes + # Where hash['mod'] = module name + # hash['freq'] = frequency + # hash['args'] = arguments + for item in cfg_mods: + if not item: + continue + if isinstance(item, str): + module_list.append( + { + "mod": item.strip(), + } + ) + elif isinstance(item, (list)): + contents = {} + # Meant to fall through... + if len(item) >= 1: + contents["mod"] = item[0].strip() + if len(item) >= 2: + contents["freq"] = item[1].strip() + if len(item) >= 3: + contents["args"] = item[2:] + if contents: + module_list.append(contents) + elif isinstance(item, (dict)): + contents = {} + valid = False + if "name" in item: + contents["mod"] = item["name"].strip() + valid = True + if "frequency" in item: + contents["freq"] = item["frequency"].strip() + if "args" in item: + contents["args"] = item["args"] or [] + if contents and valid: + module_list.append(contents) + else: + raise TypeError( + "Failed to read '%s' item in config, unknown type %s" + % (item, type_utils.obj_name(item)) + ) + return module_list + + def _fixup_modules(self, raw_mods): + mostly_mods = [] + for raw_mod in raw_mods: + raw_name = raw_mod["mod"] + freq = raw_mod.get("freq") + run_args = raw_mod.get("args") or [] + mod_name = form_module_name(raw_name) + if not mod_name: + continue + if freq and freq not in FREQUENCIES: + LOG.warning( + "Config specified module %s has an unknown frequency %s", + raw_name, + freq, + ) + # Reset it so when ran it will get set to a known value + freq = None + mod_locs, looked_locs = importer.find_module( + mod_name, ["", type_utils.obj_name(config)], ["handle"] + ) + if not mod_locs: + LOG.warning( + "Could not find module named %s (searched %s)", + mod_name, + looked_locs, + ) + continue + mod = fixup_module(importer.import_module(mod_locs[0])) + mostly_mods.append([mod, raw_name, freq, run_args]) + return mostly_mods + + def _run_modules(self, mostly_mods): + cc = self.init.cloudify() + # Return which ones ran + # and which ones failed + the exception of why it failed + failures = [] + which_ran = [] + for (mod, name, freq, args) in mostly_mods: + try: + # Try the modules frequency, otherwise fallback to a known one + if not freq: + freq = mod.frequency + if freq not in FREQUENCIES: + freq = PER_INSTANCE + LOG.debug( + "Running module %s (%s) with frequency %s", name, mod, freq + ) + + # Use the configs logger and not our own + # TODO(harlowja): possibly check the module + # for having a LOG attr and just give it back + # its own logger? + func_args = [name, self.cfg, cc, LOG, args] + # Mark it as having started running + which_ran.append(name) + # This name will affect the semaphore name created + run_name = "config-%s" % (name) + + desc = "running %s with frequency %s" % (run_name, freq) + myrep = events.ReportEventStack( + name=run_name, description=desc, parent=self.reporter + ) + + with myrep: + ran, _r = cc.run( + run_name, mod.handle, func_args, freq=freq + ) + if ran: + myrep.message = "%s ran successfully" % run_name + else: + myrep.message = "%s previously ran" % run_name + + except Exception as e: + util.logexc(LOG, "Running module %s (%s) failed", name, mod) + failures.append((name, e)) + return (which_ran, failures) + + def run_single(self, mod_name, args=None, freq=None): + # Form the users module 'specs' + mod_to_be = { + "mod": mod_name, + "args": args, + "freq": freq, + } + # Now resume doing the normal fixups and running + raw_mods = [mod_to_be] + mostly_mods = self._fixup_modules(raw_mods) + return self._run_modules(mostly_mods) + + def run_section(self, section_name): + raw_mods = self._read_modules(section_name) + mostly_mods = self._fixup_modules(raw_mods) + d_name = self.init.distro.name + + skipped = [] + forced = [] + overridden = self.cfg.get("unverified_modules", []) + active_mods = [] + all_distros = set([distros.ALL_DISTROS]) + for (mod, name, _freq, _args) in mostly_mods: + worked_distros = set(mod.distros) # Minimally [] per fixup_modules + worked_distros.update( + distros.Distro.expand_osfamily(mod.osfamilies) + ) + + # Skip only when the following conditions are all met: + # - distros are defined in the module != ALL_DISTROS + # - the current d_name isn't in distros + # - and the module is unverified and not in the unverified_modules + # override list + if worked_distros and worked_distros != all_distros: + if d_name not in worked_distros: + if name not in overridden: + skipped.append(name) + continue + forced.append(name) + active_mods.append([mod, name, _freq, _args]) + + if skipped: + LOG.info( + "Skipping modules '%s' because they are not verified " + "on distro '%s'. To run anyway, add them to " + "'unverified_modules' in config.", + ",".join(skipped), + d_name, + ) + if forced: + LOG.info("running unverified_modules: '%s'", ", ".join(forced)) + + return self._run_modules(active_mods) diff --git a/cloudinit/stages.py b/cloudinit/stages.py index 2d55daa47c8..d02c2b4f0de 100644 --- a/cloudinit/stages.py +++ b/cloudinit/stages.py @@ -11,7 +11,7 @@ from collections import namedtuple from typing import Dict, List, Optional, Set # noqa: F401 -from cloudinit import cloud, config, distros, handlers, helpers, importer +from cloudinit import cloud, distros, handlers, helpers, importer from cloudinit import log as logging from cloudinit import net, sources, type_utils, util from cloudinit.event import EventScope, EventType, userdata_to_events @@ -29,7 +29,6 @@ from cloudinit.reporting import events from cloudinit.settings import ( CLOUD_CONFIG, - FREQUENCIES, PER_ALWAYS, PER_INSTANCE, PER_ONCE, @@ -943,217 +942,6 @@ def should_run_on_boot_event(): return -class Modules(object): - def __init__(self, init, cfg_files=None, reporter=None): - self.init = init - self.cfg_files = cfg_files - # Created on first use - self._cached_cfg = None - if reporter is None: - reporter = events.ReportEventStack( - name="module-reporter", - description="module-desc", - reporting_enabled=False, - ) - self.reporter = reporter - - @property - def cfg(self): - # None check to avoid empty case causing re-reading - if self._cached_cfg is None: - merger = helpers.ConfigMerger( - paths=self.init.paths, - datasource=self.init.datasource, - additional_fns=self.cfg_files, - base_cfg=self.init.cfg, - ) - self._cached_cfg = merger.cfg - # LOG.debug("Loading 'module' config %s", self._cached_cfg) - # Only give out a copy so that others can't modify this... - return copy.deepcopy(self._cached_cfg) - - def _read_modules(self, name): - module_list = [] - if name not in self.cfg: - return module_list - cfg_mods = self.cfg.get(name) - if not cfg_mods: - return module_list - # Create 'module_list', an array of hashes - # Where hash['mod'] = module name - # hash['freq'] = frequency - # hash['args'] = arguments - for item in cfg_mods: - if not item: - continue - if isinstance(item, str): - module_list.append( - { - "mod": item.strip(), - } - ) - elif isinstance(item, (list)): - contents = {} - # Meant to fall through... - if len(item) >= 1: - contents["mod"] = item[0].strip() - if len(item) >= 2: - contents["freq"] = item[1].strip() - if len(item) >= 3: - contents["args"] = item[2:] - if contents: - module_list.append(contents) - elif isinstance(item, (dict)): - contents = {} - valid = False - if "name" in item: - contents["mod"] = item["name"].strip() - valid = True - if "frequency" in item: - contents["freq"] = item["frequency"].strip() - if "args" in item: - contents["args"] = item["args"] or [] - if contents and valid: - module_list.append(contents) - else: - raise TypeError( - "Failed to read '%s' item in config, unknown type %s" - % (item, type_utils.obj_name(item)) - ) - return module_list - - def _fixup_modules(self, raw_mods): - mostly_mods = [] - for raw_mod in raw_mods: - raw_name = raw_mod["mod"] - freq = raw_mod.get("freq") - run_args = raw_mod.get("args") or [] - mod_name = config.form_module_name(raw_name) - if not mod_name: - continue - if freq and freq not in FREQUENCIES: - LOG.warning( - "Config specified module %s has an unknown frequency %s", - raw_name, - freq, - ) - # Reset it so when ran it will get set to a known value - freq = None - mod_locs, looked_locs = importer.find_module( - mod_name, ["", type_utils.obj_name(config)], ["handle"] - ) - if not mod_locs: - LOG.warning( - "Could not find module named %s (searched %s)", - mod_name, - looked_locs, - ) - continue - mod = config.fixup_module(importer.import_module(mod_locs[0])) - mostly_mods.append([mod, raw_name, freq, run_args]) - return mostly_mods - - def _run_modules(self, mostly_mods): - cc = self.init.cloudify() - # Return which ones ran - # and which ones failed + the exception of why it failed - failures = [] - which_ran = [] - for (mod, name, freq, args) in mostly_mods: - try: - # Try the modules frequency, otherwise fallback to a known one - if not freq: - freq = mod.frequency - if freq not in FREQUENCIES: - freq = PER_INSTANCE - LOG.debug( - "Running module %s (%s) with frequency %s", name, mod, freq - ) - - # Use the configs logger and not our own - # TODO(harlowja): possibly check the module - # for having a LOG attr and just give it back - # its own logger? - func_args = [name, self.cfg, cc, config.LOG, args] - # Mark it as having started running - which_ran.append(name) - # This name will affect the semaphore name created - run_name = "config-%s" % (name) - - desc = "running %s with frequency %s" % (run_name, freq) - myrep = events.ReportEventStack( - name=run_name, description=desc, parent=self.reporter - ) - - with myrep: - ran, _r = cc.run( - run_name, mod.handle, func_args, freq=freq - ) - if ran: - myrep.message = "%s ran successfully" % run_name - else: - myrep.message = "%s previously ran" % run_name - - except Exception as e: - util.logexc(LOG, "Running module %s (%s) failed", name, mod) - failures.append((name, e)) - return (which_ran, failures) - - def run_single(self, mod_name, args=None, freq=None): - # Form the users module 'specs' - mod_to_be = { - "mod": mod_name, - "args": args, - "freq": freq, - } - # Now resume doing the normal fixups and running - raw_mods = [mod_to_be] - mostly_mods = self._fixup_modules(raw_mods) - return self._run_modules(mostly_mods) - - def run_section(self, section_name): - raw_mods = self._read_modules(section_name) - mostly_mods = self._fixup_modules(raw_mods) - d_name = self.init.distro.name - - skipped = [] - forced = [] - overridden = self.cfg.get("unverified_modules", []) - active_mods = [] - all_distros = set([distros.ALL_DISTROS]) - for (mod, name, _freq, _args) in mostly_mods: - worked_distros = set(mod.distros) # Minimally [] per fixup_modules - worked_distros.update( - distros.Distro.expand_osfamily(mod.osfamilies) - ) - - # Skip only when the following conditions are all met: - # - distros are defined in the module != ALL_DISTROS - # - the current d_name isn't in distros - # - and the module is unverified and not in the unverified_modules - # override list - if worked_distros and worked_distros != all_distros: - if d_name not in worked_distros: - if name not in overridden: - skipped.append(name) - continue - forced.append(name) - active_mods.append([mod, name, _freq, _args]) - - if skipped: - LOG.info( - "Skipping modules '%s' because they are not verified " - "on distro '%s'. To run anyway, add them to " - "'unverified_modules' in config.", - ",".join(skipped), - d_name, - ) - if forced: - LOG.info("running unverified_modules: '%s'", ", ".join(forced)) - - return self._run_modules(active_mods) - - def read_runtime_config(): return util.read_conf(RUN_CLOUD_CONFIG) From 82e9a58b252fb6cdf5428984515468773ac21698 Mon Sep 17 00:00:00 2001 From: James Falcon Date: Fri, 15 Apr 2022 08:35:40 -0500 Subject: [PATCH 3/9] Update modules.py to validate on a module's meta dict and update remaining modules accordingly --- .../config/cc_refresh_rmc_and_interface.py | 20 +-- cloudinit/config/cc_reset_rmc.py | 20 +-- cloudinit/config/cc_write_files_deferred.py | 52 ++++---- cloudinit/config/modules.py | 117 ++++++++++++------ 4 files changed, 131 insertions(+), 78 deletions(-) diff --git a/cloudinit/config/cc_refresh_rmc_and_interface.py b/cloudinit/config/cc_refresh_rmc_and_interface.py index 87be534890e..6435278982e 100644 --- a/cloudinit/config/cc_refresh_rmc_and_interface.py +++ b/cloudinit/config/cc_refresh_rmc_and_interface.py @@ -25,23 +25,29 @@ - Refreshing RMC - Disabling NetworkManager from handling IPv6 interface, as IPv6 interface is used for communication between RMC daemon and PowerVM hypervisor. - -**Internal name:** ``cc_refresh_rmc_and_interface`` - -**Module frequency:** always - -**Supported distros:** RHEL - """ import errno from cloudinit import log as logging from cloudinit import netinfo, subp, util +from cloudinit.config.schema import MetaSchema +from cloudinit.distros import ALL_DISTROS from cloudinit.settings import PER_ALWAYS frequency = PER_ALWAYS +# This module is undocumented +meta: MetaSchema = { + "id": "cc_refresh_rmc_and_interface", + "name": "Refresh IPv6 Interface and RMC", + "title": "Ensure Network Manager is not managing IPv6 interface", + "description": __doc__, + "distros": [ALL_DISTROS], + "frequency": PER_ALWAYS, + "examples": [], +} + LOG = logging.getLogger(__name__) # Ensure that /opt/rsct/bin has been added to standard PATH of the # distro. The symlink to rmcctrl is /usr/sbin/rsct/bin/rmcctrl . diff --git a/cloudinit/config/cc_reset_rmc.py b/cloudinit/config/cc_reset_rmc.py index 3b9299031d9..def6d24d68b 100644 --- a/cloudinit/config/cc_reset_rmc.py +++ b/cloudinit/config/cc_reset_rmc.py @@ -28,22 +28,28 @@ In order to do so, it restarts RSCT service. Prerequisite of using this module is to install RSCT packages. - -**Internal name:** ``cc_reset_rmc`` - -**Module frequency:** per instance - -**Supported distros:** rhel, sles and ubuntu - """ import os from cloudinit import log as logging from cloudinit import subp, util +from cloudinit.config.schema import MetaSchema +from cloudinit.distros import ALL_DISTROS from cloudinit.settings import PER_INSTANCE frequency = PER_INSTANCE +# This module is undocumented +meta: MetaSchema = { + "id": "cc_reset_rmc", + "name": "Reset RMC", + "title": "reset rsct node id", + "description": __doc__, + "distros": [ALL_DISTROS], + "frequency": PER_INSTANCE, + "examples": [], +} + # RMCCTRL is expected to be in system PATH (/opt/rsct/bin) # The symlink for RMCCTRL and RECFGCT are # /usr/sbin/rsct/bin/rmcctrl and diff --git a/cloudinit/config/cc_write_files_deferred.py b/cloudinit/config/cc_write_files_deferred.py index c189950bfe8..8705df144ac 100644 --- a/cloudinit/config/cc_write_files_deferred.py +++ b/cloudinit/config/cc_write_files_deferred.py @@ -2,31 +2,36 @@ # # This file is part of cloud-init. See LICENSE file for license information. -"""Defer writing certain files""" +""" +Write Files Deferred +------------ +**Summary:** Defer writing certain files -from cloudinit import util -from cloudinit.config.cc_write_files import DEFAULT_DEFER, write_files +This module is based on `'Write Files' `__, and +will handle all files from the write_files list, that have been +marked as deferred and thus are not being processed by the +write-files module. -# meta is not used in this module, but it remains as code documentation -# -# id: cc_write_files_deferred' -# name: 'Write Deferred Files -# distros: ['all'], -# frequency: PER_INSTANCE, -# title: -# write certain files, whose creation as been deferred, during -# final stage -# description: -# This module is based on `'Write Files' `__, and -# will handle all files from the write_files list, that have been -# marked as deferred and thus are not being processed by the -# write-files module. -# -# *Please note that his module is not exposed to the user through -# its own dedicated top-level directive.* +*Please note that his module is not exposed to the user through +its own dedicated top-level directive.* +""" -# Not exposed, because related modules should document this behaviour -__doc__ = None +from cloudinit import util +from cloudinit.config.cc_write_files import DEFAULT_DEFER, write_files +from cloudinit.config.schema import MetaSchema +from cloudinit.distros import ALL_DISTROS +from cloudinit.settings import PER_INSTANCE + +# This module isn't documented +meta: MetaSchema = { + "id": "cc_write_files_deferred", + "name": "Write Files Deferred", + "title": "Defer writing certain files", + "description": __doc__, + "distros": [ALL_DISTROS], + "frequency": PER_INSTANCE, + "examples": [], +} def handle(name, cfg, _cloud, log, _args): @@ -44,6 +49,3 @@ def handle(name, cfg, _cloud, log, _args): ) return write_files(name, filtered_files) - - -# vi: ts=4 expandtab diff --git a/cloudinit/config/modules.py b/cloudinit/config/modules.py index 9cf957da666..8b40f4cfcef 100644 --- a/cloudinit/config/modules.py +++ b/cloudinit/config/modules.py @@ -8,11 +8,14 @@ import copy -from cloudinit import config, distros, helpers, importer +from cloudinit import config, importer from cloudinit import log as logging from cloudinit import type_utils, util -from cloudinit.reporting import events -from cloudinit.settings import FREQUENCIES, PER_INSTANCE +from cloudinit.distros import ALL_DISTROS +from cloudinit.helpers import ConfigMerger +from cloudinit.reporting.events import ReportEventStack +from cloudinit.settings import FREQUENCIES +from cloudinit.stages import Init LOG = logging.getLogger(__name__) @@ -35,28 +38,31 @@ def form_module_name(name): return canon_name -def fixup_module(mod, def_freq=PER_INSTANCE): - if not hasattr(mod, "frequency"): - setattr(mod, "frequency", def_freq) - else: - freq = mod.frequency - if freq and freq not in FREQUENCIES: - LOG.warning("Module %s has an unknown frequency %s", mod, freq) - if not hasattr(mod, "distros"): - setattr(mod, "distros", []) - if not hasattr(mod, "osfamilies"): - setattr(mod, "osfamilies", []) - return mod +def validate_module(mod, name): + if ( + not hasattr(mod, "meta") + or "frequency" not in mod.meta + or "distros" not in mod.meta + ): + raise ValueError( + f"Module '{mod}' with name '{name}' MUST have a 'meta' attribute " + "of type 'MetaSchema'." + ) + if mod.meta["frequency"] not in FREQUENCIES: + raise ValueError( + f"Module '{mod}' with name '{name}' has an invalid frequency " + f"{mod.meta['frequency']}." + ) class Modules(object): - def __init__(self, init, cfg_files=None, reporter=None): + def __init__(self, init: Init, cfg_files=None, reporter=None): self.init = init self.cfg_files = cfg_files # Created on first use self._cached_cfg = None if reporter is None: - reporter = events.ReportEventStack( + reporter = ReportEventStack( name="module-reporter", description="module-desc", reporting_enabled=False, @@ -67,7 +73,7 @@ def __init__(self, init, cfg_files=None, reporter=None): def cfg(self): # None check to avoid empty case causing re-reading if self._cached_cfg is None: - merger = helpers.ConfigMerger( + merger = ConfigMerger( paths=self.init.paths, datasource=self.init.datasource, additional_fns=self.cfg_files, @@ -79,16 +85,25 @@ def cfg(self): return copy.deepcopy(self._cached_cfg) def _read_modules(self, name): + """Read the modules from the config file given the specified name. + + Returns a list of module definitions. E.g., + [ + { + "mod": "bootcmd", + "freq": "always" + "args": "some_arg", + } + ] + + Note that in the default case, only "mod" will be set. + """ module_list = [] if name not in self.cfg: return module_list cfg_mods = self.cfg.get(name) if not cfg_mods: return module_list - # Create 'module_list', an array of hashes - # Where hash['mod'] = module name - # hash['freq'] = frequency - # hash['args'] = arguments for item in cfg_mods: if not item: continue @@ -129,6 +144,22 @@ def _read_modules(self, name): return module_list def _fixup_modules(self, raw_mods): + """Convert list of returned from _read_modules() into new format. + + Returns a list of of lists having a module reference, module name, + frequency, and run args. E.g.: + [ + [ + , + "bootcmd", + "always", + "some_arg", + ] + ] + + Invalid modules and arguments are ingnored. + Also ensures that the module has the required meta fields. + """ mostly_mods = [] for raw_mod in raw_mods: raw_name = raw_mod["mod"] @@ -155,7 +186,8 @@ def _fixup_modules(self, raw_mods): looked_locs, ) continue - mod = fixup_module(importer.import_module(mod_locs[0])) + mod = importer.import_module(mod_locs[0]) + validate_module(mod, raw_name) mostly_mods.append([mod, raw_name, freq, run_args]) return mostly_mods @@ -167,11 +199,6 @@ def _run_modules(self, mostly_mods): which_ran = [] for (mod, name, freq, args) in mostly_mods: try: - # Try the modules frequency, otherwise fallback to a known one - if not freq: - freq = mod.frequency - if freq not in FREQUENCIES: - freq = PER_INSTANCE LOG.debug( "Running module %s (%s) with frequency %s", name, mod, freq ) @@ -184,10 +211,10 @@ def _run_modules(self, mostly_mods): # Mark it as having started running which_ran.append(name) # This name will affect the semaphore name created - run_name = "config-%s" % (name) + run_name = f"config-%s" % (name) desc = "running %s with frequency %s" % (run_name, freq) - myrep = events.ReportEventStack( + myrep = ReportEventStack( name=run_name, description=desc, parent=self.reporter ) @@ -218,28 +245,40 @@ def run_single(self, mod_name, args=None, freq=None): return self._run_modules(mostly_mods) def run_section(self, section_name): + """Runs all modules in the given section. + + section_name - One of the modules lists as defined in + /etc/cloud/cloud.cfg. One of: + - cloud_init_modules + - cloud_config_modules + - cloud_final_modules + """ raw_mods = self._read_modules(section_name) mostly_mods = self._fixup_modules(raw_mods) - d_name = self.init.distro.name + distro_name = self.init.distro.name skipped = [] forced = [] overridden = self.cfg.get("unverified_modules", []) active_mods = [] - all_distros = set([distros.ALL_DISTROS]) for (mod, name, _freq, _args) in mostly_mods: - worked_distros = set(mod.distros) # Minimally [] per fixup_modules - worked_distros.update( - distros.Distro.expand_osfamily(mod.osfamilies) - ) + if mod is None: + continue + print("worked_distros") + print("worked_distros") + print("worked_distros") + print("worked_distros") + print("worked_distros") + print(mod) + worked_distros = mod.meta["distros"] # Skip only when the following conditions are all met: # - distros are defined in the module != ALL_DISTROS # - the current d_name isn't in distros # - and the module is unverified and not in the unverified_modules # override list - if worked_distros and worked_distros != all_distros: - if d_name not in worked_distros: + if worked_distros and worked_distros != [ALL_DISTROS]: + if distro_name not in worked_distros: if name not in overridden: skipped.append(name) continue @@ -252,7 +291,7 @@ def run_section(self, section_name): "on distro '%s'. To run anyway, add them to " "'unverified_modules' in config.", ",".join(skipped), - d_name, + distro_name, ) if forced: LOG.info("running unverified_modules: '%s'", ", ".join(forced)) From d087f2efa45cdb2b5de2a4163cb26b336200bc47 Mon Sep 17 00:00:00 2001 From: James Falcon Date: Fri, 15 Apr 2022 10:13:18 -0500 Subject: [PATCH 4/9] tests --- cloudinit/config/modules.py | 6 -- tests/unittests/config/test_schema.py | 78 ++++--------------------- tests/unittests/runs/test_merge_run.py | 3 +- tests/unittests/runs/test_simple_run.py | 11 ++-- tests/unittests/test_data.py | 11 ++-- 5 files changed, 24 insertions(+), 85 deletions(-) diff --git a/cloudinit/config/modules.py b/cloudinit/config/modules.py index 8b40f4cfcef..a869dbfc5f7 100644 --- a/cloudinit/config/modules.py +++ b/cloudinit/config/modules.py @@ -264,12 +264,6 @@ def run_section(self, section_name): for (mod, name, _freq, _args) in mostly_mods: if mod is None: continue - print("worked_distros") - print("worked_distros") - print("worked_distros") - print("worked_distros") - print("worked_distros") - print(mod) worked_distros = mod.meta["distros"] # Skip only when the following conditions are all met: diff --git a/tests/unittests/config/test_schema.py b/tests/unittests/config/test_schema.py index a367cda1c25..14f0be5c338 100644 --- a/tests/unittests/config/test_schema.py +++ b/tests/unittests/config/test_schema.py @@ -9,6 +9,7 @@ from copy import copy from pathlib import Path from textwrap import dedent +from typing import List import pytest import yaml @@ -54,17 +55,19 @@ def get_metas() -> dict: return get_module_variable("meta") -def get_module_variable(var_name) -> dict: - """Inspect modules and get variable from module matching var_name""" - schemas = {} - +def get_modules() -> List[str]: + """Return list of module names in cloudinit/config""" files = list( Path(cloud_init_project_dir("cloudinit/config/")).glob("cc_*.py") ) - modules = [mod.stem for mod in files] + return [mod.stem for mod in files] + - for module in modules: +def get_module_variable(var_name) -> dict: + """Inspect modules and get variable from module matching var_name""" + schemas = {} + for module in get_modules(): importlib.import_module("cloudinit.config.{}".format(module)) for k, v in sys.modules.items(): @@ -92,68 +95,7 @@ def test_static_schema_file_is_valid(self, caplog): def test_get_schema_coalesces_known_schema(self): """Every cloudconfig module with schema is listed in allOf keyword.""" schema = get_schema() - assert sorted( - [ - "cc_apk_configure", - "cc_apt_configure", - "cc_apt_pipelining", - "cc_bootcmd", - "cc_byobu", - "cc_ca_certs", - "cc_chef", - "cc_debug", - "cc_disable_ec2_metadata", - "cc_disk_setup", - "cc_fan", - "cc_foo", - "cc_final_message", - "cc_growpart", - "cc_grub_dpkg", - "cc_install_hotplug", - "cc_keyboard", - "cc_keys_to_console", - "cc_landscape", - "cc_locale", - "cc_lxd", - "cc_mcollective", - "cc_migrator", - "cc_mounts", - "cc_ntp", - "cc_package_update_upgrade_install", - "cc_phone_home", - "cc_power_state_change", - "cc_puppet", - "cc_resizefs", - "cc_resolv_conf", - "cc_rightscale_userdata", - "cc_rh_subscription", - "cc_rsyslog", - "cc_runcmd", - "cc_salt_minion", - "cc_scripts_per_boot", - "cc_scripts_per_instance", - "cc_scripts_per_once", - "cc_scripts_user", - "cc_scripts_vendor", - "cc_seed_random", - "cc_set_hostname", - "cc_set_passwords", - "cc_snap", - "cc_spacewalk", - "cc_ssh_authkey_fingerprints", - "cc_ssh_import_id", - "cc_ssh", - "cc_timezone", - "cc_ubuntu_advantage", - "cc_ubuntu_drivers", - "cc_update_etc_hosts", - "cc_update_hostname", - "cc_users_groups", - "cc_write_files", - "cc_yum_add_repo", - "cc_zypper_add_repo", - ] - ) == sorted( + assert sorted(get_modules()) == sorted( [meta["id"] for meta in get_metas().values() if meta is not None] ) assert "http://json-schema.org/draft-04/schema#" == schema["$schema"] diff --git a/tests/unittests/runs/test_merge_run.py b/tests/unittests/runs/test_merge_run.py index 1b1b55955f7..5f217a3d36c 100644 --- a/tests/unittests/runs/test_merge_run.py +++ b/tests/unittests/runs/test_merge_run.py @@ -5,6 +5,7 @@ import tempfile from cloudinit import safeyaml, stages, util +from cloudinit.config.modules import Modules from cloudinit.settings import PER_INSTANCE from tests.unittests import helpers @@ -49,7 +50,7 @@ def test_none_ds(self): self.assertEqual(1, len(mirrors)) mirror = mirrors[0] self.assertEqual(mirror["arches"], ["i386", "amd64", "blah"]) - mods = stages.Modules(initer) + mods = Modules(initer) (which_ran, failures) = mods.run_section("cloud_init_modules") self.assertTrue(len(failures) == 0) self.assertTrue(os.path.exists("/etc/blah.ini")) diff --git a/tests/unittests/runs/test_simple_run.py b/tests/unittests/runs/test_simple_run.py index 38cf949402c..2b51117cfd9 100644 --- a/tests/unittests/runs/test_simple_run.py +++ b/tests/unittests/runs/test_simple_run.py @@ -4,6 +4,7 @@ import os from cloudinit import safeyaml, stages, util +from cloudinit.config.modules import Modules from cloudinit.settings import PER_INSTANCE from tests.unittests import helpers @@ -71,7 +72,7 @@ def test_none_ds_runs_modules_which_do_not_define_distros(self): freq=PER_INSTANCE, ) - mods = stages.Modules(initer) + mods = Modules(initer) (which_ran, failures) = mods.run_section("cloud_init_modules") self.assertTrue(len(failures) == 0) self.assertTrue(os.path.exists("/etc/blah.ini")) @@ -99,7 +100,7 @@ def test_none_ds_skips_modules_which_define_unmatched_distros(self): freq=PER_INSTANCE, ) - mods = stages.Modules(initer) + mods = Modules(initer) (which_ran, failures) = mods.run_section("cloud_init_modules") self.assertTrue(len(failures) == 0) self.assertIn( @@ -128,7 +129,7 @@ def test_none_ds_runs_modules_which_distros_all(self): freq=PER_INSTANCE, ) - mods = stages.Modules(initer) + mods = Modules(initer) (which_ran, failures) = mods.run_section("cloud_init_modules") self.assertTrue(len(failures) == 0) self.assertIn("runcmd", which_ran) @@ -163,7 +164,7 @@ def test_none_ds_forces_run_via_unverified_modules(self): freq=PER_INSTANCE, ) - mods = stages.Modules(initer) + mods = Modules(initer) (which_ran, failures) = mods.run_section("cloud_init_modules") self.assertTrue(len(failures) == 0) self.assertIn("spacewalk", which_ran) @@ -197,7 +198,7 @@ def test_none_ds_run_with_no_config_modules(self): freq=PER_INSTANCE, ) - mods = stages.Modules(initer) + mods = Modules(initer) (which_ran, failures) = mods.run_section("cloud_init_modules") self.assertTrue(len(failures) == 0) self.assertEqual([], which_ran) diff --git a/tests/unittests/test_data.py b/tests/unittests/test_data.py index 010ac7416d3..75c304a8cdb 100644 --- a/tests/unittests/test_data.py +++ b/tests/unittests/test_data.py @@ -19,6 +19,7 @@ from cloudinit import log, safeyaml, sources, stages from cloudinit import user_data as ud from cloudinit import util +from cloudinit.config.modules import Modules from cloudinit.settings import PER_INSTANCE from tests.unittests import helpers @@ -141,7 +142,7 @@ def test_simple_jsonp_vendor_and_vendor2_and_user(self): args=[PER_INSTANCE], freq=PER_INSTANCE, ) - mods = stages.Modules(initer) + mods = Modules(initer) (_which_ran, _failures) = mods.run_section("cloud_init_modules") cfg = mods.cfg self.assertIn("vendor_data", cfg) @@ -187,7 +188,7 @@ def test_simple_jsonp_no_vendor_consumed(self): args=[PER_INSTANCE], freq=PER_INSTANCE, ) - mods = stages.Modules(initer) + mods = Modules(initer) (_which_ran, _failures) = mods.run_section("cloud_init_modules") cfg = mods.cfg self.assertEqual("qux", cfg["baz"]) @@ -296,7 +297,7 @@ def test_vendor_user_yaml_cloud_config(self): args=[PER_INSTANCE], freq=PER_INSTANCE, ) - mods = stages.Modules(initer) + mods = Modules(initer) (_which_ran, _failures) = mods.run_section("cloud_init_modules") cfg = mods.cfg self.assertIn("vendor_data", cfg) @@ -338,7 +339,7 @@ def test_vendordata_script(self): args=[PER_INSTANCE], freq=PER_INSTANCE, ) - mods = stages.Modules(initer) + mods = Modules(initer) (_which_ran, _failures) = mods.run_section("cloud_init_modules") vendor_script = initer.paths.get_ipath_cur("vendor_scripts") vendor_script_fns = "%s%s/part-001" % (new_root, vendor_script) @@ -634,7 +635,7 @@ def test_dont_allow_user_data(self, mock_cfg): args=[PER_INSTANCE], freq=PER_INSTANCE, ) - mods = stages.Modules(initer) + mods = Modules(initer) (_which_ran, _failures) = mods.run_section("cloud_init_modules") cfg = mods.cfg self.assertIn("vendor_data", cfg) From e7f1eb1bace8e34a2523222f245e9d3c56026409 Mon Sep 17 00:00:00 2001 From: James Falcon Date: Fri, 15 Apr 2022 15:52:30 -0500 Subject: [PATCH 5/9] Remove line that has been commented for years --- cloudinit/config/modules.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cloudinit/config/modules.py b/cloudinit/config/modules.py index a869dbfc5f7..9afa87acadc 100644 --- a/cloudinit/config/modules.py +++ b/cloudinit/config/modules.py @@ -80,7 +80,6 @@ def cfg(self): base_cfg=self.init.cfg, ) self._cached_cfg = merger.cfg - # LOG.debug("Loading 'module' config %s", self._cached_cfg) # Only give out a copy so that others can't modify this... return copy.deepcopy(self._cached_cfg) From 95fea7d796b7a29ee96240dec0aebe416b528eed Mon Sep 17 00:00:00 2001 From: James Falcon Date: Fri, 15 Apr 2022 15:55:44 -0500 Subject: [PATCH 6/9] forgot to finish an f-string --- cloudinit/config/modules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cloudinit/config/modules.py b/cloudinit/config/modules.py index 9afa87acadc..c58afe81dfb 100644 --- a/cloudinit/config/modules.py +++ b/cloudinit/config/modules.py @@ -210,7 +210,7 @@ def _run_modules(self, mostly_mods): # Mark it as having started running which_ran.append(name) # This name will affect the semaphore name created - run_name = f"config-%s" % (name) + run_name = f"config-{name}" desc = "running %s with frequency %s" % (run_name, freq) myrep = ReportEventStack( From 3fbe5e0eb1047c2101c9a08cfe223258299fcfe4 Mon Sep 17 00:00:00 2001 From: James Falcon Date: Fri, 15 Apr 2022 16:31:28 -0500 Subject: [PATCH 7/9] add test and fix module based on new test --- cloudinit/config/cc_spacewalk.py | 3 +-- tests/unittests/config/test_schema.py | 32 +++++++++++++++++++++------ 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/cloudinit/config/cc_spacewalk.py b/cloudinit/config/cc_spacewalk.py index d319efef010..6820a816a51 100644 --- a/cloudinit/config/cc_spacewalk.py +++ b/cloudinit/config/cc_spacewalk.py @@ -7,7 +7,6 @@ from cloudinit.config.schema import MetaSchema, get_meta_doc from cloudinit.settings import PER_INSTANCE -distros = ["redhat", "fedora"] MODULE_DESCRIPTION = """\ This module installs spacewalk and applies basic configuration. If the ``spacewalk`` config key is present spacewalk will be installed. The server to @@ -23,7 +22,7 @@ "name": "Spacewalk", "title": "Install and configure spacewalk", "description": MODULE_DESCRIPTION, - "distros": distros, + "distros": ["rhel", "fedora"], "frequency": PER_INSTANCE, "examples": [ dedent( diff --git a/tests/unittests/config/test_schema.py b/tests/unittests/config/test_schema.py index 14f0be5c338..74758581b79 100644 --- a/tests/unittests/config/test_schema.py +++ b/tests/unittests/config/test_schema.py @@ -9,6 +9,7 @@ from copy import copy from pathlib import Path from textwrap import dedent +from types import ModuleType from typing import List import pytest @@ -30,6 +31,8 @@ validate_cloudconfig_metaschema, validate_cloudconfig_schema, ) +from cloudinit.distros import OSFAMILIES +from cloudinit.settings import FREQUENCIES from cloudinit.util import write_file from tests.unittests.helpers import ( CiTestCase, @@ -55,7 +58,7 @@ def get_metas() -> dict: return get_module_variable("meta") -def get_modules() -> List[str]: +def get_module_names() -> List[str]: """Return list of module names in cloudinit/config""" files = list( Path(cloud_init_project_dir("cloudinit/config/")).glob("cc_*.py") @@ -64,15 +67,20 @@ def get_modules() -> List[str]: return [mod.stem for mod in files] +def get_modules() -> List[ModuleType]: + """Return list of modules in cloudinit/config""" + return [ + importlib.import_module(f"cloudinit.config.{module}") + for module in get_module_names() + ] + + def get_module_variable(var_name) -> dict: """Inspect modules and get variable from module matching var_name""" schemas = {} - for module in get_modules(): - importlib.import_module("cloudinit.config.{}".format(module)) - + get_modules() for k, v in sys.modules.items(): path = Path(k) - if "cloudinit.config" == path.stem and path.suffix[1:4] == "cc_": module_name = path.suffix[1:] members = inspect.getmembers(v) @@ -95,7 +103,7 @@ def test_static_schema_file_is_valid(self, caplog): def test_get_schema_coalesces_known_schema(self): """Every cloudconfig module with schema is listed in allOf keyword.""" schema = get_schema() - assert sorted(get_modules()) == sorted( + assert sorted(get_module_names()) == sorted( [meta["id"] for meta in get_metas().values() if meta is not None] ) assert "http://json-schema.org/draft-04/schema#" == schema["$schema"] @@ -1046,4 +1054,14 @@ def test_validate_bad_module(self): validate_cloudconfig_metaschema(validator, schema, throw=False) -# vi: ts=4 expandtab syntax=python +class TestMeta: + def test_valid_meta_for_every_module(self): + all_distros = { + name for distro in OSFAMILIES.values() for name in distro + } + all_distros.add("all") + for module in get_modules(): + assert "frequency" in module.meta + assert "distros" in module.meta + assert {module.meta["frequency"]}.issubset(FREQUENCIES) + assert set(module.meta["distros"]).issubset(all_distros) From efae7fecc825a993da172935368bf9eea34aea5a Mon Sep 17 00:00:00 2001 From: James Falcon Date: Mon, 18 Apr 2022 09:59:42 -0500 Subject: [PATCH 8/9] addressing comments --- .../config/cc_refresh_rmc_and_interface.py | 31 ++++++++--------- cloudinit/config/cc_reset_rmc.py | 28 +++++++-------- cloudinit/config/cc_write_files_deferred.py | 23 ++++++------- cloudinit/config/modules.py | 34 +++++++++++-------- 4 files changed, 59 insertions(+), 57 deletions(-) diff --git a/cloudinit/config/cc_refresh_rmc_and_interface.py b/cloudinit/config/cc_refresh_rmc_and_interface.py index 6435278982e..3ed5612b986 100644 --- a/cloudinit/config/cc_refresh_rmc_and_interface.py +++ b/cloudinit/config/cc_refresh_rmc_and_interface.py @@ -4,11 +4,18 @@ # # This file is part of cloud-init. See LICENSE file for license information. -""" -Refresh IPv6 interface and RMC ------------------------------- -**Summary:** Ensure Network Manager is not managing IPv6 interface +"""Refresh IPv6 interface and RMC: +Ensure Network Manager is not managing IPv6 interface""" + +import errno + +from cloudinit import log as logging +from cloudinit import netinfo, subp, util +from cloudinit.config.schema import MetaSchema +from cloudinit.distros import ALL_DISTROS +from cloudinit.settings import PER_ALWAYS +MODULE_DESCRIPTION = """\ This module is IBM PowerVM Hypervisor specific Reliable Scalable Cluster Technology (RSCT) is a set of software components @@ -27,27 +34,19 @@ is used for communication between RMC daemon and PowerVM hypervisor. """ -import errno - -from cloudinit import log as logging -from cloudinit import netinfo, subp, util -from cloudinit.config.schema import MetaSchema -from cloudinit.distros import ALL_DISTROS -from cloudinit.settings import PER_ALWAYS - -frequency = PER_ALWAYS - -# This module is undocumented meta: MetaSchema = { "id": "cc_refresh_rmc_and_interface", "name": "Refresh IPv6 Interface and RMC", "title": "Ensure Network Manager is not managing IPv6 interface", - "description": __doc__, + "description": MODULE_DESCRIPTION, "distros": [ALL_DISTROS], "frequency": PER_ALWAYS, "examples": [], } +# This module is undocumented in our schema docs +__doc__ = "" + LOG = logging.getLogger(__name__) # Ensure that /opt/rsct/bin has been added to standard PATH of the # distro. The symlink to rmcctrl is /usr/sbin/rsct/bin/rmcctrl . diff --git a/cloudinit/config/cc_reset_rmc.py b/cloudinit/config/cc_reset_rmc.py index def6d24d68b..57f024efb30 100644 --- a/cloudinit/config/cc_reset_rmc.py +++ b/cloudinit/config/cc_reset_rmc.py @@ -3,13 +3,18 @@ # Author: Aman Kumar Sinha # # This file is part of cloud-init. See LICENSE file for license information. +"""Reset RMC: Reset rsct node id""" -""" -Reset RMC ------------- -**Summary:** reset rsct node id +import os +from cloudinit import log as logging +from cloudinit import subp, util +from cloudinit.config.schema import MetaSchema +from cloudinit.distros import ALL_DISTROS +from cloudinit.settings import PER_INSTANCE + +MODULE_DESCRIPTION = """\ Reset RMC module is IBM PowerVM Hypervisor specific Reliable Scalable Cluster Technology (RSCT) is a set of software components, @@ -29,27 +34,20 @@ Prerequisite of using this module is to install RSCT packages. """ -import os -from cloudinit import log as logging -from cloudinit import subp, util -from cloudinit.config.schema import MetaSchema -from cloudinit.distros import ALL_DISTROS -from cloudinit.settings import PER_INSTANCE - -frequency = PER_INSTANCE - -# This module is undocumented meta: MetaSchema = { "id": "cc_reset_rmc", "name": "Reset RMC", "title": "reset rsct node id", - "description": __doc__, + "description": MODULE_DESCRIPTION, "distros": [ALL_DISTROS], "frequency": PER_INSTANCE, "examples": [], } +# This module is undocumented in our schema docs +__doc__ = "" + # RMCCTRL is expected to be in system PATH (/opt/rsct/bin) # The symlink for RMCCTRL and RECFGCT are # /usr/sbin/rsct/bin/rmcctrl and diff --git a/cloudinit/config/cc_write_files_deferred.py b/cloudinit/config/cc_write_files_deferred.py index 8705df144ac..dbbe90f6da9 100644 --- a/cloudinit/config/cc_write_files_deferred.py +++ b/cloudinit/config/cc_write_files_deferred.py @@ -2,11 +2,15 @@ # # This file is part of cloud-init. See LICENSE file for license information. -""" -Write Files Deferred ------------- -**Summary:** Defer writing certain files +"""Write Files Deferred: Defer writing certain files""" + +from cloudinit import util +from cloudinit.config.cc_write_files import DEFAULT_DEFER, write_files +from cloudinit.config.schema import MetaSchema +from cloudinit.distros import ALL_DISTROS +from cloudinit.settings import PER_INSTANCE +MODULE_DESCRIPTION = """\ This module is based on `'Write Files' `__, and will handle all files from the write_files list, that have been marked as deferred and thus are not being processed by the @@ -15,14 +19,6 @@ *Please note that his module is not exposed to the user through its own dedicated top-level directive.* """ - -from cloudinit import util -from cloudinit.config.cc_write_files import DEFAULT_DEFER, write_files -from cloudinit.config.schema import MetaSchema -from cloudinit.distros import ALL_DISTROS -from cloudinit.settings import PER_INSTANCE - -# This module isn't documented meta: MetaSchema = { "id": "cc_write_files_deferred", "name": "Write Files Deferred", @@ -33,6 +29,9 @@ "examples": [], } +# This module is undocumented in our schema docs +__doc__ = "" + def handle(name, cfg, _cloud, log, _args): file_list = cfg.get("write_files", []) diff --git a/cloudinit/config/modules.py b/cloudinit/config/modules.py index c58afe81dfb..ab3a0e130a2 100644 --- a/cloudinit/config/modules.py +++ b/cloudinit/config/modules.py @@ -7,6 +7,8 @@ # This file is part of cloud-init. See LICENSE file for license information. import copy +from collections import namedtuple +from typing import List from cloudinit import config, importer from cloudinit import log as logging @@ -24,6 +26,9 @@ # we will not find something else with the same # name in the lookup path... MOD_PREFIX = "cc_" +ModuleDetails = namedtuple( + "ModuleDetails", ["module", "name", "frequency", "run_args"] +) def form_module_name(name): @@ -53,6 +58,11 @@ def validate_module(mod, name): f"Module '{mod}' with name '{name}' has an invalid frequency " f"{mod.meta['frequency']}." ) + if hasattr(mod, "schema"): + raise ValueError( + f"Module '{mod}' with name '{name}' has a JSON 'schema' attribute " + "defined. Please define schema in cloud-init-schema,json." + ) class Modules(object): @@ -142,20 +152,9 @@ def _read_modules(self, name): ) return module_list - def _fixup_modules(self, raw_mods): + def _fixup_modules(self, raw_mods) -> List[ModuleDetails]: """Convert list of returned from _read_modules() into new format. - Returns a list of of lists having a module reference, module name, - frequency, and run args. E.g.: - [ - [ - , - "bootcmd", - "always", - "some_arg", - ] - ] - Invalid modules and arguments are ingnored. Also ensures that the module has the required meta fields. """ @@ -187,10 +186,17 @@ def _fixup_modules(self, raw_mods): continue mod = importer.import_module(mod_locs[0]) validate_module(mod, raw_name) - mostly_mods.append([mod, raw_name, freq, run_args]) + mostly_mods.append( + ModuleDetails( + module=mod, + name=raw_name, + frequency=freq, + run_args=run_args, + ) + ) return mostly_mods - def _run_modules(self, mostly_mods): + def _run_modules(self, mostly_mods: List[ModuleDetails]): cc = self.init.cloudify() # Return which ones ran # and which ones failed + the exception of why it failed From 7a6727b19ffa8fe2a9d359fd1a25f3a2380686c2 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Wed, 27 Apr 2022 17:30:27 -0600 Subject: [PATCH 9/9] mypy fix --- cloudinit/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cloudinit/util.py b/cloudinit/util.py index 444f17d6325..9fb0c661616 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -824,7 +824,7 @@ def make_url( def mergemanydict(srcs, reverse=False) -> dict: if reverse: srcs = reversed(srcs) - merged_cfg = {} + merged_cfg: dict = {} for cfg in srcs: if cfg: # Figure out which mergers to apply...