diff --git a/VERSION b/VERSION index 7cab95ee..63115ced 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1!10.16.0 +1!10.17.0 diff --git a/pycloudlib/instance.py b/pycloudlib/instance.py index 9a2a5dbe..8c4df070 100644 --- a/pycloudlib/instance.py +++ b/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 -c 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): diff --git a/tests/unit_tests/test_instance.py b/tests/unit_tests/test_instance.py index 4f7884a2..4c88acd4 100644 --- a/tests/unit_tests/test_instance.py +++ b/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