Merge ~chad.smith/cloud-init:fix-modules-cmdline-help into cloud-init:master

[Chad Smith]

cli: Fix error in cloud-init modules --mode=init.

The cli help docs and argument parser allow the 'init' mode value
which caused a traceback.

Fix the cli to support 'init', 'config' and 'final' modes for the
cloud-init modules subcommand.

Add a check in the cli to raise a ValueError if a new
subcommand ends up allowing an unsupported/unimplemented modes.

Drive by unit test additions for a bit better coverage of error
handling.

LP: #1736600

cli: Allow providing 'init' mode to cloud-init modules --mode.

The cli help docs and argument parser allow the 'init' mode value
which caused a traceback.

Fix the cli to support 'init', 'config' and 'final' modes for the cloud-init modules subcommand.

Add a check in the cli to raise a ValueError if a new
subcommand ends up allowing an unsupported/unimplemented modes.

Drive by unit test additions for a bit better coverage of error
handling.

LP: #1736600

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

PASSED: Continuous integration, rev:171e54e007a6a5357b194fb5805451a7bc670682
https://jenkins.ubuntu.com/server/job/cloud-init-ci/621/
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/621/rebuild

review: Approve (continuous-integration)

[Chad Smith (chad.smith)]

Thanks your your merge proposal.
Your branch /fix-modules-cmdline-help, needs a commit message that adheres to our git commit message formatting
guidelines before it can be reviewed and landed.

1. All lines must be less than 74 characters.
2. The commit message should adhere to git commit message format conventions:

<Your one-liner subject line>
<empty line>
<more detailed description about the functional changes of your branch>
<optional empty line>
<optional bug link of the format 'LP: #<bug-id>'>

The following issues were found:
The merge proposal /fix-modules-cmdline-help does not have a commit message

Please update your commit message for this branch on launchpad and set the branch back to 'Needs review'.

Thanks again,
Your friendly neighborhood cloud-init robot.

review: Needs Fixing

[Chad Smith (chad.smith)]

Thanks your your merge proposal.
Your branch /fix-modules-cmdline-help, needs a commit message which observes our git commit message format
guidelines before it can be reviewed and landed.

1. All lines must be less than 74 characters.
2. The commit message should adhere to git commit message format:

A one-liner subject line

More detailed paragraphs describing the functional changes of the
branch.

LP: #<bug-id> # if it fixes a specific bug

We discovered the following issues:
------------------------------
The merge proposal /fix-modules-cmdline-help does not have a commit message
------------------------------

Please set the branch back to 'Needs Review' after resolving these issues with
your commit message.

Thanks again,
Your friendly neighborhood cloud-init robot.

review: Needs Fixing

[Chad Smith (chad.smith)]

Thanks your your merge proposal.
Your branch /fix-modules-cmdline-help, needs a commit message which observes our git commit message format
guidelines before it can be reviewed and landed.

1. All lines must be less than 74 characters.
2. The commit message should adhere to git commit message format:

A one-liner subject line

More detailed paragraphs describing the functional changes of the
branch.

LP: #<bug-id> # if it fixes a specific bug

We discovered the following issues:
------------------------------
Commit message lints:
 - Commit message line #4 has 71 too many characters.Line begins with: "which caused a traceback"...
------------------------------

Please set the branch back to 'Needs Review' after resolving these issues with
your commit message.

Thanks again,
Your friendly neighborhood cloud-init robot.

review: Needs Fixing

[Chad Smith (chad.smith)]

was testing a bot voter

review: Abstain

[Scott Moser (smoser)]

love the bot voting.
running the 'init' modules does make sense.
there are 3 sets of modules:
 init, config, final

the 'init' modules just tag along with either init-local or init-net.

This restores functionality to
  cloud-init modules -m init

http://paste.ubuntu.com/26178768/

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

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

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

review: Needs Fixing (continuous-integration)

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

PASSED: Continuous integration, rev:7fb36cd65d7394e638d1cf016baa193dc677ab9e
https://jenkins.ubuntu.com/server/job/cloud-init-ci/630/
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/630/rebuild

review: Approve (continuous-integration)

[Scott Moser (smoser)]

one comment in line on test, take it or leave it.

review: Approve

[Chad Smith (chad.smith)]

Thank you for your merge proposal.

Your branch has been set to 'Work in progress'.
Please set the branch back to 'Needs Review' after resolving the issues below.

Thanks again,
Your friendly neighborhood cloud-init robot.

Your branch /fix-modules-cmdline-help needs a final commit message which observes our
commit message guidelines before it can be reviewed and landed.

1. All lines must be less than 74 characters.
2. The commit message listed in launchpad needs to have the format:

A one-liner subject line

More detailed paragraphs describing the functional changes of the
branch.

LP: #<bug-id> # if it fixes a specific bug

Please fix the following errors:
------------------------------
Commit message lints:
 - Commit message line #6 has 22 too many characters.Line begins with: "Fix the cli to support"...
------------------------------

review: Needs Fixing

[Chad Smith (chad.smith)]

review: Abstain

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

PASSED: Continuous integration, rev:3cf942c94deba0554d0829e13c14d14c9d596be6
https://jenkins.ubuntu.com/server/job/cloud-init-ci/633/
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/633/rebuild

review: Approve (continuous-integration)

diff --git a/cloudinit/cmd/main.py b/cloudinit/cmd/main.py
index aa56225..30b37fe 100644
--- a/cloudinit/cmd/main.py
+++ b/cloudinit/cmd/main.py
@@ -603,7 +603,11 @@ def status_wrapper(name, args, data_d=None, link_d=None):
     else:
         raise ValueError("unknown name: %s" % name)
 
-    modes = ('init', 'init-local', 'modules-config', 'modules-final')
+    modes = ('init', 'init-local', 'modules-init', 'modules-config',
+             'modules-final')
+    if mode not in modes:
+        raise ValueError(
+            "Invalid cloud init mode specified '{0}'".format(mode))
 
     status = None
     if mode == 'init-local':
@@ -615,16 +619,18 @@ def status_wrapper(name, args, data_d=None, link_d=None):
         except Exception:
             pass
 
+    nullstatus = {
+        'errors': [],
+        'start': None,
+        'finished': None,
+    }
     if status is None:
-        nullstatus = {
-            'errors': [],
-            'start': None,
-            'finished': None,
-        }
         status = {'v1': {}}
         for m in modes:
             status['v1'][m] = nullstatus.copy()
         status['v1']['datasource'] = None
+    elif mode not in status['v1']:
+        status['v1'][mode] = nullstatus.copy()
 
     v1 = status['v1']
     v1['stage'] = mode
diff --git a/tests/unittests/test_cli.py b/tests/unittests/test_cli.py
index a8d28ae..0c0f427 100644
--- a/tests/unittests/test_cli.py
+++ b/tests/unittests/test_cli.py
@@ -1,9 +1,12 @@
 # This file is part of cloud-init. See LICENSE file for license information.
 
+from collections import namedtuple
+import os
 import six
 
 from cloudinit.cmd import main as cli
 from cloudinit.tests import helpers as test_helpers
+from cloudinit.util import load_file, load_json
 
 
 mock = test_helpers.mock
@@ -11,6 +14,8 @@ mock = test_helpers.mock
 
 class TestCLI(test_helpers.FilesystemMockingTestCase):
 
+    with_logs = True
+
     def setUp(self):
         super(TestCLI, self).setUp()
         self.stderr = six.StringIO()
@@ -24,6 +29,76 @@ class TestCLI(test_helpers.FilesystemMockingTestCase):
         except SystemExit as e:
             return e.code
 
+    def test_status_wrapper_errors_on_invalid_name(self):
+        """status_wrapper will error when the name parameter is not valid.
+
+        Valid name values are only init and modules.
+        """
+        tmpd = self.tmp_dir()
+        data_d = self.tmp_path('data', tmpd)
+        link_d = self.tmp_path('link', tmpd)
+        FakeArgs = namedtuple('FakeArgs', ['action', 'local', 'mode'])
+
+        def myaction():
+            raise Exception('Should not call myaction')
+
+        myargs = FakeArgs(('doesnotmatter', myaction), False, 'bogusmode')
+        with self.assertRaises(ValueError) as cm:
+            cli.status_wrapper('init1', myargs, data_d, link_d)
+        self.assertEqual('unknown name: init1', str(cm.exception))
+        self.assertNotIn('Should not call myaction', self.logs.getvalue())
+
+    def test_status_wrapper_errors_on_invalid_modes(self):
+        """status_wrapper will error if a parameter combination is invalid."""
+        tmpd = self.tmp_dir()
+        data_d = self.tmp_path('data', tmpd)
+        link_d = self.tmp_path('link', tmpd)
+        FakeArgs = namedtuple('FakeArgs', ['action', 'local', 'mode'])
+
+        def myaction():
+            raise Exception('Should not call myaction')
+
+        myargs = FakeArgs(('modules_name', myaction), False, 'bogusmode')
+        with self.assertRaises(ValueError) as cm:
+            cli.status_wrapper('modules', myargs, data_d, link_d)
+        self.assertEqual(
+            "Invalid cloud init mode specified 'modules-bogusmode'",
+            str(cm.exception))
+        self.assertNotIn('Should not call myaction', self.logs.getvalue())
+
+    def test_status_wrapper_init_local_writes_fresh_status_info(self):
+        """When running in init-local mode, status_wrapper writes status.json.
+
+        Old status and results artifacts are also removed.
+        """
+        tmpd = self.tmp_dir()
+        data_d = self.tmp_path('data', tmpd)
+        link_d = self.tmp_path('link', tmpd)
+        status_link = self.tmp_path('status.json', link_d)
+        # Write old artifacts which will be removed or updated.
+        for _dir in data_d, link_d:
+            test_helpers.populate_dir(
+                _dir, {'status.json': 'old', 'result.json': 'old'})
+
+        FakeArgs = namedtuple('FakeArgs', ['action', 'local', 'mode'])
+
+        def myaction(name, args):
+            # Return an error to watch status capture them
+            return 'SomeDatasource', ['an error']
+
+        myargs = FakeArgs(('ignored_name', myaction), True, 'bogusmode')
+        cli.status_wrapper('init', myargs, data_d, link_d)
+        # No errors reported in status
+        status_v1 = load_json(load_file(status_link))['v1']
+        self.assertEqual(['an error'], status_v1['init-local']['errors'])
+        self.assertEqual('SomeDatasource', status_v1['datasource'])
+        self.assertFalse(
+            os.path.exists(self.tmp_path('result.json', data_d)),
+            'unexpected result.json found')
+        self.assertFalse(
+            os.path.exists(self.tmp_path('result.json', link_d)),
+            'unexpected result.json link found')
+
     def test_no_arguments_shows_usage(self):
         exit_code = self._call_main()
         self.assertIn('usage: cloud-init', self.stderr.getvalue())