From c6687ced936c3f1fe56af3b1d2b8f39f371e3eff Mon Sep 17 00:00:00 2001 From: ash Date: Thu, 1 Feb 2024 10:03:14 +0000 Subject: [PATCH 1/4] Don't return unused ID in GraphQL API --- softpack_core/artifacts.py | 3 +++ softpack_core/schemas/environment.py | 8 ++------ softpack_core/schemas/package_collection.py | 2 -- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/softpack_core/artifacts.py b/softpack_core/artifacts.py index cfd9029..46e33ac 100644 --- a/softpack_core/artifacts.py +++ b/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] diff --git a/softpack_core/schemas/environment.py b/softpack_core/schemas/environment.py index 9bf55bc..0cc7272 100644 --- a/softpack_core/schemas/environment.py +++ b/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 @@ -294,6 +292,7 @@ def create(cls, env: EnvironmentInput) -> CreateResponse: # type: ignore ) r.raise_for_status() except Exception as e: + # TODO: clean up scheduled build in repo return BuilderError( message="Connection to builder failed: " + "".join(format_exception_only(type(e), e)) @@ -575,12 +574,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: diff --git a/softpack_core/schemas/package_collection.py b/softpack_core/schemas/package_collection.py index cf4521a..6b0a3f2 100644 --- a/softpack_core/schemas/package_collection.py +++ b/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] From 26e7aede495018e99074401d14e32dbff858042d Mon Sep 17 00:00:00 2001 From: ash Date: Thu, 1 Feb 2024 10:39:24 +0000 Subject: [PATCH 2/4] Tidy up environments that could not even be scheduled with the builder --- softpack_core/schemas/environment.py | 10 ++++------ tests/integration/test_environment.py | 17 +++++++++++++++++ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/softpack_core/schemas/environment.py b/softpack_core/schemas/environment.py index 0cc7272..5e8b20d 100644 --- a/softpack_core/schemas/environment.py +++ b/softpack_core/schemas/environment.py @@ -251,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 ) @@ -267,8 +267,6 @@ def create(cls, env: EnvironmentInput) -> CreateResponse: # type: ignore version += 1 - env.name = name - # Send build request try: host = app.settings.builder.host @@ -276,7 +274,7 @@ def create(cls, env: EnvironmentInput) -> CreateResponse: # type: ignore 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, @@ -292,7 +290,7 @@ def create(cls, env: EnvironmentInput) -> CreateResponse: # type: ignore ) r.raise_for_status() except Exception as e: - # TODO: clean up scheduled build in repo + cls.delete(env.name, env.path) return BuilderError( message="Connection to builder failed: " + "".join(format_exception_only(type(e), e)) diff --git a/tests/integration/test_environment.py b/tests/integration/test_environment.py index 78b1f57..df068f3 100644 --- a/tests/integration/test_environment.py +++ b/tests/integration/test_environment.py @@ -26,6 +26,7 @@ State, UpdateEnvironmentSuccess, WriteArtifactSuccess, + BuilderError, ) from tests.integration.utils import file_in_remote @@ -126,6 +127,22 @@ 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: From f7db003ca9209fd8e9c8dcc4710d0e580162fd90 Mon Sep 17 00:00:00 2001 From: ash Date: Thu, 1 Feb 2024 11:27:49 +0000 Subject: [PATCH 3/4] Add test against race conditions in commit_and_push --- tests/integration/test_artifacts.py | 34 +++++++++++++++++++++++++++ tests/integration/test_environment.py | 6 +++-- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_artifacts.py b/tests/integration/test_artifacts.py index 147b940..462fdb6 100644 --- a/tests/integration/test_artifacts.py +++ b/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 diff --git a/tests/integration/test_environment.py b/tests/integration/test_environment.py index df068f3..34153a2 100644 --- a/tests/integration/test_environment.py +++ b/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, @@ -26,7 +27,6 @@ State, UpdateEnvironmentSuccess, WriteArtifactSuccess, - BuilderError, ) from tests.integration.utils import file_in_remote @@ -127,7 +127,9 @@ 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): +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) From 7c453ceceb503d7218ee300b245d299c5782ec45 Mon Sep 17 00:00:00 2001 From: ash Date: Thu, 1 Feb 2024 11:48:37 +0000 Subject: [PATCH 4/4] 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 | 1 + .github/workflows/preview.yml | 1 + .github/workflows/release.yml | 1 + 3 files changed, 3 insertions(+) diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 303efb5..27a51eb 100644 --- a/.github/workflows/dev.yml +++ b/.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 diff --git a/.github/workflows/preview.yml b/.github/workflows/preview.yml index 383320d..c1609d0 100644 --- a/.github/workflows/preview.yml +++ b/.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 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 7f0c639..517cb5c 100644 --- a/.github/workflows/release.yml +++ b/.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