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

2 files changed, 87 insertions, 6 deletions

diff --git a/cloudinit/cmd/main.py b/cloudinit/cmd/main.py
index aa56225d..30b37fe1 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 a8d28ae6..0c0f427a 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())