Remove destructive Docker cache cleanup
This commit is contained in:
parent
c7b4abc347
commit
d2ad4f7c96
2 changed files with 63 additions and 130 deletions
|
|
@ -1741,36 +1741,6 @@ def _local_image_reference_config_id(
|
|||
return config_id
|
||||
|
||||
|
||||
def _remove_new_image_reference(
|
||||
docker_binary: Path,
|
||||
docker_host: str,
|
||||
docker_config: Path,
|
||||
image_reference: str,
|
||||
expected_config_id: str,
|
||||
) -> None:
|
||||
observed_config_id = _local_image_reference_config_id(
|
||||
docker_binary,
|
||||
docker_host,
|
||||
docker_config,
|
||||
image_reference,
|
||||
)
|
||||
if observed_config_id is None:
|
||||
return
|
||||
_require(observed_config_id == expected_config_id, "new registry image reference changed before cleanup")
|
||||
_run_docker(
|
||||
docker_binary,
|
||||
docker_host,
|
||||
docker_config,
|
||||
["image", "rm", image_reference],
|
||||
check=True,
|
||||
timeout=120,
|
||||
)
|
||||
_require(
|
||||
_local_image_reference_config_id(docker_binary, docker_host, docker_config, image_reference) is None,
|
||||
"new registry image reference cleanup did not pass",
|
||||
)
|
||||
|
||||
|
||||
def inspect_registry_image(
|
||||
image_input: dict[str, Any],
|
||||
image_reference: str,
|
||||
|
|
@ -1781,7 +1751,6 @@ def inspect_registry_image(
|
|||
docker_binary: Path,
|
||||
docker_host: str,
|
||||
docker_config: Path,
|
||||
cleanup_new_reference: bool = False,
|
||||
) -> dict[str, Any]:
|
||||
"""Pull and inspect one immutable Artifact Registry image without starting it."""
|
||||
|
||||
|
|
@ -1807,18 +1776,8 @@ def inspect_registry_image(
|
|||
"image input differs from the expected reviewed input SHA-256",
|
||||
)
|
||||
_validate_docker_authority(docker_binary, docker_host, docker_config)
|
||||
prestate_observed = False
|
||||
pulled_config_id: str | None = None
|
||||
inspection: dict[str, Any] | None = None
|
||||
primary_error: BaseException | None = None
|
||||
with _DockerCacheLock(docker_config):
|
||||
try:
|
||||
if cleanup_new_reference:
|
||||
_require(
|
||||
_local_image_reference_config_id(docker_binary, docker_host, docker_config, image_reference) is None,
|
||||
"exact registry image reference must be absent before inspection cleanup",
|
||||
)
|
||||
prestate_observed = True
|
||||
_run_docker(
|
||||
docker_binary,
|
||||
docker_host,
|
||||
|
|
@ -1851,30 +1810,7 @@ def inspect_registry_image(
|
|||
"inspected image differs from the expected reviewed config digest",
|
||||
)
|
||||
except (OSError, subprocess.SubprocessError, PackageError) as exc:
|
||||
primary_error = exc
|
||||
finally:
|
||||
try:
|
||||
if cleanup_new_reference and prestate_observed:
|
||||
if pulled_config_id is None:
|
||||
pulled_config_id = _local_image_reference_config_id(
|
||||
docker_binary,
|
||||
docker_host,
|
||||
docker_config,
|
||||
image_reference,
|
||||
)
|
||||
if pulled_config_id is not None:
|
||||
_remove_new_image_reference(
|
||||
docker_binary,
|
||||
docker_host,
|
||||
docker_config,
|
||||
image_reference,
|
||||
pulled_config_id,
|
||||
)
|
||||
except (OSError, subprocess.SubprocessError, PackageError) as cleanup_error:
|
||||
raise PackageError("immutable registry image cleanup failed") from cleanup_error
|
||||
if primary_error is not None:
|
||||
raise PackageError("immutable registry image inspection failed") from primary_error
|
||||
_require(inspection is not None, "immutable registry image inspection failed")
|
||||
raise PackageError("immutable registry image inspection failed") from exc
|
||||
return inspection
|
||||
|
||||
|
||||
|
|
@ -2392,7 +2328,6 @@ def main(argv: list[str] | None = None) -> int:
|
|||
docker_binary=args.docker_binary,
|
||||
docker_host=args.docker_host,
|
||||
docker_config=args.docker_config,
|
||||
cleanup_new_reference=False,
|
||||
)
|
||||
result = build_release_descriptor(image_input, inspection)
|
||||
publish_finalize_bundle(args.output_bundle, result)
|
||||
|
|
|
|||
|
|
@ -1064,7 +1064,7 @@ def test_local_image_inventory_rejects_inventory_and_inspection_disagreement(
|
|||
|
||||
|
||||
@pytest.mark.parametrize("config_digest", ["sha256:" + "3" * 64, "sha256:" + "a" * 64])
|
||||
def test_registry_inspection_uses_explicit_local_authority_and_removes_new_reference(
|
||||
def test_registry_inspection_absent_pull_dependent_retains_new_reference(
|
||||
tmp_path: Path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
config_digest: str,
|
||||
|
|
@ -1075,7 +1075,7 @@ def test_registry_inspection_uses_explicit_local_authority_and_removes_new_refer
|
|||
prefix = [str(docker_binary), "--host", docker_host, "--config", str(docker_config)]
|
||||
calls: list[list[str]] = []
|
||||
environments: list[dict[str, str]] = []
|
||||
state = {"present": False}
|
||||
state = {"present": False, "dependent": False, "inspection_calls": 0}
|
||||
payload = json.dumps(
|
||||
[docker_inspection_fixture(image_input, reference, config_digest=config_digest)]
|
||||
).encode()
|
||||
|
|
@ -1099,6 +1099,9 @@ def test_registry_inspection_uses_explicit_local_authority_and_removes_new_refer
|
|||
stderr=b"",
|
||||
)
|
||||
if operation[:2] == ["image", "inspect"]:
|
||||
state["inspection_calls"] += 1
|
||||
if state["inspection_calls"] == 2:
|
||||
state["dependent"] = True
|
||||
return subprocess.CompletedProcess(
|
||||
arguments,
|
||||
0,
|
||||
|
|
@ -1109,9 +1112,7 @@ def test_registry_inspection_uses_explicit_local_authority_and_removes_new_refer
|
|||
state["present"] = True
|
||||
return subprocess.CompletedProcess(arguments, 0, stdout=b"pulled\n", stderr=b"")
|
||||
if operation[:2] == ["image", "rm"]:
|
||||
assert "--force" not in operation
|
||||
state["present"] = False
|
||||
return subprocess.CompletedProcess(arguments, 0, stdout=b"removed\n", stderr=b"")
|
||||
raise AssertionError("digest reference must never be removed")
|
||||
raise AssertionError(operation)
|
||||
|
||||
monkeypatch.setattr(package.subprocess, "run", fake_run)
|
||||
|
|
@ -1125,19 +1126,18 @@ def test_registry_inspection_uses_explicit_local_authority_and_removes_new_refer
|
|||
docker_binary=docker_binary,
|
||||
docker_host=docker_host,
|
||||
docker_config=docker_config,
|
||||
cleanup_new_reference=True,
|
||||
)
|
||||
|
||||
assert inspection["status"] == "pass"
|
||||
assert state["present"] is False
|
||||
assert state == {"present": True, "dependent": True, "inspection_calls": 2}
|
||||
assert [call[5:] for call in calls if call[5] == "pull"] == [
|
||||
["pull", "--quiet", "--platform", package.PLATFORM, reference]
|
||||
]
|
||||
assert all(not any(name.startswith("DOCKER_") for name in environment) for environment in environments)
|
||||
assert [call[5:] for call in calls if call[5:7] == ["image", "rm"]] == [["image", "rm", reference]]
|
||||
assert not any(call[5:7] == ["image", "rm"] for call in calls)
|
||||
|
||||
|
||||
def test_registry_inspection_default_retains_preexisting_reference(
|
||||
def test_registry_inspection_retains_preexisting_reference(
|
||||
tmp_path: Path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
|
|
@ -1145,7 +1145,7 @@ def test_registry_inspection_default_retains_preexisting_reference(
|
|||
reference = f"{package.IMAGE_REPOSITORY}@sha256:{IMAGE_DIGEST}"
|
||||
docker_binary, docker_host, docker_config = docker_authority(tmp_path)
|
||||
payload = json.dumps([docker_inspection_fixture(image_input, reference)]).encode()
|
||||
state = {"present": True, "pulls": 0, "removed": False}
|
||||
state = {"present": True, "pulls": 0}
|
||||
|
||||
def fake_run(arguments: list[str], **_kwargs):
|
||||
operation = arguments[5:]
|
||||
|
|
@ -1162,8 +1162,7 @@ def test_registry_inspection_default_retains_preexisting_reference(
|
|||
state["pulls"] += 1
|
||||
return subprocess.CompletedProcess(arguments, 0, stdout=b"pulled\n", stderr=b"")
|
||||
if operation[:2] == ["image", "rm"]:
|
||||
state["removed"] = True
|
||||
return subprocess.CompletedProcess(arguments, 0, stdout=b"removed\n", stderr=b"")
|
||||
raise AssertionError("digest reference must never be removed")
|
||||
raise AssertionError(operation)
|
||||
|
||||
monkeypatch.setattr(package.subprocess, "run", fake_run)
|
||||
|
|
@ -1180,10 +1179,10 @@ def test_registry_inspection_default_retains_preexisting_reference(
|
|||
)
|
||||
|
||||
assert inspection["status"] == "pass"
|
||||
assert state == {"present": True, "pulls": 1, "removed": False}
|
||||
assert state == {"present": True, "pulls": 1}
|
||||
|
||||
|
||||
def test_registry_inspection_rejects_reviewed_config_digest_mismatch_and_cleans_reference(
|
||||
def test_registry_inspection_rejects_config_digest_mismatch_and_retains_reference(
|
||||
tmp_path: Path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
|
|
@ -1224,14 +1223,13 @@ def test_registry_inspection_rejects_reviewed_config_digest_mismatch_and_cleans_
|
|||
docker_binary=docker_binary,
|
||||
docker_host=docker_host,
|
||||
docker_config=docker_config,
|
||||
cleanup_new_reference=True,
|
||||
)
|
||||
|
||||
assert "expected reviewed config digest" in str(raised.value.__cause__)
|
||||
assert state == {"present": False, "removed": True}
|
||||
assert state == {"present": True, "removed": False}
|
||||
|
||||
|
||||
def test_registry_inspection_failure_is_redacted_and_cleans_new_reference(
|
||||
def test_registry_inspection_pull_failure_is_redacted_and_does_not_remove_reference(
|
||||
tmp_path: Path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
|
|
@ -1272,14 +1270,13 @@ def test_registry_inspection_failure_is_redacted_and_cleans_new_reference(
|
|||
docker_binary=docker_binary,
|
||||
docker_host=docker_host,
|
||||
docker_config=docker_config,
|
||||
cleanup_new_reference=True,
|
||||
)
|
||||
|
||||
assert state["present"] is False
|
||||
assert state["present"] is True
|
||||
assert "credential=value" not in str(raised.value)
|
||||
|
||||
|
||||
def test_registry_inspection_validation_failure_removes_new_reference(
|
||||
def test_registry_inspection_validation_failure_retains_reference(
|
||||
tmp_path: Path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
|
|
@ -1327,13 +1324,12 @@ def test_registry_inspection_validation_failure_removes_new_reference(
|
|||
docker_binary=docker_binary,
|
||||
docker_host=docker_host,
|
||||
docker_config=docker_config,
|
||||
cleanup_new_reference=True,
|
||||
)
|
||||
|
||||
assert state == {"present": False, "removed": True}
|
||||
assert state == {"present": True, "removed": False}
|
||||
|
||||
|
||||
def test_registry_inspection_cleanup_failure_prevents_success(
|
||||
def test_registry_inspection_never_calls_image_rm_after_success(
|
||||
tmp_path: Path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
|
|
@ -1368,21 +1364,22 @@ def test_registry_inspection_cleanup_failure_prevents_success(
|
|||
|
||||
monkeypatch.setattr(package.subprocess, "run", fake_run)
|
||||
|
||||
with pytest.raises(package.PackageError, match="cleanup failed"):
|
||||
package.inspect_registry_image(
|
||||
image_input,
|
||||
reference,
|
||||
expected_revision=image_input["runtime"]["teleo_git_head"],
|
||||
expected_input_sha256=image_input["input_sha256"],
|
||||
expected_config_digest="sha256:" + "3" * 64,
|
||||
docker_binary=docker_binary,
|
||||
docker_host=docker_host,
|
||||
docker_config=docker_config,
|
||||
cleanup_new_reference=True,
|
||||
)
|
||||
inspection = package.inspect_registry_image(
|
||||
image_input,
|
||||
reference,
|
||||
expected_revision=image_input["runtime"]["teleo_git_head"],
|
||||
expected_input_sha256=image_input["input_sha256"],
|
||||
expected_config_digest="sha256:" + "3" * 64,
|
||||
docker_binary=docker_binary,
|
||||
docker_host=docker_host,
|
||||
docker_config=docker_config,
|
||||
)
|
||||
|
||||
assert inspection["status"] == "pass"
|
||||
assert state["present"] is True
|
||||
|
||||
|
||||
def test_registry_inspection_refuses_to_remove_changed_reference(
|
||||
def test_registry_inspection_has_no_post_inspection_cleanup_window(
|
||||
tmp_path: Path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
|
|
@ -1424,24 +1421,24 @@ def test_registry_inspection_refuses_to_remove_changed_reference(
|
|||
|
||||
monkeypatch.setattr(package.subprocess, "run", fake_run)
|
||||
|
||||
with pytest.raises(package.PackageError, match="cleanup failed"):
|
||||
package.inspect_registry_image(
|
||||
image_input,
|
||||
reference,
|
||||
expected_revision=image_input["runtime"]["teleo_git_head"],
|
||||
expected_input_sha256=image_input["input_sha256"],
|
||||
expected_config_digest="sha256:" + "3" * 64,
|
||||
docker_binary=docker_binary,
|
||||
docker_host=docker_host,
|
||||
docker_config=docker_config,
|
||||
cleanup_new_reference=True,
|
||||
)
|
||||
inspection = package.inspect_registry_image(
|
||||
image_input,
|
||||
reference,
|
||||
expected_revision=image_input["runtime"]["teleo_git_head"],
|
||||
expected_input_sha256=image_input["input_sha256"],
|
||||
expected_config_digest="sha256:" + "3" * 64,
|
||||
docker_binary=docker_binary,
|
||||
docker_host=docker_host,
|
||||
docker_config=docker_config,
|
||||
)
|
||||
|
||||
assert inspection["status"] == "pass"
|
||||
assert state["present"] is True
|
||||
assert state["removed"] is False
|
||||
assert state["inventory_calls"] == 1
|
||||
|
||||
|
||||
def test_registry_inspection_rejects_preexisting_reference_without_mutation(
|
||||
def test_registry_inspection_accepts_preexisting_reference_without_removal(
|
||||
tmp_path: Path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
|
|
@ -1463,25 +1460,27 @@ def test_registry_inspection_rejects_preexisting_reference_without_mutation(
|
|||
)
|
||||
if operation[:2] == ["image", "inspect"]:
|
||||
return subprocess.CompletedProcess(arguments, 0, stdout=payload, stderr=b"")
|
||||
if operation[0] == "pull":
|
||||
return subprocess.CompletedProcess(arguments, 0, stdout=b"pulled\n", stderr=b"")
|
||||
if operation[:2] == ["image", "rm"]:
|
||||
raise AssertionError("digest reference must never be removed")
|
||||
raise AssertionError(operation)
|
||||
|
||||
monkeypatch.setattr(package.subprocess, "run", fake_run)
|
||||
|
||||
with pytest.raises(package.PackageError, match="inspection failed") as raised:
|
||||
package.inspect_registry_image(
|
||||
image_input,
|
||||
reference,
|
||||
expected_revision=image_input["runtime"]["teleo_git_head"],
|
||||
expected_input_sha256=image_input["input_sha256"],
|
||||
expected_config_digest="sha256:" + "3" * 64,
|
||||
docker_binary=docker_binary,
|
||||
docker_host=docker_host,
|
||||
docker_config=docker_config,
|
||||
cleanup_new_reference=True,
|
||||
)
|
||||
inspection = package.inspect_registry_image(
|
||||
image_input,
|
||||
reference,
|
||||
expected_revision=image_input["runtime"]["teleo_git_head"],
|
||||
expected_input_sha256=image_input["input_sha256"],
|
||||
expected_config_digest="sha256:" + "3" * 64,
|
||||
docker_binary=docker_binary,
|
||||
docker_host=docker_host,
|
||||
docker_config=docker_config,
|
||||
)
|
||||
|
||||
assert "must be absent" in str(raised.value.__cause__)
|
||||
assert not any(operation and operation[0] == "pull" for operation in operations)
|
||||
assert inspection["status"] == "pass"
|
||||
assert any(operation and operation[0] == "pull" for operation in operations)
|
||||
assert not any(operation[:2] == ["image", "rm"] for operation in operations)
|
||||
|
||||
|
||||
|
|
@ -1702,7 +1701,6 @@ def test_finalize_embeds_inspection_before_rendering_unit(
|
|||
"docker_binary": docker_binary,
|
||||
"docker_host": docker_host,
|
||||
"docker_config": docker_config,
|
||||
"cleanup_new_reference": False,
|
||||
}
|
||||
assert json.loads(capsys.readouterr().out) == {"schema": package.RELEASE_SCHEMA, "status": "pass"}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue