fix(remote): don't crash if the repo exists (#294)

craft_application/services/remotebuild.py

@@ -16,6 +16,7 @@
 """Service class for remote build commands."""
 from __future__ import annotations
 
+import contextlib
 import datetime
 import itertools
 import os
@@ -27,6 +28,7 @@
 from urllib import parse
 
 import craft_cli
+import launchpadlib.errors  # type: ignore[import-untyped]
 import platformdirs
 
 from craft_application import errors, launchpad, models
@@ -117,8 +119,8 @@ def start_builds(
 
         self._name = utils.get_build_id(self._app.name, project.name, project_dir)
         self._lp_project = self._ensure_project()
-        _, self._repository = self._new_repository(project_dir)
-        self._recipe = self._new_recipe(
+        _, self._repository = self._ensure_repository(project_dir)
+        self._recipe = self._ensure_recipe(
             self._name, self._repository, architectures=architectures
         )
         self._check_timeout()
@@ -249,15 +251,18 @@ def _ensure_project(self) -> launchpad.models.Project:
                 summary=f"Automatically-generated project for housing {self.lp.username}'s remote builds in snapcraft, charmcraft, etc.",
             )
 
-    def _new_repository(
+    def _ensure_repository(
         self, project_dir: pathlib.Path
     ) -> tuple[WorkTree, launchpad.models.GitRepository]:
-        """Create a repository on the local machine and on Launchpad."""
+        """Create a repository on the local machine and ensure it's on Launchpad."""
         work_tree = WorkTree(self._app.name, self._name, project_dir)
         work_tree.init_repo()
-        lp_repository = launchpad.models.GitRepository.new(
-            self.lp, self._name, target=self._lp_project.name
-        )
+        try:
+            lp_repository = self.lp.new_repository(self._name, self._lp_project.name)
+        except launchpadlib.errors.HTTPError:
+            lp_repository = self.lp.get_repository(
+                name=self._name, project=self._lp_project.name
+            )
 
         token = lp_repository.get_access_token(
             f"{self._app.name} {self._app.version} remote build",
@@ -276,17 +281,27 @@ def _new_repository(
 
     def _get_repository(self) -> launchpad.models.GitRepository:
         """Get an existing repository on Launchpad."""
-        return launchpad.models.GitRepository.get(
-            self.lp, name=self._name, owner=self.lp.username
-        )
+        return self.lp.get_repository(name=self._name, owner=self.lp.username)
 
-    def _new_recipe(
+    def _ensure_recipe(
         self,
         name: str,
         repository: launchpad.models.GitRepository,
         **kwargs: Any,  # noqa: ANN401
     ) -> launchpad.models.Recipe:
         """Create a new recipe."""
+        # There are 3 different ways this can fail in expected ways, all of them HTTPErrors:
+        #
+        # 1. Recipe doesn't exist (or you don't have access to see it)
+        # 2. You don't have access to delete the recipe
+        # 3. There is an ongoing build with the recipep
+        #
+        # In case 1, we don't care and are about to make the new recipe anyway.
+        # In the other cases, the errors from `new()` end up being more useful.
+        with contextlib.suppress(launchpadlib.errors.HTTPError):
+            recipe = self._get_recipe()
+            recipe.delete()
+
         return self.RecipeClass.new(
             self.lp, name, self.lp.username, git_ref=repository.git_https_url, **kwargs
         )

tests/unit/services/conftest.py

@@ -94,6 +94,16 @@ def fake_launchpad(app_metadata, mock_git_repository, mock_project_entry):
                     ),
                 )
             ),
+            getByName=get_mock_callable(
+                return_value=get_mock_lazr_entry(
+                    "snap",
+                    requestBuilds=get_mock_callable(
+                        return_value=get_mock_lazr_collection(
+                            [], status="Completed", builds=[]
+                        )
+                    ),
+                ),
+            ),
         ),
     )
     return launchpad.Launchpad(app_metadata.name, lp)

tests/unit/services/test_remotebuild.py

@@ -18,6 +18,7 @@
 import pathlib
 from unittest import mock
 
+import launchpadlib.errors
 import lazr.restfulclient.errors
 import lazr.restfulclient.resource
 import pytest
@@ -115,7 +116,7 @@ def test_is_project_private_error(remote_build_service):
         remote_build_service.is_project_private()
 
 
-def test_new_repository(
+def test_ensure_repository_creation(
     monkeypatch, tmp_path, remote_build_service, mock_lp_project, mock_push_url
 ):
     wrapped_repository = mock.Mock(
@@ -129,7 +130,45 @@ def test_new_repository(
     sentinel.write_text("I am a sentinel file.")
     remote_build_service._lp_project = mock_lp_project
 
-    work_tree, lp_repository = remote_build_service._new_repository(tmp_path)
+    work_tree, lp_repository = remote_build_service._ensure_repository(tmp_path)
+
+    assert (work_tree.repo_dir / "sentinel_file").read_text() == sentinel.read_text()
+    mock_push_url.assert_called_once_with(
+        "https://craft_test_user:[email protected]/~user/+git/my_repo",
+        "main",
+    )
+    expiry = wrapped_repository.get_access_token.call_args.kwargs["expiry"]
+
+    # Ensure that we're getting a timezone-aware object in the method.
+    # This is here as a regression test because if you're in a timezone behind UTC
+    # the expiry time sent to Launchpad will have already expired and Launchpad
+    # does not catch that. So instead we make sure thot even when using the easternmost
+    # time zone the expiry time is still in the future.
+    tz = datetime.timezone(datetime.timedelta(hours=14))
+    assert expiry > datetime.datetime.now(tz=tz)
+
+
+def test_ensure_repository_already_exists(
+    monkeypatch, tmp_path, remote_build_service, mock_lp_project, mock_push_url
+):
+    monkeypatch.setattr(
+        launchpad.models.GitRepository,
+        "new",
+        mock.Mock(side_effect=launchpadlib.errors.Conflict("nope", "broken")),
+    )
+    wrapped_repository = mock.Mock(
+        spec=launchpad.models.GitRepository,
+        git_https_url="https://git.launchpad.net/~user/+git/my_repo",
+        **{"get_access_token.return_value": "super_secret_token"},
+    )
+    repository_get = mock.Mock(return_value=wrapped_repository)
+    monkeypatch.setattr(launchpad.models.GitRepository, "get", repository_get)
+    sentinel = tmp_path / "sentinel_file"
+    sentinel.write_text("I am a sentinel file.")
+    remote_build_service._lp_project = mock_lp_project
+    remote_build_service._name = "some-repo-name"
+
+    work_tree, lp_repository = remote_build_service._ensure_repository(tmp_path)
 
     assert (work_tree.repo_dir / "sentinel_file").read_text() == sentinel.read_text()
     mock_push_url.assert_called_once_with(