cloud-init-dev team mailing list archive

[Lars Kellogg-Stedman <lars@xxxxxxxxxx>]

Diff comments:

> === modified file 'cloudinit/stages.py'
> --- cloudinit/stages.py	2016-06-10 21:22:17 +0000
> +++ cloudinit/stages.py	2016-07-21 18:53:48 +0000
> @@ -808,31 +808,43 @@
>      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
> +        distro = self.init.distro
> +
> +        def is_usable_here(mod):
> +            # Ok how this works is the following, if the module provides
> +            # a distro set and its empty, then its accepted to work everywhere
> +            # if it is not empty and the current distro is in that set
> +            # then its accepted to work, otherwise if the module provides
> +            # a 'is_usable_on' function that takes one argument (the distro
> +            # being loaded) and returns true then it is expected to work,
> +            # otherwise it is not expected to work in the current system
> +            # and it will be skipped (unless the module name has itself been
> +            # included via a 'unverified_modules' configuration set/list).

I think we need to get away from thinking of this as *just* a distribution check.  This should be a general can-i-run-or-not check, and the language in the log messages, etc, should reflect that.  Ideally, a module would provide a failure message indicating why it is not usable (something like: raise ModuleNotUsable('puppet is not installed'), or maybe just a return a tuple (status, reason) or something) and we would emit this in our log message, rather than the current "...because they are not verified on distro..." message.  Instead we would have a per-module message like: LOG.info("skipping module %s: %s", module, reason)

> +            worked_distros = set(mod.distros)
> +            worked_distros.update(
> +                distros.Distro.expand_osfamily(mod.osfamilies))
> +            if len(worked_distros) == 0 or distro.name in worked_distros:
> +                return True
> +            is_usable_func = getattr(mod, 'is_usable_on', None)
> +            if is_usable_func is not None and six.callable(is_usable_func):
> +                return is_usable_func(distro)
> +            return False
>  
>          skipped = []
>          forced = []
>          overridden = self.cfg.get('unverified_modules', [])
>          for (mod, name, _freq, _args) in mostly_mods:
> -            worked_distros = set(mod.distros)
> -            worked_distros.update(
> -                distros.Distro.expand_osfamily(mod.osfamilies))
> -
> -            # module does not declare 'distros' or lists this distro
> -            if not worked_distros or d_name in worked_distros:
> -                continue
> -
>              if name in overridden:
>                  forced.append(name)
>              else:
> -                skipped.append(name)
> -
> +                if not is_usable_here(mod):
> +                    skipped.append(name)
>          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.", skipped, d_name)
> +                     "'unverified_modules' in config.", skipped, distro.name)
>          if forced:
> -            LOG.info("running unverified_modules: %s", forced)
> +            LOG.info("Running unverified modules: %s", forced)
>  
>          return self._run_modules(mostly_mods)
>  


-- 
https://code.launchpad.net/~harlowja/cloud-init/cloud-init-dynamic-distro-check/+merge/300805
Your team cloud init development team is requested to review the proposed merge of lp:~harlowja/cloud-init/cloud-init-dynamic-distro-check into lp:cloud-init.