add template_repository column to the incarnation table (#280)

.github/workflows/ci.yml

@@ -66,7 +66,7 @@ jobs:
           python -m poetry install
 
       - name: mypy
-        run: python -m mypy .
+        run: python -m mypy src tests
 
 
   unit-tests:

.gitignore

@@ -6,3 +6,4 @@ docs/build/
 .idea
 test.db
 **/.DS_Store
+.dmypy.json

alembic/versions/2023_02_27-b5550893860c_add_template_repository_to_incarnation.py

@@ -0,0 +1,30 @@
+"""add template repository to incarnation
+
+Revision ID: b5550893860c
+Revises: 0c83b17b732d
+Create Date: 2023-02-27 13:41:45.635667+00:00
+
+"""
+import sqlalchemy as sa
+
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = "b5550893860c"
+down_revision = "0c83b17b732d"
+branch_labels = None
+depends_on = None
+
+
+def upgrade() -> None:
+    with op.batch_alter_table("incarnation") as batch_op:
+        batch_op.add_column(sa.Column("template_repository", sa.String(), nullable=True))
+        batch_op.alter_column("incarnation_repository", nullable=False)
+        batch_op.alter_column("target_directory", nullable=False)
+
+
+def downgrade() -> None:
+    with op.batch_alter_table("incarnation") as batch_op:
+        batch_op.drop_column("template_repository")
+        batch_op.alter_column("incarnation_repository", nullable=True)
+        batch_op.alter_column("target_directory", nullable=True)

src/foxops/database/dal.py

@@ -1,7 +1,7 @@
 from contextlib import asynccontextmanager
 from typing import AsyncIterator
 
-from sqlalchemy import select, text
+from sqlalchemy import insert, select, update
 from sqlalchemy.exc import NoResultFound
 from sqlalchemy.ext.asyncio import AsyncConnection, AsyncEngine
 
@@ -59,48 +59,52 @@ async def create_incarnation(
             ):
                 return incarnation
 
-            query = await conn.execute(
-                text(
-                    """
-                    INSERT INTO incarnation
-                        (incarnation_repository, target_directory, commit_sha, merge_request_id)
-                    VALUES
-                        (:incarnation_repository, :target_directory, :commit_sha, :merge_request_id)
-                    RETURNING *
-                    """
-                ),
-                {
-                    "incarnation_repository": desired_incarnation_state.incarnation_repository,
-                    "target_directory": desired_incarnation_state.target_directory,
-                    "commit_sha": commit_sha,
-                    "merge_request_id": merge_request_id,
-                },
+            query = (
+                insert(incarnations)
+                .values(
+                    incarnation_repository=desired_incarnation_state.incarnation_repository,
+                    target_directory=desired_incarnation_state.target_directory,
+                    template_repository=desired_incarnation_state.template_repository,
+                    commit_sha=commit_sha,
+                    merge_request_id=merge_request_id,
+                )
+                .returning(*incarnations.columns)
             )
+            result = await conn.execute(query)
 
-            row = query.one()
+            row = result.one()
+            await conn.commit()
+            return Incarnation.from_orm(row)
+
+    async def update_incarnation_template_repository(
+        self, incarnation_id: int, template_repository: str
+    ) -> Incarnation:
+        query = (
+            update(incarnations)
+            .where(incarnations.c.id == incarnation_id)
+            .values(template_repository=template_repository)
+            .returning(*incarnations.columns)
+        )
+        async with self.connection() as conn:
+            result = await conn.execute(query)
+
+            row = result.one()
             await conn.commit()
             return Incarnation.from_orm(row)
 
     async def update_incarnation(
         self, id: int, commit_sha: GitSha, merge_request_id: MergeRequestId | None
     ) -> Incarnation:
+        query = (
+            update(incarnations)
+            .where(incarnations.c.id == id)
+            .values(commit_sha=commit_sha, merge_request_id=merge_request_id)
+            .returning(*incarnations.columns)
+        )
         async with self.connection() as conn:
-            query = await conn.execute(
-                text(
-                    """
-                    UPDATE incarnation
-                    SET
-                        commit_sha = :commit_sha,
-                        merge_request_id = :merge_request_id
-                    WHERE
-                        id = :id
-                    RETURNING *
-                    """
-                ),
-                {"id": id, "commit_sha": commit_sha, "merge_request_id": merge_request_id},
-            )
+            result = await conn.execute(query)
 
-            row = query.one()
+            row = result.one()
             await conn.commit()
             return Incarnation.from_orm(row)
 

src/foxops/database/repositories/change.py

@@ -112,6 +112,7 @@ async def create_incarnation_with_first_change(
         self,
         incarnation_repository: str,
         target_directory: str,
+        template_repository: str,
         commit_sha: str,
         requested_version_hash: str,
         requested_version: str,
@@ -123,6 +124,7 @@ async def create_incarnation_with_first_change(
                 .values(
                     incarnation_repository=incarnation_repository,
                     target_directory=target_directory,
+                    template_repository=template_repository,
                     commit_sha=commit_sha,
                 )
                 .returning(incarnations.c.id)

src/foxops/database/schema.py

@@ -17,8 +17,10 @@
     "incarnation",
     meta,
     Column("id", Integer, primary_key=True),
-    Column("incarnation_repository", String),
-    Column("target_directory", String),
+    Column("incarnation_repository", String, nullable=False),
+    Column("target_directory", String, nullable=False),
+    # template_repository: to be changed to nullable=False, once all data has been migrated
+    Column("template_repository", String, nullable=True),
     Column("commit_sha", String),
     Column("merge_request_id", String, nullable=True),
     UniqueConstraint("incarnation_repository", "target_directory", name="incarnation_identity"),

src/foxops/hosters/gitlab/gitlab.py

@@ -415,6 +415,12 @@ async def get_merge_request_status(self, incarnation_repository: str, merge_requ
         response = await self.client.get(
             f"/projects/{quote_plus(incarnation_repository)}/merge_requests/{merge_request_id}"
         )
+        if response.status_code == 404:
+            if not await self.__project_exists(incarnation_repository):
+                raise IncarnationRepositoryNotFound(incarnation_repository)
+
+            # if the merge request does not exist, we assume it has been closed (because it was deleted)
+            return MergeRequestStatus.CLOSED
         response.raise_for_status()
 
         merge_request: MergeRequest = response.json()

src/foxops/models/incarnation.py

@@ -13,6 +13,7 @@ class Incarnation(BaseModel):
     id: int
     incarnation_repository: str
     target_directory: str
+    template_repository: str | None
     commit_sha: str
     merge_request_id: str | None
 

src/foxops/routers/incarnations.py

@@ -79,6 +79,21 @@ async def list_incarnations(
     return [await change_service.get_incarnation_basic(i) for i in incarnation_ids]
 
 
+@router.post("/upgrade-all")
+async def upgrade_all_incarnations(
+    delete_nonexisting: bool = False,
+    change_service: ChangeService = Depends(get_change_service),
+):
+    failed_upgrades, successful_upgrades, deleted_incarnations = await change_service.upgrade_all_incarnations(
+        delete_nonexisting=delete_nonexisting,
+    )
+    return {
+        "failed_upgrades": failed_upgrades,
+        "successful_upgrades": successful_upgrades,
+        "deleted_incarnations": deleted_incarnations,
+    }
+
+
 @router.post(
     "/legacy",
     responses={

src/foxops/services/change.py

@@ -15,6 +15,7 @@
 )
 from foxops.engine import TemplateData
 from foxops.engine.patching.git_diff_patch import PatchResult
+from foxops.errors import IncarnationRepositoryNotFound
 from foxops.external.git import GitError, GitRepository
 from foxops.hosters import Hoster
 from foxops.hosters.types import MergeRequestStatus
@@ -32,6 +33,10 @@ class IncarnationAlreadyUpgraded(Exception):
     pass
 
 
+class IncarnationUpgradeFailedAsNoIncarnationStateExists(Exception):
+    pass
+
+
 class ChangeRejectedDueToNoChanges(Exception):
     pass
 
@@ -115,6 +120,7 @@ async def create_incarnation(
             change = await self._change_repository.create_incarnation_with_first_change(
                 incarnation_repository=incarnation_repository,
                 target_directory=target_directory,
+                template_repository=template_repository,
                 commit_sha=commit_sha,
                 requested_version_hash=incarnation_state.template_repository_version_hash,
                 requested_version=template_repository_version,
@@ -158,7 +164,7 @@ async def initialize_legacy_incarnation(self, incarnation_id: int) -> Change:
             mr_status = await self._hoster.get_merge_request_status(
                 incarnation.incarnation_repository, incarnation.merge_request_id
             )
-            if mr_status != MergeRequestStatus.MERGED:
+            if mr_status not in (MergeRequestStatus.MERGED, MergeRequestStatus.CLOSED):
                 raise ChangeFailed(
                     f"Cannot initialize legacy incarnation {incarnation_id} because it has "
                     f"a pending merge request: {incarnation.merge_request_id}. "
@@ -170,12 +176,16 @@ async def initialize_legacy_incarnation(self, incarnation_id: int) -> Change:
             incarnation.incarnation_repository, incarnation.target_directory
         )
         if get_incarnation_state_result is None:
-            raise ChangeFailed(
+            raise IncarnationUpgradeFailedAsNoIncarnationStateExists(
                 f"Cannot initialize legacy incarnation {incarnation_id} because it does not have a .fengine.yaml file. "
-                f"This is NOT expected at this stage. Please investigate."
+                f"This could happen if it is a subdirectory incarnation, where the subdirectory was already deleted."
             )
         commit_sha, incarnation_state = get_incarnation_state_result
 
+        await self._incarnation_repository.update_incarnation_template_repository(
+            incarnation_id, incarnation_state.template_repository
+        )
+
         change_in_db = await self._change_repository.create_change(
             incarnation_id=incarnation_id,
             revision=1,
@@ -189,6 +199,43 @@ async def initialize_legacy_incarnation(self, incarnation_id: int) -> Change:
 
         return await self.get_change(change_in_db.id)
 
+    async def upgrade_all_incarnations(self, delete_nonexisting: bool = False):
+        failed_upgrades = []
+        successful_upgrades = []
+        deleted_incarnations = []
+
+        async for incarnation in self._incarnation_repository.get_incarnations():
+            try:
+                await self.initialize_legacy_incarnation(incarnation.id)
+            except IncarnationAlreadyUpgraded:
+                continue
+            except (IncarnationRepositoryNotFound, IncarnationUpgradeFailedAsNoIncarnationStateExists):
+                if delete_nonexisting:
+                    await self._incarnation_repository.delete_incarnation(incarnation.id)
+                    deleted_incarnations.append(
+                        (incarnation.id, incarnation.incarnation_repository, incarnation.target_directory)
+                    )
+                else:
+                    failed_upgrades.append(
+                        (
+                            incarnation.id,
+                            incarnation.incarnation_repository,
+                            incarnation.target_directory,
+                            "repository not found",
+                        )
+                    )
+            except Exception as e:
+                self._log.exception("Failed to upgrade incarnation", incarnation_id=incarnation.id)
+                failed_upgrades.append(
+                    (incarnation.id, incarnation.incarnation_repository, incarnation.target_directory, str(e))
+                )
+            else:
+                successful_upgrades.append(
+                    (incarnation.id, incarnation.incarnation_repository, incarnation.target_directory)
+                )
+
+        return failed_upgrades, successful_upgrades, deleted_incarnations
+
     async def create_change_direct(
         self, incarnation_id: int, requested_version: str | None = None, requested_data: TemplateData | None = None
     ) -> Change:
@@ -445,6 +492,10 @@ async def _prepared_change_environment(
         incarnation = await self._incarnation_repository.get_incarnation(incarnation_id)
         last_change_id = await self.get_latest_change_id_for_incarnation(incarnation_id)
 
+        await self._upgrade_incarnation_if_possible(incarnation_id)
+        if incarnation.template_repository is None:
+            raise Exception("upgrade failed. Should not happen.")
+
         # if the previous change was of type merge request and is still open, we dont want to continue
         last_change: ChangeWithMergeRequest | Change
         match await self.get_change_type(last_change_id):
@@ -467,18 +518,9 @@ async def _prepared_change_environment(
 
         incarnation_repo_metadata = await self._hoster.get_repository_metadata(incarnation.incarnation_repository)
 
-        # Fetch the template repository
-        # NOTE (ahg, 01/2023): Ideally, in the future we can just read this from the DB
-        async with self._hoster.cloned_repository(incarnation.incarnation_repository) as local_incarnation_repository:
-            incarnation_state = fengine.load_incarnation_state(
-                local_incarnation_repository.directory / incarnation.target_directory / ".fengine.yaml"
-            )
-
         async with (
             self._hoster.cloned_repository(incarnation.incarnation_repository) as local_incarnation_repository,
-            self._hoster.cloned_repository(
-                incarnation_state.template_repository, bare=True
-            ) as local_template_repository,
+            self._hoster.cloned_repository(incarnation.template_repository, bare=True) as local_template_repository,
         ):
             branch_name = generate_foxops_branch_name(
                 prefix="update-to",

tests/database/test_change_repository.py

@@ -174,6 +174,7 @@ async def test_create_incarnation_with_first_change(change_repository: ChangeRep
     change = await change_repository.create_incarnation_with_first_change(
         incarnation_repository="incarnation",
         target_directory=".",
+        template_repository="template",
         commit_sha="commit_sha",
         requested_version_hash="dummy template sha",
         requested_version="v1",
@@ -196,6 +197,7 @@ async def test_delete_incarnation_also_deletes_associated_changes(change_reposit
     change = await change_repository.create_incarnation_with_first_change(
         incarnation_repository="incarnation",
         target_directory=".",
+        template_repository="template",
         commit_sha="commit_sha",
         requested_version_hash="dummy template sha",
         requested_version="v1",

tests/e2e/conftest.py

@@ -3,6 +3,7 @@
 import uuid
 from urllib.parse import quote_plus
 
+import httpx
 import pytest
 from httpx import AsyncClient, Client, Timeout
 
@@ -73,6 +74,31 @@ async def create_test_gitlab_client(gitlab_test_address: str, gitlab_test_user_t
     )
 
 
+@pytest.fixture(scope="function")
+async def gitlab_project_factory(gitlab_test_client: AsyncClient):
+    async def _factory(name: str):
+        response = await gitlab_test_client.post("/projects", json={"name": name})
+        response.raise_for_status()
+        project = response.json()
+
+        created_project_ids.append(project["id"])
+
+        return project
+
+    created_project_ids: list[int] = []
+
+    yield _factory
+
+    # cleanup all projects that were created during the test, ignoring those that were already remove in the test
+    for project_id in created_project_ids:
+        response = await gitlab_test_client.delete(f"/projects/{project_id}")
+        try:
+            response.raise_for_status()
+        except httpx.HTTPStatusError as e:
+            if e.response.status_code != 404:
+                raise
+
+
 @pytest.fixture(name="empty_incarnation_gitlab_repository")
 async def create_empty_incarnation_gitlab_repository(gitlab_test_client: AsyncClient):
     response = await gitlab_test_client.post("/projects", json={"name": f"incarnation-{str(uuid.uuid4())}"})