cli: cloud-id report not-run or disabled state as cloud-id (#1162)

This fix has two elements:
 - cloud-init status will not correctly report 'not-run' prior to systemd
   generator running. Only report "disabled" when generator has run
   and /run/cloud-init/disabled exists.
 -  Expose not-run and disabled state in cloud-id responses
 -  Add unique error codes from cloud-id for error, disabled and not-run.
  
The new cloud-id exit codes:
 0: success
 1: error
 2: cloud-init is in disabled state
 3: cloud-init generator has not run yet

5 files changed, 196 insertions, 111 deletions

diff --git a/cloudinit/cmd/cloud_id.py b/cloudinit/cmd/cloud_id.py
index b92b03a8..b71c19de 100755
--- a/cloudinit/cmd/cloud_id.py
+++ b/cloudinit/cmd/cloud_id.py
@@ -6,6 +6,7 @@ import argparse
 import json
 import sys
 
+from cloudinit.cmd.status import UXAppStatus, get_status_details
 from cloudinit.sources import (
     INSTANCE_JSON_FILE,
     METADATA_UNKNOWN,
@@ -62,8 +63,16 @@ def handle_args(name, args):
 
     Print the canonical cloud-id on which the instance is running.
 
-    @return: 0 on success, 1 otherwise.
+    @return: 0 on success, 1 on error, 2 on disabled, 3 on cloud-init not-run.
     """
+    status, _status_details, _time = get_status_details()
+    if status == UXAppStatus.DISABLED:
+        sys.stdout.write("{0}\n".format(status.value))
+        return 2
+    elif status == UXAppStatus.NOT_RUN:
+        sys.stdout.write("{0}\n".format(status.value))
+        return 3
+
     try:
         instance_data = json.load(open(args.instance_data))
     except IOError:
diff --git a/cloudinit/cmd/status.py b/cloudinit/cmd/status.py
index cff16c34..597e5bfb 100644
--- a/cloudinit/cmd/status.py
+++ b/cloudinit/cmd/status.py
@@ -5,6 +5,7 @@
 """Define 'status' utility and handler as part of cloud-init commandline."""
 
 import argparse
+import enum
 import os
 import sys
 from time import gmtime, sleep, strftime
@@ -15,12 +16,17 @@ from cloudinit.util import get_cmdline, load_file, load_json
 
 CLOUDINIT_DISABLED_FILE = "/etc/cloud/cloud-init.disabled"
 
+
 # customer visible status messages
-STATUS_ENABLED_NOT_RUN = "not run"
-STATUS_RUNNING = "running"
-STATUS_DONE = "done"
-STATUS_ERROR = "error"
-STATUS_DISABLED = "disabled"
+@enum.unique
+class UXAppStatus(enum.Enum):
+    """Enum representing user-visible cloud-init application status."""
+
+    NOT_RUN = "not-run"
+    RUNNING = "running"
+    DONE = "done"
+    ERROR = "error"
+    DISABLED = "disabled"
 
 
 def get_parser(parser=None):
@@ -61,23 +67,20 @@ def handle_status_args(name, args):
     # Read configured paths
     init = Init(ds_deps=[])
     init.read_cfg()
-
-    status, status_detail, time = _get_status_details(init.paths)
+    status, status_detail, time = get_status_details(init.paths)
     if args.wait:
-        while status in (STATUS_ENABLED_NOT_RUN, STATUS_RUNNING):
+        while status in (UXAppStatus.NOT_RUN, UXAppStatus.RUNNING):
             sys.stdout.write(".")
             sys.stdout.flush()
-            status, status_detail, time = _get_status_details(init.paths)
+            status, status_detail, time = get_status_details(init.paths)
             sleep(0.25)
         sys.stdout.write("\n")
+    print("status: {0}".format(status.value))
     if args.long:
-        print("status: {0}".format(status))
         if time:
             print("time: {0}".format(time))
         print("detail:\n{0}".format(status_detail))
-    else:
-        print("status: {0}".format(status))
-    return 1 if status == STATUS_ERROR else 0
+    return 1 if status == UXAppStatus.ERROR else 0
 
 
 def _is_cloudinit_disabled(disable_file, paths):
@@ -100,22 +103,29 @@ def _is_cloudinit_disabled(disable_file, paths):
     elif "cloud-init=disabled" in cmdline_parts:
         is_disabled = True
         reason = "Cloud-init disabled by kernel parameter cloud-init=disabled"
-    elif not os.path.exists(os.path.join(paths.run_dir, "enabled")):
+    elif os.path.exists(os.path.join(paths.run_dir, "disabled")):
         is_disabled = True
         reason = "Cloud-init disabled by cloud-init-generator"
-    else:
+    elif os.path.exists(os.path.join(paths.run_dir, "enabled")):
         reason = "Cloud-init enabled by systemd cloud-init-generator"
+    else:
+        reason = "Systemd generator may not have run yet."
     return (is_disabled, reason)
 
 
-def _get_status_details(paths):
+def get_status_details(paths=None):
     """Return a 3-tuple of status, status_details and time of last event.
 
     @param paths: An initialized cloudinit.helpers.paths object.
 
     Values are obtained from parsing paths.run_dir/status.json.
     """
-    status = STATUS_ENABLED_NOT_RUN
+    if not paths:
+        init = Init(ds_deps=[])
+        init.read_cfg()
+        paths = init.paths
+
+    status = UXAppStatus.NOT_RUN
     status_detail = ""
     status_v1 = {}
 
@@ -126,18 +136,18 @@ def _get_status_details(paths):
         CLOUDINIT_DISABLED_FILE, paths
     )
     if is_disabled:
-        status = STATUS_DISABLED
+        status = UXAppStatus.DISABLED
         status_detail = reason
     if os.path.exists(status_file):
         if not os.path.exists(result_file):
-            status = STATUS_RUNNING
+            status = UXAppStatus.RUNNING
         status_v1 = load_json(load_file(status_file)).get("v1", {})
     errors = []
     latest_event = 0
     for key, value in sorted(status_v1.items()):
         if key == "stage":
             if value:
-                status = STATUS_RUNNING
+                status = UXAppStatus.RUNNING
                 status_detail = "Running in stage: {0}".format(value)
         elif key == "datasource":
             status_detail = value
@@ -146,15 +156,15 @@ def _get_status_details(paths):
             start = value.get("start") or 0
             finished = value.get("finished") or 0
             if finished == 0 and start != 0:
-                status = STATUS_RUNNING
+                status = UXAppStatus.RUNNING
             event_time = max(start, finished)
             if event_time > latest_event:
                 latest_event = event_time
     if errors:
-        status = STATUS_ERROR
+        status = UXAppStatus.ERROR
         status_detail = "\n".join(errors)
-    elif status == STATUS_ENABLED_NOT_RUN and latest_event > 0:
-        status = STATUS_DONE
+    elif status == UXAppStatus.NOT_RUN and latest_event > 0:
+        status = UXAppStatus.DONE
     if latest_event:
         time = strftime("%a, %d %b %Y %H:%M:%S %z", gmtime(latest_event))
     else:
diff --git a/doc/man/cloud-id.1 b/doc/man/cloud-id.1
index 59fecdd9..c56587c8 100644
--- a/doc/man/cloud-id.1
+++ b/doc/man/cloud-id.1
@@ -6,6 +6,15 @@ cloud-id \- Report the canonical cloud-id for this instance
 .SH SYNOPSIS
 .BR "cloud-id" " [-h] [-j] [-l] [-i <INSTANCE_DATA>]"
 
+.SH DESCRIPTION
+cloud-id is the lowercase name of the cloud datasource discovered.
+
+The cloud-id will be 'not-run' when systemd generator has not run yet.
+The cloud-id will be 'disabled' when cloud-init is disabled or when
+ds-identify did not find a valid datasource.
+
+See cloud-init status --long for more information.
+
 .SH OPTIONS
 .TP
 .B "-h, --help"
@@ -24,6 +33,20 @@ Report extended cloud-id information as tab-delimited string
 Path to instance-data.json file. Default is
 /run/cloud-init/instance-data.json
 
+.SH EXIT STATUS
+.TP
+0
+On success
+.TP
+1
+Due to an error
+.TP
+2
+Due to cloud-init in a disabled state. See: cloud-init status --long
+.TP
+3
+The cloud-init generator and discovery has not yet run.
+
 .SH COPYRIGHT
 Copyright (C) 2021 Canonical Ltd. License GPL-3 or Apache-2.0
 
diff --git a/tests/unittests/cmd/test_cloud_id.py b/tests/unittests/cmd/test_cloud_id.py
index 42941d4f..907297a6 100644
--- a/tests/unittests/cmd/test_cloud_id.py
+++ b/tests/unittests/cmd/test_cloud_id.py
@@ -3,123 +3,143 @@
 """Tests for cloud-id command line utility."""
 
 from collections import namedtuple
-from io import StringIO
+
+import pytest
 
 from cloudinit import util
 from cloudinit.cmd import cloud_id
-from tests.unittests.helpers import CiTestCase, mock
+from tests.unittests.helpers import mock
 
+M_PATH = "cloudinit.cmd.cloud_id."
 
-class TestCloudId(CiTestCase):
 
-    args = namedtuple("cloudidargs", "instance_data json long")
+class TestCloudId:
 
-    def setUp(self):
-        super(TestCloudId, self).setUp()
-        self.tmp = self.tmp_dir()
-        self.instance_data = self.tmp_path("instance-data.json", dir=self.tmp)
+    args = namedtuple("cloudidargs", "instance_data json long")
 
     def test_cloud_id_arg_parser_defaults(self):
         """Validate the argument defaults when not provided by the end-user."""
         cmd = ["cloud-id"]
         with mock.patch("sys.argv", cmd):
             args = cloud_id.get_parser().parse_args()
-        self.assertEqual(
-            "/run/cloud-init/instance-data.json", args.instance_data
-        )
-        self.assertEqual(False, args.long)
-        self.assertEqual(False, args.json)
+        assert "/run/cloud-init/instance-data.json" == args.instance_data
+        assert False is args.long
+        assert False is args.json
 
-    def test_cloud_id_arg_parse_overrides(self):
+    def test_cloud_id_arg_parse_overrides(self, tmpdir):
         """Override argument defaults by specifying values for each param."""
-        util.write_file(self.instance_data, "{}")
+        instance_data = tmpdir.join("instance-data.json")
+        instance_data.write("{}")
         cmd = [
             "cloud-id",
             "--instance-data",
-            self.instance_data,
+            instance_data.strpath,
             "--long",
             "--json",
         ]
         with mock.patch("sys.argv", cmd):
             args = cloud_id.get_parser().parse_args()
-        self.assertEqual(self.instance_data, args.instance_data)
-        self.assertEqual(True, args.long)
-        self.assertEqual(True, args.json)
-
-    def test_cloud_id_missing_instance_data_json(self):
+        assert instance_data.strpath == args.instance_data
+        assert True is args.long
+        assert True is args.json
+
+    @mock.patch(M_PATH + "get_status_details")
+    def test_cloud_id_missing_instance_data_json(
+        self, get_status_details, tmpdir, capsys
+    ):
         """Exit error when the provided instance-data.json does not exist."""
-        cmd = ["cloud-id", "--instance-data", self.instance_data]
+        get_status_details.return_value = cloud_id.UXAppStatus.DONE, "n/a", ""
+        instance_data = tmpdir.join("instance-data.json")
+        cmd = ["cloud-id", "--instance-data", instance_data.strpath]
         with mock.patch("sys.argv", cmd):
-            with mock.patch("sys.stderr", new_callable=StringIO) as m_stderr:
-                with self.assertRaises(SystemExit) as context_manager:
-                    cloud_id.main()
-        self.assertEqual(1, context_manager.exception.code)
-        self.assertIn(
-            "Error:\nFile not found '%s'" % self.instance_data,
-            m_stderr.getvalue(),
-        )
-
-    def test_cloud_id_non_json_instance_data(self):
+            with pytest.raises(SystemExit) as context_manager:
+                cloud_id.main()
+        assert 1 == context_manager.value.code
+        _out, err = capsys.readouterr()
+        assert "Error:\nFile not found '%s'" % instance_data.strpath in err
+
+    @mock.patch(M_PATH + "get_status_details")
+    def test_cloud_id_non_json_instance_data(
+        self, get_status_details, tmpdir, capsys
+    ):
         """Exit error when the provided instance-data.json is not json."""
-        cmd = ["cloud-id", "--instance-data", self.instance_data]
-        util.write_file(self.instance_data, "{")
+        get_status_details.return_value = cloud_id.UXAppStatus.DONE, "n/a", ""
+        instance_data = tmpdir.join("instance-data.json")
+        cmd = ["cloud-id", "--instance-data", instance_data.strpath]
+        instance_data.write("{")
         with mock.patch("sys.argv", cmd):
-            with mock.patch("sys.stderr", new_callable=StringIO) as m_stderr:
-                with self.assertRaises(SystemExit) as context_manager:
-                    cloud_id.main()
-        self.assertEqual(1, context_manager.exception.code)
-        self.assertIn(
-            "Error:\nFile '%s' is not valid json." % self.instance_data,
-            m_stderr.getvalue(),
+            with pytest.raises(SystemExit) as context_manager:
+                cloud_id.main()
+        assert 1 == context_manager.value.code
+        _out, err = capsys.readouterr()
+        assert (
+            "Error:\nFile '%s' is not valid json." % instance_data.strpath
+            in err
         )
 
-    def test_cloud_id_from_cloud_name_in_instance_data(self):
+    @mock.patch(M_PATH + "get_status_details")
+    def test_cloud_id_from_cloud_name_in_instance_data(
+        self, get_status_details, tmpdir, capsys
+    ):
         """Report canonical cloud-id from cloud_name in instance-data."""
-        util.write_file(
-            self.instance_data,
+        instance_data = tmpdir.join("instance-data.json")
+        get_status_details.return_value = cloud_id.UXAppStatus.DONE, "n/a", ""
+        instance_data.write(
             '{"v1": {"cloud_name": "mycloud", "region": "somereg"}}',
         )
-        cmd = ["cloud-id", "--instance-data", self.instance_data]
+        cmd = ["cloud-id", "--instance-data", instance_data.strpath]
         with mock.patch("sys.argv", cmd):
-            with mock.patch("sys.stdout", new_callable=StringIO) as m_stdout:
-                with self.assertRaises(SystemExit) as context_manager:
-                    cloud_id.main()
-        self.assertEqual(0, context_manager.exception.code)
-        self.assertEqual("mycloud\n", m_stdout.getvalue())
-
-    def test_cloud_id_long_name_from_instance_data(self):
+            with pytest.raises(SystemExit) as context_manager:
+                cloud_id.main()
+        assert 0 == context_manager.value.code
+        out, _err = capsys.readouterr()
+        assert "mycloud\n" == out
+
+    @mock.patch(M_PATH + "get_status_details")
+    def test_cloud_id_long_name_from_instance_data(
+        self, get_status_details, tmpdir, capsys
+    ):
         """Report long cloud-id format from cloud_name and region."""
-        util.write_file(
-            self.instance_data,
+        get_status_details.return_value = cloud_id.UXAppStatus.DONE, "n/a", ""
+        instance_data = tmpdir.join("instance-data.json")
+        instance_data.write(
             '{"v1": {"cloud_name": "mycloud", "region": "somereg"}}',
         )
-        cmd = ["cloud-id", "--instance-data", self.instance_data, "--long"]
+        cmd = ["cloud-id", "--instance-data", instance_data.strpath, "--long"]
         with mock.patch("sys.argv", cmd):
-            with mock.patch("sys.stdout", new_callable=StringIO) as m_stdout:
-                with self.assertRaises(SystemExit) as context_manager:
-                    cloud_id.main()
-        self.assertEqual(0, context_manager.exception.code)
-        self.assertEqual("mycloud\tsomereg\n", m_stdout.getvalue())
-
-    def test_cloud_id_lookup_from_instance_data_region(self):
+            with pytest.raises(SystemExit) as context_manager:
+                cloud_id.main()
+        out, _err = capsys.readouterr()
+        assert 0 == context_manager.value.code
+        assert "mycloud\tsomereg\n" == out
+
+    @mock.patch(M_PATH + "get_status_details")
+    def test_cloud_id_lookup_from_instance_data_region(
+        self, get_status_details, tmpdir, capsys
+    ):
         """Report discovered canonical cloud_id when region lookup matches."""
-        util.write_file(
-            self.instance_data,
+        get_status_details.return_value = cloud_id.UXAppStatus.DONE, "n/a", ""
+        instance_data = tmpdir.join("instance-data.json")
+        instance_data.write(
             '{"v1": {"cloud_name": "aws", "region": "cn-north-1",'
             ' "platform": "ec2"}}',
         )
-        cmd = ["cloud-id", "--instance-data", self.instance_data, "--long"]
+        cmd = ["cloud-id", "--instance-data", instance_data.strpath, "--long"]
         with mock.patch("sys.argv", cmd):
-            with mock.patch("sys.stdout", new_callable=StringIO) as m_stdout:
-                with self.assertRaises(SystemExit) as context_manager:
-                    cloud_id.main()
-        self.assertEqual(0, context_manager.exception.code)
-        self.assertEqual("aws-china\tcn-north-1\n", m_stdout.getvalue())
-
-    def test_cloud_id_lookup_json_instance_data_adds_cloud_id_to_json(self):
+            with pytest.raises(SystemExit) as context_manager:
+                cloud_id.main()
+        assert 0 == context_manager.value.code
+        out, _err = capsys.readouterr()
+        assert "aws-china\tcn-north-1\n" == out
+
+    @mock.patch(M_PATH + "get_status_details")
+    def test_cloud_id_lookup_json_instance_data_adds_cloud_id_to_json(
+        self, get_status_details, tmpdir, capsys
+    ):
         """Report v1 instance-data content with cloud_id when --json set."""
-        util.write_file(
-            self.instance_data,
+        get_status_details.return_value = cloud_id.UXAppStatus.DONE, "n/a", ""
+        instance_data = tmpdir.join("instance-data.json")
+        instance_data.write(
             '{"v1": {"cloud_name": "unknown", "region": "dfw",'
             ' "platform": "openstack", "public_ssh_keys": []}}',
         )
@@ -132,13 +152,36 @@ class TestCloudId(CiTestCase):
                 "region": "dfw",
             }
         )
-        cmd = ["cloud-id", "--instance-data", self.instance_data, "--json"]
+        cmd = ["cloud-id", "--instance-data", instance_data.strpath, "--json"]
+        with mock.patch("sys.argv", cmd):
+            with pytest.raises(SystemExit) as context_manager:
+                cloud_id.main()
+        out, _err = capsys.readouterr()
+        assert 0 == context_manager.value.code
+        assert expected + "\n" == out
+
+    @pytest.mark.parametrize(
+        "status, exit_code",
+        (
+            (cloud_id.UXAppStatus.DISABLED, 2),
+            (cloud_id.UXAppStatus.NOT_RUN, 3),
+            (cloud_id.UXAppStatus.RUNNING, 0),
+        ),
+    )
+    @mock.patch(M_PATH + "get_status_details")
+    def test_cloud_id_unique_exit_codes_for_status(
+        self, get_status_details, status, exit_code, tmpdir, capsys
+    ):
+        """cloud-id returns unique exit codes for status."""
+        get_status_details.return_value = status, "n/a", ""
+        instance_data = tmpdir.join("instance-data.json")
+        if status == cloud_id.UXAppStatus.RUNNING:
+            instance_data.write("{}")
+        cmd = ["cloud-id", "--instance-data", instance_data.strpath, "--json"]
         with mock.patch("sys.argv", cmd):
-            with mock.patch("sys.stdout", new_callable=StringIO) as m_stdout:
-                with self.assertRaises(SystemExit) as context_manager:
-                    cloud_id.main()
-        self.assertEqual(0, context_manager.exception.code)
-        self.assertEqual(expected + "\n", m_stdout.getvalue())
+            with pytest.raises(SystemExit) as context_manager:
+                cloud_id.main()
+        assert exit_code == context_manager.value.code
 
 
 # vi: ts=4 expandtab
diff --git a/tests/unittests/cmd/test_status.py b/tests/unittests/cmd/test_status.py
index acd1fea5..17d27597 100644
--- a/tests/unittests/cmd/test_status.py
+++ b/tests/unittests/cmd/test_status.py
@@ -89,7 +89,7 @@ class TestStatus(CiTestCase):
         )
 
     def test__is_cloudinit_disabled_true_on_kernel_cmdline(self):
-        """When using systemd and disable_file is present return disabled."""
+        """When kernel command line disables cloud-init return True."""
         (is_disabled, reason) = wrap_and_call(
             "cloudinit.cmd.status",
             {
@@ -107,9 +107,9 @@ class TestStatus(CiTestCase):
         )
 
     def test__is_cloudinit_disabled_true_when_generator_disables(self):
-        """When cloud-init-generator doesn't write enabled file return True."""
-        enabled_file = os.path.join(self.paths.run_dir, "enabled")
-        self.assertFalse(os.path.exists(enabled_file))
+        """When cloud-init-generator writes disabled file return True."""
+        disabled_file = os.path.join(self.paths.run_dir, "disabled")
+        ensure_file(disabled_file)
         (is_disabled, reason) = wrap_and_call(
             "cloudinit.cmd.status",
             {"uses_systemd": True, "get_cmdline": "something"},
@@ -137,7 +137,7 @@ class TestStatus(CiTestCase):
         )
 
     def test_status_returns_not_run(self):
-        """When status.json does not exist yet, return 'not run'."""
+        """When status.json does not exist yet, return 'not-run'."""
         self.assertFalse(
             os.path.exists(self.status_file), "Unexpected status.json found"
         )
@@ -154,7 +154,7 @@ class TestStatus(CiTestCase):
                 cmdargs,
             )
         self.assertEqual(0, retcode)
-        self.assertEqual("status: not run\n", m_stdout.getvalue())
+        self.assertEqual("status: not-run\n", m_stdout.getvalue())
 
     def test_status_returns_disabled_long_on_presence_of_disable_file(self):
         """When cloudinit is disabled, return disabled reason."""