Merge ~rjschwei/cloud-init:emptyStageOK into cloud-init:master

[Robert Schweikert]

stages: fix tracebacks if a module stage is undefined or empty

In /etc/cloud/cloud.cfg, users and imagees can configure which modules run
during a specific cloud-init stage by modifying one of the following
lists: cloud_init_modules, cloud_init_modules, cloud_init_final_modules.

If any of the configured module lists are absent or empty, cloud-init will
emit the same message it already does for existing lists that only contain
modules which are not unsupported on that platform:

No 'config' modules to run under section 'cloud_config_modules'

LP: #1770462

The user may create a config file that has an empty stage such as

cloud_init_modules:

cloud_config_modules:
 - mounts

if a stage is empty a traceback is generated. With this change an informational message logged and continues processing

[Robert Schweikert (rjschwei)]

I looked around the testing code but couldn't figure out where and how I would test this, thus this does not include any testing.

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:f89dc9b8813793992b492a5ce5bef49849448697
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1113/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1113/rebuild

review: Needs Fixing (continuous-integration)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:7c681cc9740a5a15cd3d5346c6e215f28b373ec3
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1114/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1114/rebuild

review: Approve (continuous-integration)

[Chad Smith (chad.smith)]

This log/condition is already 'handled' in cloudinit/cmd/main.py:110 with a message like the following:

No 'config' modules to run under section 'cloud_config_modules', but we should still avoid the Traceback as you say.

you can see this log on a system configured with an empty cloud_config_modules section by running "cloud-init modules --mode config".

root@myb1:~# cloud-init modules --mode config
Cloud-init v. 18.2 running 'modules:config' at Thu, 10 May 2018 19:46:42 +0000. Up 1174.00 seconds.
No 'config' modules to run under section 'cloud_config_modules'

I propose we drop your additional log message, and just return module_list at that point in your patch.

Here is a patch with a test for that behavior not generating a Traceback.
http://paste.ubuntu.com/p/Dkb22CnkMm/

[Chad Smith (chad.smith)]

review: Needs Fixing

[Chad Smith (chad.smith)]

Thanks agian for this branch Robert.

I marked workinprogress on the branch so it's off my radar for the moment. When you feel review comments are addressed and you want eyes on it again, please reset the status at the top to "Needs review"

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:636a27373ef9d7b389a6a273930ab3e0ea73acdc
https://jenkins.ubuntu.com/server/job/cloud-init-ci/45/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/45/rebuild

review: Approve (continuous-integration)

[Chad Smith (chad.smith)]

a very belated approve for this.

review: Approve

[Chad Smith (chad.smith)]

An upstream commit landed for this bug.

To view that commit see the following URL:
https://git.launchpad.net/cloud-init/commit/?id=fef2616b

diff --git a/cloudinit/stages.py b/cloudinit/stages.py
index 3998cf6..286607b 100644
--- a/cloudinit/stages.py
+++ b/cloudinit/stages.py
@@ -697,7 +697,9 @@ class Modules(object):
         module_list = []
         if name not in self.cfg:
             return module_list
-        cfg_mods = self.cfg[name]
+        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
diff --git a/tests/unittests/test_runs/test_simple_run.py b/tests/unittests/test_runs/test_simple_run.py
index 762974e..d67c422 100644
--- a/tests/unittests/test_runs/test_simple_run.py
+++ b/tests/unittests/test_runs/test_simple_run.py
@@ -1,5 +1,6 @@
 # This file is part of cloud-init. See LICENSE file for license information.
 
+import copy
 import os
 
 
@@ -127,8 +128,9 @@ class TestSimpleRun(helpers.FilesystemMockingTestCase):
         """run_section forced skipped modules by using unverified_modules."""
 
         # re-write cloud.cfg with unverified_modules override
-        self.cfg['unverified_modules'] = ['spacewalk']  # Would have skipped
-        cloud_cfg = util.yaml_dumps(self.cfg)
+        cfg = copy.deepcopy(self.cfg)
+        cfg['unverified_modules'] = ['spacewalk']  # Would have skipped
+        cloud_cfg = util.yaml_dumps(cfg)
         util.ensure_dir(os.path.join(self.new_root, 'etc', 'cloud'))
         util.write_file(os.path.join(self.new_root, 'etc',
                                      'cloud', 'cloud.cfg'), cloud_cfg)
@@ -150,4 +152,30 @@ class TestSimpleRun(helpers.FilesystemMockingTestCase):
             "running unverified_modules: 'spacewalk'",
             self.logs.getvalue())
 
+    def test_none_ds_run_with_no_config_modules(self):
+        """run_section will report no modules run when none are configured."""
+
+        # re-write cloud.cfg with unverified_modules override
+        cfg = copy.deepcopy(self.cfg)
+        # Represent empty configuration in /etc/cloud/cloud.cfg
+        cfg['cloud_init_modules'] = None
+        cloud_cfg = util.yaml_dumps(cfg)
+        util.ensure_dir(os.path.join(self.new_root, 'etc', 'cloud'))
+        util.write_file(os.path.join(self.new_root, 'etc',
+                                     'cloud', 'cloud.cfg'), cloud_cfg)
+
+        initer = stages.Init()
+        initer.read_cfg()
+        initer.initialize()
+        initer.fetch()
+        initer.instancify()
+        initer.update()
+        initer.cloudify().run('consume_data', initer.consume_data,
+                              args=[PER_INSTANCE], freq=PER_INSTANCE)
+
+        mods = stages.Modules(initer)
+        (which_ran, failures) = mods.run_section('cloud_init_modules')
+        self.assertTrue(len(failures) == 0)
+        self.assertEqual([], which_ran)
+
 # vi: ts=4 expandtab