diff --git a/pycloudlib/instance.py b/pycloudlib/instance.py index b13ceaa8..ed8f4b2b 100644 --- a/pycloudlib/instance.py +++ b/pycloudlib/instance.py @@ -91,16 +91,11 @@ def _wait_for_instance_start(self): detecting when an instance has started through their API. """ - def wait(self, *, raise_on_cloudinit_failure: bool = True): - """Wait for instance to be up and cloud-init to be complete. - - :param raise_on_cloudinit_failure: - If True (the default), raise an `OSError` if cloud-init does not - complete successfully. - """ + def wait(self): + """Wait for instance to be up and cloud-init to be complete.""" self._wait_for_instance_start() self._wait_for_execute() - self._wait_for_cloudinit(raise_on_failure=raise_on_cloudinit_failure) + self._wait_for_cloudinit() @abstractmethod def wait_for_delete(self): @@ -396,13 +391,8 @@ def _wait_for_execute(self): ) ) - def _wait_for_cloudinit(self, *, raise_on_failure: bool): - """Wait until cloud-init has finished. - - :param raise_on_failure: - When `True`, if the process waiting for cloud-init exits non-zero - then this method will raise an `OSError`. - """ + def _wait_for_cloudinit(self): + """Wait until cloud-init has finished.""" self._log.debug('_wait_for_cloudinit to complete') result = self.execute("cloud-init status --help") @@ -424,10 +414,3 @@ def _wait_for_cloudinit(self, *, raise_on_failure: bool): ) ) result = self.execute(cmd, description='waiting for start') - - if raise_on_failure and result.failed: - raise OSError( - 'cloud-init failed to start: out: %s error: %s' % ( - result.stdout, result.stderr - ) - ) diff --git a/pycloudlib/lxd/instance.py b/pycloudlib/lxd/instance.py index 69ab45e4..d2d9a871 100644 --- a/pycloudlib/lxd/instance.py +++ b/pycloudlib/lxd/instance.py @@ -332,42 +332,3 @@ def wait_for_stop(self): return time.sleep(1) raise TimeoutError - - def _wait_for_cloudinit(self, *, raise_on_failure: bool): - """Wait until cloud-init has finished. - - If the instance is a virtual machine, we need to wait for the - lxd-agent to be ready before getting the cloud-init status. - To do that, we retry on vm instances if raise_on_failure - is specified. - - :param raise_on_failure: - When `True`, if the process waiting for cloud-init exits non-zero - then this method will raise an `OSError`. - """ - if self.is_vm: - retries = [30, 45, 60, 75, 90, 105] - - for sleep_time in retries: - try: - super()._wait_for_cloudinit( - raise_on_failure=True) - break - except OSError as e: - # We are getting those types of messages when booting - # lxd vms. However, if we wait a moment before trying - # again, the error fades away. That's why we are treating - # it here. - lxd_errors = [ - "Failed to connect to lxd-agent", - "cloud-init failed to start: out: .......", - "Error: Instance is not running", - ] - if any([x in str(e) for x in lxd_errors]): - time.sleep(sleep_time) - continue - if raise_on_failure: - raise e - else: - super()._wait_for_cloudinit( - raise_on_failure=raise_on_failure) diff --git a/pycloudlib/lxd/tests/test_instance.py b/pycloudlib/lxd/tests/test_instance.py deleted file mode 100644 index 8c6802c9..00000000 --- a/pycloudlib/lxd/tests/test_instance.py +++ /dev/null @@ -1,41 +0,0 @@ -"""Tests related to pycloudlib.lxd.instance module.""" -from unittest import mock -import pytest - -from pycloudlib.instance import BaseInstance -from pycloudlib.lxd.instance import LXDInstance - - -class TestWaitForCloudinit: - """Tests covering pycloudlib.lxd.instance._wait_for_cloudinit method.""" - - @mock.patch("pycloudlib.lxd.instance.time.sleep") - @mock.patch.object(BaseInstance, "_wait_for_cloudinit") - @mock.patch.object(LXDInstance, "is_vm", new_callable=mock.PropertyMock) - def test_wait_for_vm_with_raise_parameter( - self, m_is_vm, m_wait_for_cloudinit, m_sleep - ): # pylint: disable=W0212 - """Test covering _wait_for_cloudinit on LXD vms.""" - m_is_vm.return_value = True - instance = LXDInstance(name=None) - - m_wait_for_cloudinit.side_effect = [ - OSError("Failed to connect to lxd-agent"), - OSError("Failed to connect to lxd-agent"), - OSError( - "cloud-init failed to start: out: ................... error"), - OSError("cloud-init error") - ] - - with pytest.raises(OSError) as excinfo: - instance._wait_for_cloudinit(raise_on_failure=True) - - assert m_wait_for_cloudinit.call_args_list == [ - mock.call(raise_on_failure=True), - mock.call(raise_on_failure=True), - mock.call(raise_on_failure=True), - mock.call(raise_on_failure=True) - ] - assert m_wait_for_cloudinit.call_count == 4 - assert m_sleep.call_count == 3 - assert "cloud-init error" == str(excinfo.value) diff --git a/pycloudlib/tests/test_instance.py b/pycloudlib/tests/test_instance.py index 3eab0901..8c66aac9 100644 --- a/pycloudlib/tests/test_instance.py +++ b/pycloudlib/tests/test_instance.py @@ -1,5 +1,4 @@ """Tests related to pycloudlib.instance module.""" -from contextlib import suppress as noop from unittest import mock import pytest @@ -55,8 +54,7 @@ def test_all_rcs_acceptable(self, instance_cls, cloud_name): class TestWait: """Tests covering pycloudlib.instance.Instance.wait.""" - @pytest.mark.parametrize("raise_on_cloudinit_failure", [True, False, None]) - def test_wait(self, raise_on_cloudinit_failure, concrete_instance_cls): + def test_wait(self, concrete_instance_cls): """Test wait calls the two methods it should with correct passthrough. (`None` is used to test the default.) @@ -68,21 +66,11 @@ def test_wait(self, raise_on_cloudinit_failure, concrete_instance_cls): _wait_for_execute=mock.DEFAULT, _wait_for_cloudinit=mock.DEFAULT, ) as mocks: - if raise_on_cloudinit_failure is not None: - instance.wait( - raise_on_cloudinit_failure=raise_on_cloudinit_failure - ) - else: - instance.wait() + instance.wait() assert 1 == mocks["_wait_for_instance_start"].call_count assert 1 == mocks["_wait_for_execute"].call_count assert 1 == mocks["_wait_for_cloudinit"].call_count - kwargs = mocks["_wait_for_cloudinit"].call_args[1] - if raise_on_cloudinit_failure is None: - # We expect True by default - raise_on_cloudinit_failure = True - assert kwargs["raise_on_failure"] == raise_on_cloudinit_failure @mock.patch.object(BaseInstance, "execute") @mock.patch("pycloudlib.instance.time.sleep") @@ -123,7 +111,7 @@ def side_effect(cmd, *_args, **_kwargs): instance = concrete_instance_cls(key_pair=None) with mock.patch.object(instance, "execute") as m_execute: m_execute.side_effect = side_effect - instance._wait_for_cloudinit(raise_on_failure=True) + instance._wait_for_cloudinit() assert 2 == m_execute.call_count assert ( @@ -153,7 +141,7 @@ def side_effect(cmd, *_args, **_kwargs): instance = concrete_instance_cls(key_pair=None) with mock.patch.object(instance, "execute") as m_execute: m_execute.side_effect = side_effect - instance._wait_for_cloudinit(raise_on_failure=True) + instance._wait_for_cloudinit() assert 2 == m_execute.call_count assert ( @@ -166,16 +154,10 @@ def side_effect(cmd, *_args, **_kwargs): assert "runlevel" in first_arg assert "result.json" in first_arg - @pytest.mark.parametrize( - "raise_on_failure,expectation", - [(True, pytest.raises(OSError)), (False, noop())], - ) @pytest.mark.parametrize("has_wait", [True, False]) def test_failure_path( self, has_wait, - raise_on_failure, - expectation, concrete_instance_cls, ): """Test failure for both has_wait and !has_wait cases.""" @@ -204,8 +186,4 @@ def side_effect(cmd, *_args, **_kwargs): instance = concrete_instance_cls(key_pair=None) with mock.patch.object(instance, "execute") as m_execute: m_execute.side_effect = side_effect - with expectation as excinfo: - instance._wait_for_cloudinit(raise_on_failure=raise_on_failure) - - if raise_on_failure: - assert "out: fail_out error: fail_err" in str(excinfo.value) + instance._wait_for_cloudinit()