Remove raise_on_failure from wait calls

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
-                )
-            )

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)

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()