Merge lp:~harlowja/cloud-init/cloud-init-dynamic-distro-check into lp:~cloud-init-dev/cloud-init/trunk

Proposed by Joshua Harlow on 2016-07-21
Status: Rejected
Rejected by: Scott Moser on 2017-06-06
Proposed branch: lp:~harlowja/cloud-init/cloud-init-dynamic-distro-check
Merge into: lp:~cloud-init-dev/cloud-init/trunk
Diff against target: 115 lines (+62/-16)
2 files modified
cloudinit/exceptions.py (+28/-0)
cloudinit/stages.py (+34/-16)
To merge this branch: bzr merge lp:~harlowja/cloud-init/cloud-init-dynamic-distro-check
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Needs Fixing on 2016-07-22
cloud-init commiters 2016-07-21 Pending
Review via email: mp+300805@code.launchpad.net
To post a comment you must log in.
1260. By Joshua Harlow on 2016-07-21

Use a more advanced module 'working' function
that by default performs the same distro checking
behavior (but allows modules to provide there
own if they so desire to).

Joshua Harlow (harlowja) wrote :

Yup, I updated it to try that approach instead; seems like its better IMHO.

1261. By Joshua Harlow on 2016-07-22

Rearrange some of the pre_handling code.

FAILED: Continuous integration, rev:1261
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~harlowja/cloud-init/cloud-init-dynamic-distro-check/+merge/300805/+edit-commit-message

https://server-team-jenkins.canonical.com/job/cloud-init-ci/36/
Executed test runs:
    None: https://server-team-jenkins.canonical.com/job/lp-vote-on-merge/10/console

Click here to trigger a rebuild:
https://server-team-jenkins.canonical.com/job/cloud-init-ci/36/rebuild

review: Needs Fixing (continuous-integration)
Scott Moser (smoser) wrote :

Hello,
Thank you for taking the time to contribute to cloud-init. Cloud-init has moved its revision control system to git. As a result, we are marking all bzr merge proposals as 'rejected'. If you would like to re-submit this proposal for review, please do so by following the current HACKING documentation at http://cloudinit.readthedocs.io/en/latest/topics/hacking.html .

Unmerged revisions

1261. By Joshua Harlow on 2016-07-22

Rearrange some of the pre_handling code.

1260. By Joshua Harlow on 2016-07-21

Use a more advanced module 'working' function
that by default performs the same distro checking
behavior (but allows modules to provide there
own if they so desire to).

1259. By Joshua Harlow on 2016-07-21

Allow modules to provide a 'is_usable_on' function

This function, if it exists will allow for the module
to decide if it works on the running distro, making it
a little more flexible (vs the previous distro lists that
are more static) so that the module can itself figure out
if it should be operational or not.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'cloudinit/exceptions.py'
2--- cloudinit/exceptions.py 1970-01-01 00:00:00 +0000
3+++ cloudinit/exceptions.py 2016-07-22 07:30:56 +0000
4@@ -0,0 +1,28 @@
5+# vi: ts=4 expandtab
6+#
7+# Author: Joshua Harlow <harlowja@yahoo-inc.com>
8+#
9+# This program is free software: you can redistribute it and/or modify
10+# it under the terms of the GNU General Public License version 3, as
11+# published by the Free Software Foundation.
12+#
13+# This program is distributed in the hope that it will be useful,
14+# but WITHOUT ANY WARRANTY; without even the implied warranty of
15+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
16+# GNU General Public License for more details.
17+#
18+# You should have received a copy of the GNU General Public License
19+# along with this program. If not, see <http://www.gnu.org/licenses/>.
20+
21+
22+class ModuleNotSupported(Exception):
23+ """Exception raised when a module states it is not supported.
24+
25+ Requires that when raised that (a textual) reason that explains why
26+ the module believes it is not supported is provided so that this reason
27+ can be relayed to users to provide some useful information.
28+ """
29+
30+ def __init__(self, reason):
31+ super(ModuleNotSupported, self).__init__(reason)
32+ self.reason = reason
33
34=== modified file 'cloudinit/stages.py'
35--- cloudinit/stages.py 2016-06-10 21:22:17 +0000
36+++ cloudinit/stages.py 2016-07-22 07:30:56 +0000
37@@ -21,6 +21,7 @@
38 # along with this program. If not, see <http://www.gnu.org/licenses/>.
39
40 import copy
41+import functools
42 import os
43 import sys
44
45@@ -40,6 +41,7 @@
46 from cloudinit import cloud
47 from cloudinit import config
48 from cloudinit import distros
49+from cloudinit import exceptions
50 from cloudinit import helpers
51 from cloudinit import importer
52 from cloudinit import log as logging
53@@ -808,31 +810,47 @@
54 def run_section(self, section_name):
55 raw_mods = self._read_modules(section_name)
56 mostly_mods = self._fixup_modules(raw_mods)
57- d_name = self.init.distro.name
58+ distro = self.init.distro
59+
60+ def _pre_handle(mod, name, cfg, distro, log):
61+ worked_distros = set(mod.distros)
62+ worked_distros.update(
63+ distros.Distro.expand_osfamily(mod.osfamilies))
64+ if len(worked_distros) == 0 or distro.name in worked_distros:
65+ return
66+ reason = ("Module '%s' is not supported on distribution %s"
67+ " as it has stated it only works correctly on"
68+ " %s distributions" % (name, distro.name,
69+ sorted(worked_distros)))
70+ raise exceptions.ModuleNotSupported(reason)
71
72 skipped = []
73 forced = []
74 overridden = self.cfg.get('unverified_modules', [])
75 for (mod, name, _freq, _args) in mostly_mods:
76- worked_distros = set(mod.distros)
77- worked_distros.update(
78- distros.Distro.expand_osfamily(mod.osfamilies))
79-
80- # module does not declare 'distros' or lists this distro
81- if not worked_distros or d_name in worked_distros:
82- continue
83-
84 if name in overridden:
85 forced.append(name)
86 else:
87- skipped.append(name)
88-
89- if skipped:
90- LOG.info("Skipping modules %s because they are not verified "
91- "on distro '%s'. To run anyway, add them to "
92- "'unverified_modules' in config.", skipped, d_name)
93+ try:
94+ pre_handle_func = getattr(mod, 'pre_handle', None)
95+ if not six.callable(pre_handle_func):
96+ # Must be an older module, or something we can't
97+ # call into, so switch to the default that verifies
98+ # the basics...
99+ pre_handle_func = functools.partial(_pre_handle, mod)
100+ pre_handle_func(name, self.cfg, distro, config.LOG)
101+ except exceptions.ModuleNotSupported as e:
102+ LOG.info(
103+ "Skipping module '%s' because %s. To run anyway,"
104+ " add this module name to 'unverified_modules'"
105+ " in config.", name, e.reason)
106+ skipped.append(name)
107+ except Exception:
108+ LOG.exception("Failed at determining if module '%s'"
109+ " can be correctly ran on this system,"
110+ " assuming by default it can be ran.")
111 if forced:
112- LOG.info("running unverified_modules: %s", forced)
113+ LOG.info("Running unverified modules: %s", forced)
114
115 return self._run_modules(mostly_mods)
116