Merge "Report the errors raised by _ProcessRemoteHostArtifacts" into main

2 files changed, 44 insertions, 6 deletions

diff --git a/public/actions/remote_host_cf_device_factory.py b/public/actions/remote_host_cf_device_factory.py
index 0abfaa35..134922dd 100644
--- a/public/actions/remote_host_cf_device_factory.py
+++ b/public/actions/remote_host_cf_device_factory.py
@@ -100,10 +100,18 @@ class RemoteHostDeviceFactory(base_device_factory.BaseDeviceFactory):
         deadline = start_time + (self._avd_spec.boot_timeout_secs or
                                  constants.DEFAULT_CF_BOOT_TIMEOUT)
         self._compute_client.SetStage(constants.STAGE_ARTIFACT)
-        # TODO(b/293966645): Handle errors.CreateError.
-        image_args = self._ProcessRemoteHostArtifacts(deadline)
-        start_time = self._compute_client.RecordTime(
-            constants.TIME_ARTIFACT, start_time)
+        try:
+            image_args = self._ProcessRemoteHostArtifacts(deadline)
+        except (errors.CreateError, errors.DriverError,
+                subprocess.CalledProcessError) as e:
+            logger.exception("Fail to prepare artifacts.")
+            self._all_failures[instance] = str(e)
+            # If an SSH error or timeout happens, report the name for the
+            # caller to clean up this instance.
+            return instance
+        finally:
+            start_time = self._compute_client.RecordTime(
+                constants.TIME_ARTIFACT, start_time)
 
         self._compute_client.SetStage(constants.STAGE_BOOT_UP)
         error_msg = self._LaunchCvd(image_args, deadline)
@@ -467,11 +475,14 @@ class RemoteHostDeviceFactory(base_device_factory.BaseDeviceFactory):
 
         Returns:
             A list of string pairs, the replaced arguments.
+
+        Raises:
+            errors.CreateError if any path cannot be replaced.
         """
         if any(remote_path.isabs(path) != remote_path.isabs(old_dir) for
                _, path in launch_cvd_args):
-            raise ValueError(f"Cannot convert {launch_cvd_args} to relative "
-                             f"paths under {old_dir}")
+            raise errors.CreateError(f"Cannot convert {launch_cvd_args} to "
+                                     f"relative paths under {old_dir}")
         return [(option,
                  remote_path.join(new_dir, remote_path.relpath(path, old_dir)))
                 for option, path in launch_cvd_args]
diff --git a/public/actions/remote_host_cf_device_factory_test.py b/public/actions/remote_host_cf_device_factory_test.py
index f1310e80..89c4b05f 100644
--- a/public/actions/remote_host_cf_device_factory_test.py
+++ b/public/actions/remote_host_cf_device_factory_test.py
@@ -18,10 +18,12 @@ import time
 import unittest
 from unittest import mock
 
+from acloud import errors
 from acloud.internal import constants
 from acloud.internal.lib import driver_test_lib
 from acloud.public.actions import remote_host_cf_device_factory
 
+
 class RemoteHostDeviceFactoryTest(driver_test_lib.BaseDriverTest):
     """Test RemoteHostDeviceFactory."""
 
@@ -446,6 +448,31 @@ class RemoteHostDeviceFactoryTest(driver_test_lib.BaseDriverTest):
         self.assertEqual(["arg", "acloud_cf_1/2"],
                          mock_cvd_utils.GetRemoteLaunchCvdCmd.call_args[0][3])
 
+    @mock.patch("acloud.public.actions.remote_host_cf_device_factory.ssh")
+    @mock.patch("acloud.public.actions.remote_host_cf_device_factory."
+                "cvd_utils")
+    @mock.patch("acloud.public.actions.remote_host_cf_device_factory."
+                "subprocess.check_call")
+    def testCreateInstanceWithCreateError(self, _mock_check_call,
+                                          mock_cvd_utils, mock_ssh):
+        """Test CreateInstance with CreateError."""
+        mock_avd_spec = self._CreateMockAvdSpec()
+        mock_avd_spec.remote_image_dir = "mock_img_dir"
+
+        mock_ssh_obj = mock.Mock()
+        mock_ssh.Ssh.return_value = mock_ssh_obj
+
+        mock_cvd_utils.GetRemoteHostBaseDir.return_value = "acloud_cf_1"
+        mock_cvd_utils.FormatRemoteHostInstanceName.return_value = "inst"
+        mock_cvd_utils.LoadRemoteImageArgs.side_effect = errors.CreateError(
+            "failure")
+        factory = remote_host_cf_device_factory.RemoteHostDeviceFactory(
+            mock_avd_spec)
+
+        self.assertEqual("inst", factory.CreateInstance())
+        self.assertEqual({"inst": "failure"}, factory.GetFailures())
+        mock_cvd_utils.ExecuteRemoteLaunchCvd.assert_not_called()
+
 
 if __name__ == "__main__":
     unittest.main()