Clean up environments that couldn't be scheduled (#41)

* Don't return unused ID in GraphQL API

* Tidy up environments that could not even be scheduled with the builder

* Add test against race conditions in commit_and_push

* Update apt package lists in GHA workflows

Fixes:

Ign:10 http://azure.archive.ubuntu.com/ubuntu focal-updates/main amd64 libldap2-dev amd64 2.4.49+dfsg-2ubuntu1.9
Ign:10 http://archive.ubuntu.com/ubuntu focal-updates/main amd64 libldap2-dev amd64 2.4.49+dfsg-2ubuntu1.9
Ign:10 http://security.ubuntu.com/ubuntu focal-updates/main amd64 libldap2-dev amd64 2.4.49+dfsg-2ubuntu1.9
Err:10 mirror+file:/etc/apt/apt-mirrors.txt focal-updates/main amd64 libldap2-dev amd64 2.4.49+dfsg-2ubuntu1.9
  404  Not Found [IP: 52.252.163.49 80]
E: Failed to fetch mirror+file:/etc/apt/apt-mirrors.txt/pool/main/o/openldap/libldap2-dev_2.4.49+dfsg-2ubuntu1.9_amd64.deb  404  Not Found [IP: 52.252.163.49 80]
E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing?

.github/workflows/dev.yml

@@ -49,6 +49,7 @@ jobs:
 
       - name: Install system libraries
         run: |
+          sudo apt-get update
           sudo apt-get install -y libldap2-dev libsasl2-dev libssl-dev python-dev
 
       - name: Install dependencies

.github/workflows/preview.yml

@@ -28,6 +28,7 @@ jobs:
 
       - name: Install system libraries
         run: |
+          sudo apt-get update
           sudo apt-get install -y libldap2-dev libsasl2-dev libssl-dev python-dev
 
       - name: Install dependencies

.github/workflows/release.yml

@@ -41,6 +41,7 @@ jobs:
 
       - name: Install system libraries
         run: |
+          sudo apt-get update
           sudo apt-get install -y libldap2-dev libsasl2-dev libssl-dev python-dev
 
       - name: Install dependencies

softpack_core/artifacts.py

@@ -335,6 +335,9 @@ def commit_and_push(
             tree_oid: the oid of the tree object that will be committed. The
             tree this refers to will replace the entire contents of the repo.
             message: the commit message
+
+        Returns:
+            pygit2.Oid: the ID of the new commit
         """
         ref = self.repo.head.name
         parents = [self.repo.lookup_reference(ref).target]

softpack_core/schemas/environment.py

@@ -59,8 +59,6 @@ class DeleteEnvironmentSuccess(Success):
 class WriteArtifactSuccess(Success):
     """Artifact successfully created."""
 
-    commit_oid: str
-
 
 # Error types
 @strawberry.type
@@ -253,11 +251,11 @@ def create(cls, env: EnvironmentInput) -> CreateResponse:  # type: ignore
         if input_err is not None:
             return input_err
 
-        name = env.name
+        versionless_name = env.name
         version = 1
 
         while True:
-            env.name = name + "-" + str(version)
+            env.name = versionless_name + "-" + str(version)
             response = cls.create_new_env(
                 env, Artifacts.built_by_softpack_file
             )
@@ -269,16 +267,14 @@ def create(cls, env: EnvironmentInput) -> CreateResponse:  # type: ignore
 
             version += 1
 
-        env.name = name
-
         # Send build request
         try:
             host = app.settings.builder.host
             port = app.settings.builder.port
             r = httpx.post(
                 f"http://{host}:{port}/environments/build",
                 json={
-                    "name": f"{env.path}/{env.name}",
+                    "name": f"{env.path}/{versionless_name}",
                     "version": str(version),
                     "model": {
                         "description": env.description,
@@ -294,6 +290,7 @@ def create(cls, env: EnvironmentInput) -> CreateResponse:  # type: ignore
             )
             r.raise_for_status()
         except Exception as e:
+            cls.delete(env.name, env.path)
             return BuilderError(
                 message="Connection to builder failed: "
                 + "".join(format_exception_only(type(e), e))
@@ -575,12 +572,9 @@ async def write_artifacts(
             tree_oid = cls.artifacts.create_files(
                 Path(folder_path), new_files, overwrite=True
             )
-            commit_oid = cls.artifacts.commit_and_push(
-                tree_oid, "write artifact"
-            )
+            cls.artifacts.commit_and_push(tree_oid, "write artifact")
             return WriteArtifactSuccess(
                 message="Successfully written artifact(s)",
-                commit_oid=str(commit_oid),
             )
 
         except Exception as e:

softpack_core/schemas/package_collection.py

@@ -6,7 +6,6 @@
 
 from dataclasses import dataclass
 from typing import Iterable
-from uuid import UUID
 
 import strawberry
 
@@ -23,7 +22,6 @@ class PackageMultiVersion(Package):
 class PackageCollection:
     """A Strawberry model representing a package collection."""
 
-    id: UUID
     name: str
     packages: list[PackageMultiVersion]
 

tests/integration/test_artifacts.py

@@ -6,6 +6,7 @@
 
 import os
 import shutil
+import threading
 from pathlib import Path
 
 import pygit2
@@ -226,3 +227,36 @@ def test_iter() -> None:
     assert pkgs[1].version == "v2.0.1"
     assert pkgs[2].name == "pck3"
     assert pkgs[2].version is None
+
+
+# pygit2 protects us against producing diverging histories anyway:
+#   _pygit2.GitError: failed to create commit: current tip is not the first
+#   parent
+# but the test is nice reassurance.
+def test_simultaneous_commit():
+    parallelism = 100
+    ad = new_test_artifacts()
+    artifacts: Artifacts = ad["artifacts"]
+    initial_commit_oid = ad["initial_commit_oid"]
+
+    new_tree, _ = add_test_file_to_repo(artifacts)
+
+    e = threading.Event()
+
+    def fn(i: int):
+        e.wait()
+        artifacts.commit_and_push(new_tree, f"I am thread {i}")
+
+    threads = [
+        threading.Thread(target=fn, args=[i]) for i in range(parallelism)
+    ]
+    for thread in threads:
+        thread.start()
+    e.set()
+    for thread in threads:
+        thread.join()
+
+    commit = artifacts.repo.head.peel(pygit2.Commit)
+    for _ in range(parallelism):
+        commit = commit.parents[0]
+    assert commit.oid == initial_commit_oid

tests/integration/test_environment.py

@@ -15,6 +15,7 @@
 
 from softpack_core.artifacts import Artifacts, app
 from softpack_core.schemas.environment import (
+    BuilderError,
     CreateEnvironmentSuccess,
     DeleteEnvironmentSuccess,
     Environment,
@@ -126,6 +127,24 @@ def test_create_path_invalid_disallowed(httpx_post, testable_env_input):
     assert isinstance(result, InvalidInputError)
 
 
+def test_create_cleans_up_after_builder_failure(
+    httpx_post, testable_env_input
+):
+    httpx_post.side_effect = Exception('could not contact builder')
+    result = Environment.create(testable_env_input)
+    assert isinstance(result, BuilderError)
+
+    dir = Path(
+        Environment.artifacts.environments_root,
+        testable_env_input.path,
+        testable_env_input.name + "-1",
+    )
+    builtPath = dir / Environment.artifacts.built_by_softpack_file
+    ymlPath = dir / Environment.artifacts.environments_file
+    assert not file_in_remote(builtPath)
+    assert not file_in_remote(ymlPath)
+
+
 def builder_called_correctly(
     post_mock, testable_env_input: EnvironmentInput
 ) -> None: