update cloud-init clean cmdline remove additional network, machine-id and cloud artifacts

VERSION

@@ -1 +1 @@
-1!10.17.0
+1!10.18.0

pycloudlib/instance.py

@@ -232,15 +232,11 @@ def clean(self):
 
         This will clean out specifically the cloud-init files and system logs.
         """
-        # Note: revert this commit once bionic-pro images contain
-        # cloud-init >= v23.1 .
-        # We end up hitting LP: #1508766 on systemd == 237 (bionic) because
-        # the cloud-init's fix [1] for LP: #1999680 is not included on some
-        # bionic-pro cloud images.
-        #
-        # [1] https://github.com/canonical/cloud-init/commit/abfdf1d83995cc20e
-        self.execute("sudo cloud-init clean --logs")
-        self.execute("sudo echo 'uninitialized' > /etc/machine-id")
+        result = self.execute("sudo cloud-init clean --logs --machine-id --config all")
+        if result.failed and "unrecognized arguments" in result.stderr:
+            # Cope with cloud-init version < 23.4 which has no -c argument
+            # or version < 23.1 which has no --machine-id argument.
+            result = self.execute("sudo cloud-init clean --logs")
         self.execute("sudo rm -rf /var/log/syslog")
 
     def _run_command(self, command, stdin, get_pty=False):

tests/unit_tests/test_instance.py

@@ -1,7 +1,6 @@
 """Tests related to pycloudlib.instance module."""
 
 from itertools import repeat
-from unittest import mock
 
 import pytest
 from paramiko import SSHException
@@ -15,31 +14,31 @@
 
 
 @pytest.fixture
-def concrete_instance_cls():
+def concrete_instance_cls(mocker):
     """Return a BaseInstance subclass which can be instantiated.
 
     Source: https://stackoverflow.com/a/28738073
     """
-    with mock.patch.object(BaseInstance, "__abstractmethods__", set()):
-        yield BaseInstance
+    mocker.patch.object(BaseInstance, "__abstractmethods__", set())
+    return BaseInstance
 
 
 class TestWait:
     """Tests covering pycloudlib.instance.Instance.wait."""
 
-    def test_wait(self, concrete_instance_cls):
+    def test_wait(self, concrete_instance_cls, mocker):
         """Test wait calls the two methods it should with correct passthrough.
 
         (`None` is used to test the default.)
         """
         instance = concrete_instance_cls(key_pair=None)
-        with mock.patch.multiple(
+        mocks = mocker.patch.multiple(
             instance,
-            _wait_for_instance_start=mock.DEFAULT,
-            _wait_for_execute=mock.DEFAULT,
-            _wait_for_cloudinit=mock.DEFAULT,
-        ) as mocks:
-            instance.wait()
+            _wait_for_instance_start=mocker.DEFAULT,
+            _wait_for_execute=mocker.DEFAULT,
+            _wait_for_cloudinit=mocker.DEFAULT,
+        )
+        instance.wait()
 
         assert 1 == mocks["_wait_for_instance_start"].call_count
         assert 1 == mocks["_wait_for_execute"].call_count
@@ -52,25 +51,24 @@ def test_wait(self, concrete_instance_cls):
             pytest.param(SSHException, id="exception"),
         ],
     )
-    @mock.patch.object(BaseInstance, "execute")
-    @mock.patch("pycloudlib.instance.time.sleep")
-    @mock.patch("pycloudlib.instance.time.time")
-    @mock.patch("logging.Logger.debug")
     def test_wait_execute_failure(
         self,
-        m_debug,
-        m_time,
-        m_sleep,
-        m_execute,
         execute_effect,
         concrete_instance_cls,
+        mocker,
     ):
         """Test wait calls when execute command fails."""
+        mocker.patch("logging.Logger.debug")
+        mocker.patch("logging.Logger.info")
+        m_time = mocker.patch("pycloudlib.instance.time.time")
+        m_sleep = mocker.patch("pycloudlib.instance.time.sleep")
+        m_execute = mocker.patch.object(BaseInstance, "execute")
+
         instance = concrete_instance_cls(key_pair=None)
-        m_time.side_effect = [1, 1, 2, 10 * 60, 10 * 60 + 1]
+        m_time.side_effect = [1, 1, 2, 10 * 60 + 1]
         m_execute.side_effect = execute_effect
         expected_msg = "Instance can't be reached after 10 minutes. Failed to obtain new boot id"
-        expected_call_args = [mock.call("cat /proc/sys/kernel/random/boot_id", no_log=True)] * 2
+        expected_call_args = [mocker.call("cat /proc/sys/kernel/random/boot_id", no_log=True)] * 2
 
         with pytest.raises(PycloudlibTimeoutError) as excinfo:
             instance.wait()
@@ -80,89 +78,77 @@ def test_wait_execute_failure(
         assert expected_call_args == m_execute.call_args_list
 
 
-@mock.patch("pycloudlib.instance.BaseInstance._do_restart")
-@mock.patch("pycloudlib.instance.BaseInstance.get_boot_id")
-@mock.patch("pycloudlib.instance.BaseInstance.wait")
-@mock.patch("pycloudlib.instance.BaseInstance.wait_for_restart")
 class TestRestart:
     """Test base restart behavior."""
 
     @pytest.fixture(autouse=True)
-    def unchecked_mocks(self):
+    def setup_mocks(self, mocker):
         """Mock things we don't want as test parameters."""
-        with mock.patch("pycloudlib.instance.BaseInstance._sync_filesystem"):
-            yield
+        mocker.patch("pycloudlib.instance.BaseInstance._sync_filesystem")
+        self.m_wait_for_restart = mocker.patch("pycloudlib.instance.BaseInstance.wait_for_restart")
+        self.m_wait = mocker.patch("pycloudlib.instance.BaseInstance.wait")
+        self.m_boot_id = mocker.patch("pycloudlib.instance.BaseInstance.get_boot_id")
+        self.m_do_restart = mocker.patch("pycloudlib.instance.BaseInstance._do_restart")
 
     def test_no_wait(
         self,
-        m_wait_for_restart,
-        m_wait,
-        m_boot_id,
-        m_do_restart,
         concrete_instance_cls,
     ):
         """Test wait=False."""
         instance = concrete_instance_cls(key_pair=None)
         instance.restart(wait=False)
-        assert m_do_restart.call_count == 1
-        assert m_boot_id.call_count == 0
-        assert m_wait_for_restart.call_count == 0
-        assert m_wait.call_count == 0
+        assert self.m_do_restart.call_count == 1
+        assert self.m_boot_id.call_count == 0
+        assert self.m_wait_for_restart.call_count == 0
+        assert self.m_wait.call_count == 0
 
     def test_instance_not_reachable(
         self,
-        m_wait_for_restart,
-        m_wait,
-        m_boot_id,
-        m_do_restart,
         concrete_instance_cls,
     ):
         """Test when instance is not reachable."""
         instance = concrete_instance_cls(key_pair=None)
-        m_boot_id.side_effect = SSHException
+        self.m_boot_id.side_effect = SSHException
         instance.restart(wait=True)
-        assert m_do_restart.call_count == 1
-        assert m_wait_for_restart.call_count == 0
-        assert m_wait.call_count == 1
+        assert self.m_do_restart.call_count == 1
+        assert self.m_wait_for_restart.call_count == 0
+        assert self.m_wait.call_count == 1
 
     def test_instance_reachable(
         self,
-        m_wait_for_restart,
-        m_wait,
-        m_boot_id,
-        m_do_restart,
         concrete_instance_cls,
     ):
         """Test when instance is reachable."""
         instance = concrete_instance_cls(key_pair=None)
-        m_boot_id.side_effect = Result("11111111-1111-1111-1111-111111111111", "", 0)
+        self.m_boot_id.side_effect = Result("11111111-1111-1111-1111-111111111111", "", 0)
         instance.restart(wait=True)
-        assert m_do_restart.call_count == 1
-        assert m_wait_for_restart.call_count == 1
-        assert m_wait.call_count == 0
+        assert self.m_do_restart.call_count == 1
+        assert self.m_wait_for_restart.call_count == 1
+        assert self.m_wait.call_count == 0
 
 
 class TestWaitForRestart:
     """Tests covering pycloudlib.instance.Instance.wait_for_restart."""
 
-    @mock.patch.object(
-        BaseInstance,
-        "execute",
-        side_effect=[
-            Result("11111111-1111-1111-1111-111111111111", "", 0),
-            Result("11111111-1111-1111-1111-111111111111", "", 0),
-            Result("11111111-1111-1111-1111-111111111111", "", 0),
-            Result("11111111-1111-1111-1111-111111111111", "", 0),
-            Result("22222222-2222-2222-2222-222222222222", "", 0),
-        ],
-    )
-    @mock.patch.object(BaseInstance, "_wait_for_cloudinit")
-    @mock.patch("pycloudlib.instance.time.sleep")
-    @mock.patch("pycloudlib.instance.time.time", return_value=1)
     def test_wait_for_restart(
-        self, _m_time, _m_sleep, _m_wait_ci, m_execute, concrete_instance_cls
+        self, concrete_instance_cls, mocker
     ):
         """Test wait calls _wait_for_execute and waits till differing."""
+        mocker.patch("pycloudlib.instance.time.time", return_value=1)
+        mocker.patch("pycloudlib.instance.time.sleep")
+        mocker.patch.object(BaseInstance, "_wait_for_cloudinit")
+        m_execute = mocker.patch.object(
+            BaseInstance,
+            "execute",
+            side_effect=[
+                Result("11111111-1111-1111-1111-111111111111", "", 0),
+                Result("11111111-1111-1111-1111-111111111111", "", 0),
+                Result("11111111-1111-1111-1111-111111111111", "", 0),
+                Result("11111111-1111-1111-1111-111111111111", "", 0),
+                Result("22222222-2222-2222-2222-222222222222", "", 0),
+            ],
+        )
+
         instance = concrete_instance_cls(key_pair=None)
         instance.wait_for_restart(old_boot_id="11111111-1111-1111-1111-111111111111")
         assert m_execute.call_count == 5
@@ -174,25 +160,24 @@ def test_wait_for_restart(
             repeat(Result("11111111-1111-1111-1111-111111111111", "", 0)),
         ],
     )
-    @mock.patch.object(BaseInstance, "execute")
-    @mock.patch("pycloudlib.instance.time.sleep")
-    @mock.patch("pycloudlib.instance.time.time")
-    @mock.patch("logging.Logger.debug")
     def test_boot_id_failure(
         self,
-        m_debug,
-        m_time,
-        m_sleep,
-        m_execute,
         execute_side_effect,
         concrete_instance_cls,
+        mocker,
     ):
         """Test wait calls when execute command fails."""
+        mocker.patch("logging.Logger.debug")
+        mocker.patch("logging.Logger.info")
+        m_time = mocker.patch("pycloudlib.instance.time.time")
+        m_sleep = mocker.patch("pycloudlib.instance.time.sleep")
+        m_execute = mocker.patch.object(BaseInstance, "execute")
+
         m_execute.side_effect = execute_side_effect
         instance = concrete_instance_cls(key_pair=None)
-        m_time.side_effect = [1, 1, 2, 10 * 60, 10 * 60 + 1]
+        m_time.side_effect = [1, 1, 2, 10 * 60 + 1]
         expected_msg = "Instance can't be reached after 10 minutes. Failed to obtain new boot id"
-        expected_call_args = [mock.call("cat /proc/sys/kernel/random/boot_id", no_log=True)] * 2
+        expected_call_args = [mocker.call("cat /proc/sys/kernel/random/boot_id", no_log=True)] * 2
 
         with pytest.raises(PycloudlibTimeoutError) as excinfo:
             instance.wait_for_restart(old_boot_id="11111111-1111-1111-1111-111111111111")
@@ -207,44 +192,116 @@ def test_boot_id_failure(
 class TestWaitForCloudinit:
     """Tests covering pycloudlib.instance.Instance._wait_for_cloudinit."""
 
-    def test_with_wait_available(self, concrete_instance_cls):
+    def test_with_wait_available(self, concrete_instance_cls, mocker):
         """Test the happy path for instances with `status --wait`."""
         instance = concrete_instance_cls(key_pair=None)
-        with mock.patch.object(instance, "execute") as m_execute:
-            instance._wait_for_cloudinit()
+        m_execute = mocker.patch.object(instance, "execute")
+        instance._wait_for_cloudinit()
 
         assert (
-            mock.call(
+            mocker.call(
                 ["cloud-init", "status", "--wait", "--long"],
                 description="waiting for start",
             )
             == m_execute.call_args
         )
 
-    @mock.patch("time.sleep")
-    def test_wait_on_target_not_active(self, _m_sleep, concrete_instance_cls):
+    def test_wait_on_target_not_active(self, concrete_instance_cls, mocker):
         """Test that we wait for cloud-init is-active before calling status."""
+        mocker.patch("time.sleep")
         instance = concrete_instance_cls(key_pair=None)
-        with mock.patch.object(
+        m_execute = mocker.patch.object(
             instance,
             "execute",
             side_effect=[Result("", "", 0)] + [Result("", "", 1)] * 500,
-        ) as m_execute:
-            instance._wait_for_cloudinit()
+        )
+
+        instance._wait_for_cloudinit()
         expected = [
-            mock.call("command -v systemctl"),
+            mocker.call("command -v systemctl"),
             *(
                 [
-                    mock.call(
+                    mocker.call(
                         ["systemctl", "is-active", "cloud-init.target"],
                         no_log=True,
                     )
                 ]
                 * 300
             ),
-            mock.call(
+            mocker.call(
                 ["cloud-init", "status", "--wait", "--long"],
                 description="waiting for start",
             ),
         ]
         assert expected == m_execute.call_args_list
+
+
+class TestClean:
+    """Tests covering pycloudlib.instance.BaseInstance.clean."""
+
+    def test_clean_with_c_all_support(self, concrete_instance_cls, mocker):
+        """Test clean method when cloud-init supports -c all option."""
+        instance = concrete_instance_cls(key_pair=None)
+        m_execute = mocker.patch.object(
+            instance,
+            "execute",
+            side_effect=[
+                Result("", "", 0),  # cloud-init clean --logs --machine-id -c all
+                Result("", "", 0),  # rm -rf /var/log/syslog
+            ],
+        )
+        instance.clean()
+
+        expected_calls = [
+            mocker.call("sudo cloud-init clean --logs --machine-id -c all"),
+            mocker.call("sudo rm -rf /var/log/syslog"),
+        ]
+        assert expected_calls == m_execute.call_args_list
+
+    def test_clean_without_arg_support(self, concrete_instance_cls, mocker):
+        """Test clean method when cloud-init doesn't support -c all option."""
+        instance = concrete_instance_cls(key_pair=None)
+        m_execute = mocker.patch.object(
+            instance,
+            "execute",
+            side_effect=[
+                Result(
+                    "",
+                    "cloud-init clean: error: unrecognized arguments: -c all",
+                    2,
+                ),  # First attempt fails
+                Result("", "", 0),  # Fallback without -c all
+                Result("", "", 0),  # rm -rf /var/log/syslog
+            ],
+        )
+        instance.clean()
+
+        expected_calls = [
+            mocker.call("sudo cloud-init clean --logs --machine-id -c all"),
+            mocker.call("sudo cloud-init clean --logs"),
+            mocker.call("sudo rm -rf /var/log/syslog"),
+        ]
+        assert expected_calls == m_execute.call_args_list
+
+    def test_clean_unexpected_error(self, concrete_instance_cls, mocker):
+        """Test clean method does not fallback on unexpected error."""
+        instance = concrete_instance_cls(key_pair=None)
+        m_execute = mocker.patch.object(
+            instance,
+            "execute",
+            side_effect=[
+                Result(
+                    "",
+                    "cloud-init clean: error: permission denied",
+                    1,
+                ),  # Different error
+                Result("", "", 0),  # rm -rf /var/log/syslog
+            ],
+        )
+        instance.clean()
+
+        expected_calls = [
+            mocker.call("sudo cloud-init clean --logs --machine-id -c all"),
+            mocker.call("sudo rm -rf /var/log/syslog"),
+        ]
+        assert expected_calls == m_execute.call_args_list