feat(landscape-client): handle already registered client (#4784)

Use "--is-registered" parameter to prevent duplicate registration 
of clients on a Landscape server when cloud-init runs more than
once.
 
Log a warning if the computer was already registered with Landscape.

Generally, this condition can only happen across system reboot
in cases where:
 - an image that was already registered with landscape was cloned
   and launched as a new instance
 - the cloud platform issued a new instance-id to to the instance
   in cases like image/vm clone, new network configuration or new
   meta-data changes
 - cloud-init clean was run forcing allowing all modules to re-run

cloudinit/config/cc_landscape.py

@@ -151,8 +151,16 @@ def handle(name: str, cfg: Config, cloud: Cloud, args: list) -> None:
             ]
         )
     )
-    subp.subp(["landscape-config", "--silent"] + cmd_params)
-    util.write_file(LS_DEFAULT_FILE, "RUN=1\n")
+    try:
+        subp.subp(["landscape-config", "--silent", "--is-registered"], rcs=[5])
+        subp.subp(["landscape-config", "--silent"] + cmd_params)
+    except subp.ProcessExecutionError as e:
+        if e.exit_code == 0:
+            LOG.warning("Client already registered to Landscape")
+        else:
+            msg = f"Failure registering client:\n{e}"
+            util.logexc(LOG, msg)
+            raise RuntimeError(msg) from e
 
 
 def merge_together(objs):

tests/unittests/config/test_cc_landscape.py

@@ -3,6 +3,7 @@
 
 import pytest
 
+from cloudinit import subp
 from cloudinit.config import cc_landscape
 from cloudinit.config.schema import (
     SchemaValidationError,
@@ -14,8 +15,10 @@
 
 LOG = logging.getLogger(__name__)
 
+MPATH = "cloudinit.config.cc_landscape"
 
-@mock.patch("cloudinit.config.cc_landscape.subp.subp")
+
+@mock.patch(f"{MPATH}.subp.subp")
 class TestLandscape:
     def test_skip_empty_landscape_cloudconfig(self, m_subp):
         """Empty landscape cloud-config section does no work."""
@@ -59,6 +62,9 @@ def test_handler_restarts_landscape_client(self, m_subp, tmpdir):
             ["landscape-client"]
         )
         assert [
+            mock.call(
+                ["landscape-config", "--silent", "--is-registered"], rcs=[5]
+            ),
             mock.call(
                 [
                     "landscape-config",
@@ -91,6 +97,9 @@ def test_handler_installs_client_from_ppa_and_supports_overrides(
             }
         }
         expected_calls = [
+            mock.call(
+                ["landscape-config", "--silent", "--is-registered"], rcs=[5]
+            ),
             mock.call(
                 [
                     "landscape-config",
@@ -122,7 +131,6 @@ def test_handler_installs_client_from_ppa_and_supports_overrides(
             ["landscape-client"]
         )
         assert expected_calls == m_subp.call_args_list
-        assert "RUN=1\n" == default_fn.read()
 
     def test_handler_writes_merged_client_config_file_with_defaults(
         self, m_subp, tmpdir
@@ -136,6 +144,9 @@ def test_handler_writes_merged_client_config_file_with_defaults(
         mycloud.distro = mock.MagicMock()
         cfg = {"landscape": {"client": {}}}
         expected_calls = [
+            mock.call(
+                ["landscape-config", "--silent", "--is-registered"], rcs=[5]
+            ),
             mock.call(
                 [
                     "landscape-config",
@@ -179,6 +190,9 @@ def test_handler_writes_merged_provided_cloudconfig_with_defaults(
         mycloud.distro = mock.MagicMock()
         cfg = {"landscape": {"client": {"computer_title": 'My" PC'}}}
         expected_calls = [
+            mock.call(
+                ["landscape-config", "--silent", "--is-registered"], rcs=[5]
+            ),
             mock.call(
                 [
                     "landscape-config",
@@ -210,6 +224,37 @@ def test_handler_writes_merged_provided_cloudconfig_with_defaults(
         )
         assert expected_calls == m_subp.call_args_list
 
+    @mock.patch(f"{MPATH}.merge_together")
+    def test_handler_client_failed_registering(self, m_merge_together, m_subp):
+        """landscape-client could not be registered"""
+        mycloud = get_cloud("ubuntu")
+        mycloud.distro = mock.MagicMock()
+        cfg = {"landscape": {"client": {"computer_title": 'My" PC'}}}
+        m_subp.side_effect = subp.ProcessExecutionError(
+            "Could not register client"
+        )
+        match = (
+            "Failure registering client:\nUnexpected error while"
+            " running command.\nCommand: -\nExit code: -\nReason: -\n"
+            "Stdout: Could not register client\nStderr: -"
+        )
+        with pytest.raises(RuntimeError, match=match):
+            cc_landscape.handle("notimportant", cfg, mycloud, None)
+
+    @mock.patch(f"{MPATH}.merge_together")
+    def test_handler_client_is_already_registered(
+        self, m_merge_together, m_subp, caplog
+    ):
+        """landscape-client is already registered"""
+        mycloud = get_cloud("ubuntu")
+        mycloud.distro = mock.MagicMock()
+        cfg = {"landscape": {"client": {"computer_title": 'My" PC'}}}
+        m_subp.side_effect = subp.ProcessExecutionError(
+            "Client already registered to Landscape", exit_code=0
+        )
+        cc_landscape.handle("notimportant", cfg, mycloud, None)
+        assert "Client already registered to Landscape" in caplog.text
+
 
 class TestLandscapeSchema:
     @pytest.mark.parametrize(