build providers: destroy on create failures

snap/snapcraft.yaml

@@ -15,6 +15,7 @@ apps:
 
 build-packages:
   - build-essential
+  - intltool
   - libapt-pkg-dev
   - libffi-dev
   - libssl-dev

snapcraft/internal/build_providers/_base_provider.py

@@ -86,15 +86,21 @@ def __init__(self, *, project, echoer, is_ephemeral: bool = False) -> None:
             self.snap_filename = "{}_{}.snap".format(
                 project.info.name, project.deb_arch
             )
+
         self.provider_project_dir = os.path.join(
             BaseDirectory.save_data_path("snapcraft"),
             "projects",
-            self._get_provider_name(),
             project.info.name,
+            self._get_provider_name(),
         )
 
     def __enter__(self):
-        self.create()
+        try:
+            self.create()
+        except errors.ProviderBaseError:
+            # Destroy is idempotent
+            self.destroy()
+            raise
         return self
 
     def __exit__(self, exc_type, exc_val, exc_tb):

snapcraft/internal/build_providers/_multipass/_multipass.py

@@ -150,20 +150,25 @@ def create(self) -> None:
 
     def destroy(self) -> None:
         """Destroy the instance, trying to stop it first."""
-        if self._instance_info is None:
+        try:
+            instance_info = self._instance_info = self._get_instance_info()
+        except errors.ProviderInfoError:
+            return
+
+        if instance_info.is_stopped():
             return
 
-        if not self._instance_info.is_stopped():
-            stop_time = _get_stop_time()
-            if stop_time > 0:
-                try:
-                    self._multipass_cmd.stop(
-                        instance_name=self.instance_name, time=stop_time
-                    )
-                except errors.ProviderStopError:
-                    self._multipass_cmd.stop(instance_name=self.instance_name)
-            else:
+        stop_time = _get_stop_time()
+        if stop_time > 0:
+            try:
+                self._multipass_cmd.stop(
+                    instance_name=self.instance_name, time=stop_time
+                )
+            except errors.ProviderStopError:
                 self._multipass_cmd.stop(instance_name=self.instance_name)
+        else:
+            self._multipass_cmd.stop(instance_name=self.instance_name)
+
         if self._is_ephemeral:
             self.clean_project()
 

snapcraft/internal/build_providers/errors.py

@@ -21,7 +21,11 @@
 from snapcraft.internal.errors import SnapcraftError as _SnapcraftError
 
 
-class ProviderNotSupportedError(_SnapcraftError):
+class ProviderBaseError(_SnapcraftError):
+    """Base Exception for all provider related exceptions."""
+
+
+class ProviderNotSupportedError(ProviderBaseError):
 
     fmt = (
         "The {provider!r} provider is not supported, please choose a "
@@ -32,7 +36,7 @@ def __init__(self, *, provider: str) -> None:
         super().__init__(provider=provider)
 
 
-class ProviderCommandNotFound(_SnapcraftError):
+class ProviderCommandNotFound(ProviderBaseError):
 
     fmt = (
         "{command!r} command not found: this command is necessary to build in "
@@ -45,7 +49,7 @@ def __init__(self, *, command: str) -> None:
         super().__init__(command=command)
 
 
-class _GenericProviderError(_SnapcraftError):
+class _GenericProviderError(ProviderBaseError):
 
     fmt = (
         "An error occurred when trying to {action} the instance with "
@@ -54,7 +58,7 @@ class _GenericProviderError(_SnapcraftError):
     )
 
 
-class ProviderCommunicationError(_SnapcraftError):
+class ProviderCommunicationError(ProviderBaseError):
 
     fmt = (
         "An error occurred when trying to communicate with the instance "
@@ -93,7 +97,7 @@ def __init__(self, *, provider_name: str, exit_code: int) -> None:
         )
 
 
-class ProviderExecError(_SnapcraftError):
+class ProviderExecError(ProviderBaseError):
 
     fmt = (
         "An error occurred when trying to execute {command_string!r} with "
@@ -112,7 +116,7 @@ def __init__(
         )
 
 
-class ProviderShellError(_SnapcraftError):
+class ProviderShellError(ProviderBaseError):
 
     fmt = (
         "An error occurred when trying to provide a shell with "
@@ -123,7 +127,7 @@ def __init__(self, *, provider_name: str, exit_code: int) -> None:
         super().__init__(provider_name=provider_name, exit_code=exit_code)
 
 
-class ProviderMountError(_SnapcraftError):
+class ProviderMountError(ProviderBaseError):
 
     fmt = (
         "An error occurred when trying to mount using {provider_name!r}: "
@@ -134,7 +138,7 @@ def __init__(self, *, provider_name: str, exit_code: int) -> None:
         super().__init__(provider_name=provider_name, exit_code=exit_code)
 
 
-class ProviderUnMountError(_SnapcraftError):
+class ProviderUnMountError(ProviderBaseError):
 
     fmt = (
         "An error occurred when trying to unmount using {provider_name!r}: "
@@ -145,7 +149,7 @@ def __init__(self, *, provider_name: str, exit_code: int) -> None:
         super().__init__(provider_name=provider_name, exit_code=exit_code)
 
 
-class ProviderFileCopyError(_SnapcraftError):
+class ProviderFileCopyError(ProviderBaseError):
 
     fmt = (
         "An error occurred when trying to copy files using {provider_name!r}: "
@@ -156,7 +160,7 @@ def __init__(self, *, provider_name: str, exit_code: int) -> None:
         super().__init__(provider_name=provider_name, exit_code=exit_code)
 
 
-class ProviderInfoError(_SnapcraftError):
+class ProviderInfoError(ProviderBaseError):
 
     fmt = (
         "An error occurred when using {provider_name!r} to "
@@ -169,15 +173,15 @@ def __init__(self, *, provider_name: str, exit_code: int, stderr: bytes) -> None
         )
 
 
-class ProviderInstanceNotFoundError(_SnapcraftError):
+class ProviderInstanceNotFoundError(ProviderBaseError):
 
     fmt = "Cannot find an instance named {instance_name!r}."
 
     def __init__(self, *, instance_name: str) -> None:
         super().__init__(instance_name=instance_name)
 
 
-class ProviderInfoDataKeyError(_SnapcraftError):
+class ProviderInfoDataKeyError(ProviderBaseError):
 
     fmt = (
         "The data returned by {provider_name!r} was not expected. "
@@ -192,7 +196,7 @@ def __init__(
         )
 
 
-class ProviderBadDataError(_SnapcraftError):
+class ProviderBadDataError(ProviderBaseError):
 
     fmt = (
         "The data returned by {provider_name!r} was not expected "

tests/unit/build_providers/__init__.py

@@ -72,7 +72,6 @@ def build_project(self) -> None:
 
     def create(self):
         self.create_mock("create")
-        # raise EnvironmentError(self.create_mock.call_args)
 
     def destroy(self):
         self.destroy_mock("destroy")

tests/unit/build_providers/multipass/test_multipass.py

@@ -94,6 +94,7 @@ def setUp(self):
                 provider_name="multipass", exit_code=1, stderr=b"error"
             ),
             _DEFAULT_INSTANCE_INFO.encode(),
+            _DEFAULT_INSTANCE_INFO.encode(),
         ]
 
     def test_ephemeral_instance_with_contextmanager(self):
@@ -134,7 +135,7 @@ def test_ephemeral_instance_with_contextmanager(self):
                 ),
             ]
         )
-        self.assertThat(self.multipass_cmd_mock().info.call_count, Equals(2))
+        self.assertThat(self.multipass_cmd_mock().info.call_count, Equals(3))
         self.multipass_cmd_mock().info.assert_has_calls(
             [
                 mock.call(instance_name=self.instance_name, output_format="json"),
@@ -399,6 +400,7 @@ def execute_effect(*, command, instance_name, hide_output):
                 provider_name="multipass", exit_code=1, stderr=b"error"
             ),
             _DEFAULT_INSTANCE_INFO.encode(),
+            _DEFAULT_INSTANCE_INFO.encode(),
         ]
 
     def test_lifecycle(self):
@@ -435,7 +437,7 @@ def test_lifecycle(self):
             target="{}:{}".format(self.instance_name, "/home/multipass/project"),
         )
         self.multipass_cmd_mock().umount.assert_not_called()
-        self.assertThat(self.multipass_cmd_mock().info.call_count, Equals(2))
+        self.assertThat(self.multipass_cmd_mock().info.call_count, Equals(3))
         self.multipass_cmd_mock().info.assert_has_calls(
             [
                 mock.call(instance_name=self.instance_name, output_format="json"),

tests/unit/build_providers/test_base_provider.py

@@ -14,8 +14,9 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+import contextlib
 import os
-from unittest.mock import call
+from unittest.mock import call, Mock
 
 from testtools.matchers import Equals, EndsWith, DirExists, Not
 
@@ -32,14 +33,43 @@ def test_initialize(self):
         self.assertThat(
             provider.provider_project_dir,
             EndsWith(
-                os.path.join("snapcraft", "projects", "stub-provider", "project-name")
+                os.path.join("snapcraft", "projects", "project-name", "stub-provider")
             ),
         )
         self.assertThat(
             provider.snap_filename,
             Equals("project-name_{}.snap".format(self.project.deb_arch)),
         )
 
+    def test_context(self):
+        with ProviderImpl(project=self.project, echoer=self.echoer_mock) as provider:
+            provider.shell()
+            fake_provider = provider
+
+        fake_provider.create_mock.assert_called_once_with("create")
+        fake_provider.destroy_mock.assert_called_once_with("destroy")
+
+    def test_context_fails_create(self):
+        create_mock = Mock()
+        destroy_mock = Mock()
+
+        class BadProviderImpl(ProviderImpl):
+            def create(self):
+                super().create()
+                create_mock("create bad")
+                raise errors.ProviderBaseError()
+
+            def destroy(self):
+                super().destroy()
+                destroy_mock("destroy bad")
+
+        with contextlib.suppress(errors.ProviderBaseError):
+            with BadProviderImpl(project=self.project, echoer=self.echoer_mock):
+                pass
+
+        create_mock.assert_called_once_with("create bad")
+        destroy_mock.assert_called_once_with("destroy bad")
+
     def test_initialize_snap_filename_with_version(self):
         self.project.info.version = "test-version"