Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 25 additions & 9 deletions deploy/release.sh
Original file line number Diff line number Diff line change
Expand Up @@ -85,33 +85,48 @@ refresh_trending_scout_pin() {
echo "Pinned trending scout image: $TRENDING_SCOUT_IMAGE_ID"
}

verify_trending_scout_pin() {
if ! docker image inspect "$TRENDING_SCOUT_IMAGE" >/dev/null 2>&1; then
echo "ERROR: Trending scout image was removed during cleanup: $TRENDING_SCOUT_IMAGE"
exit 1
fi

CURRENT_SCOUT_IMAGE_ID=$(docker image inspect --format '{{.Id}}' "$TRENDING_SCOUT_IMAGE")
assert_trending_scout_pin() {
if ! docker inspect "$TRENDING_SCOUT_PIN_NAME" >/dev/null 2>&1; then
echo "ERROR: Trending scout pin container is missing: $TRENDING_SCOUT_PIN_NAME"
exit 1
fi
PINNED_SCOUT_IMAGE_ID=$(docker inspect --format '{{.Image}}' "$TRENDING_SCOUT_PIN_NAME")
PINNED_SCOUT_RUNNING=$(docker inspect --format '{{.State.Running}}' "$TRENDING_SCOUT_PIN_NAME")
if [ "$CURRENT_SCOUT_IMAGE_ID" != "$TRENDING_SCOUT_IMAGE_ID" ] || \
[ "$PINNED_SCOUT_IMAGE_ID" != "$TRENDING_SCOUT_IMAGE_ID" ] || \
if [ "$PINNED_SCOUT_IMAGE_ID" != "$TRENDING_SCOUT_IMAGE_ID" ] || \
[ "$PINNED_SCOUT_RUNNING" != "true" ]; then
echo "ERROR: Trending scout pin no longer protects the scheduled image"
exit 1
fi
}

restore_trending_scout_tag() {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

restore_trending_scout_tag calls assert_trending_scout_pin, and then verify_trending_scout_pin re-runs the same assert immediately after. This double-assert is harmless but slightly redundant; consider documenting why both are needed (restore needs the pin valid before re-tagging, verify confirms the end state). Alternatively keep it — the cheap re-check is defensible for safety.

# Docker prune can remove a mutable tag even while the pin retains its image
# ID. Recreate the cron's tag from that protected immutable reference.
assert_trending_scout_pin
docker tag "$TRENDING_SCOUT_IMAGE_ID" "$TRENDING_SCOUT_IMAGE"

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docker tag "$TRENDING_SCOUT_IMAGE_ID" "$TRENDING_SCOUT_IMAGE" is not guarded by || exit 1. A failed tag would be caught by the subsequent verify_trending_scout_pin, but consider adding an explicit error message here (mirroring the other ERROR blocks) so the failure mode is self-describing rather than surfacing as a generic 'tag does not resolve' later.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the whole bug stems from a mutable tag being pruned, consider whether the cron should reference the immutable ID (or a distinct pinned tag) directly instead of relying on re-tagging after every prune. Re-tagging is a valid fix, but a short comment explaining why the mutable tag must be preserved (cron scheduler resolution) would help future maintainers avoid regressing this.

}

verify_trending_scout_pin() {
assert_trending_scout_pin
if ! docker image inspect "$TRENDING_SCOUT_IMAGE" >/dev/null 2>&1; then
echo "ERROR: Trending scout image was not restored after cleanup: $TRENDING_SCOUT_IMAGE"
exit 1
fi

CURRENT_SCOUT_IMAGE_ID=$(docker image inspect --format '{{.Id}}' "$TRENDING_SCOUT_IMAGE")
if [ "$CURRENT_SCOUT_IMAGE_ID" != "$TRENDING_SCOUT_IMAGE_ID" ]; then
echo "ERROR: Trending scout tag does not resolve to the protected image"
exit 1
fi
}

# ========== DISK SPACE CHECK & CLEANUP ==========
echo "Pre-cleanup disk usage:"
df -h /
refresh_trending_scout_pin
docker system prune -af || true
docker builder prune -af || true
restore_trending_scout_tag
verify_trending_scout_pin

echo "Post-cleanup disk usage:"
Expand Down Expand Up @@ -555,6 +570,7 @@ fi
# Stopped rollback containers still protect their known-good images here.
echo "Cleaning up unused images..."
docker image prune -af
restore_trending_scout_tag
verify_trending_scout_pin
df -h /

Expand Down
10 changes: 9 additions & 1 deletion tests/test_deploy_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,26 @@ def test_release_refreshes_and_verifies_the_trending_scout_pin() -> None:
assert "--memory 8m --cpus 0.01" in release_script
assert "--label panicradar.role=trending-scout-image-pin" not in release_script
assert 'docker inspect --format \'{{.Image}}\' "$TRENDING_SCOUT_PIN_NAME"' in release_script
assert 'docker tag "$TRENDING_SCOUT_IMAGE_ID" "$TRENDING_SCOUT_IMAGE"' in release_script
assert "Trending scout pin container is missing" in release_script

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new assertion checks for the exact docker tag "$TRENDING_SCOUT_IMAGE_ID" "$TRENDING_SCOUT_IMAGE" string but does not assert it appears before the verify/tag-resolution logic in any ordering sense — that is only covered for the prune points below. Minor: this string-match is sufficient for now, but a dedicated unit test for restore_trending_scout_tag (e.g., asserting it calls assert before docker tag) would tighten coverage.


refresh_call = release_script.index("\nrefresh_trending_scout_pin\n")
system_prune = release_script.index("docker system prune -af")
post_deploy_prune = release_script.index("docker image prune -af")
restore_after_system_prune = release_script.index(
"\nrestore_trending_scout_tag\n", system_prune
)
verify_after_system_prune = release_script.index(
"\nverify_trending_scout_pin\n", system_prune
)
restore_after_post_deploy_prune = release_script.index(
"\nrestore_trending_scout_tag\n", post_deploy_prune
)
verify_after_post_deploy_prune = release_script.index(
"\nverify_trending_scout_pin\n", post_deploy_prune
)

assert refresh_call < system_prune
assert system_prune < restore_after_system_prune < verify_after_system_prune
assert verify_after_system_prune < post_deploy_prune
assert verify_after_post_deploy_prune > post_deploy_prune
assert post_deploy_prune < restore_after_post_deploy_prune < verify_after_post_deploy_prune
Loading