fix(ci): #315 address review nits — drop fragile FAIL_RE, validate --timeout, log image id
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 10s
Build ErsatzTV Image / decisions.md append-only (pull_request) Successful in 11s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Failing after 8s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 18s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 5m3s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 8m48s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 10s
Build ErsatzTV Image / decisions.md append-only (pull_request) Successful in 11s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Failing after 8s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 18s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 5m3s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 8m48s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Adversarial review (MERGEABLE-WITH-NITS) findings: - Remove the broad FAIL_RE log-scan (matched benign ErsatzTV startup noise — library scans against absent media mounts, EF connection retries — risking a false-FAIL that blocks a good release). It was also redundant: a failed migration faults the BackgroundService -> default StopHost -> container exit, which the early-exit check already catches reliably (per the reviewer's own analysis). Migration failure is now detected by early container exit + timeout + the post-boot serve probe. - Validate --timeout is a positive integer (was: '--timeout abc' -> 0 -> instant false-FAIL). - Log the resolved image id after (attempted) pull, so a pull-failure that rehearses a stale local :latest is visible to the operator. Re-validated live on bumblebee: :latest vs the 283MB prod-copy -> migrations clean, PASS, image digest logged, no leftover temp dir/container. shellcheck + bash -n clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
+17
-10
@@ -51,6 +51,8 @@ done
|
||||
|
||||
command -v docker >/dev/null 2>&1 || die "docker not found on PATH (run this on the docker host)"
|
||||
[ -n "$IMAGE" ] || die "--image <ref> is required (the image about to be promoted)"
|
||||
case "$TIMEOUT" in ''|*[!0-9]*) die "--timeout must be a positive integer of seconds (got: '$TIMEOUT')";; esac
|
||||
[ "$TIMEOUT" -gt 0 ] || die "--timeout must be greater than 0"
|
||||
|
||||
# Resolve the backup DB: explicit --db, else newest ersatztv.sqlite3 under $BACKUP_DIR.
|
||||
if [ -z "$DB" ]; then
|
||||
@@ -85,7 +87,13 @@ chmod u+rw "$WORK/config/ersatztv.sqlite3"
|
||||
SIZE=$(du -h "$WORK/config/ersatztv.sqlite3" | cut -f1)
|
||||
echo "migration-smoke: image=$IMAGE db-copy=${SIZE} timeout=${TIMEOUT}s"
|
||||
|
||||
docker pull "$IMAGE" >/dev/null 2>&1 || echo "migration-smoke: (pull failed/offline — using local image if present)"
|
||||
if ! docker pull "$IMAGE" >/dev/null 2>&1; then
|
||||
# Not fatal (offline / locally-built images), but for a mutable tag (:latest) a stale local copy
|
||||
# would rehearse the wrong image — so we log the resolved image ID below for the operator to verify.
|
||||
echo "migration-smoke: WARNING — pull failed; rehearsing whatever '$IMAGE' resolves to locally"
|
||||
fi
|
||||
IMG_ID=$(docker image inspect "$IMAGE" --format '{{.Id}}' 2>/dev/null || echo "unknown")
|
||||
echo "migration-smoke: rehearsing $IMAGE ($IMG_ID)"
|
||||
|
||||
# Boot the new image against the prod-copy. DatabaseMigratorService (a BackgroundService) applies
|
||||
# pending migrations on startup; it logs "Applying database migrations" then "Done applying database
|
||||
@@ -97,20 +105,19 @@ docker run -d --name "$NAME" --memory 2g \
|
||||
-v "$WORK/config:/config" \
|
||||
"$IMAGE" >/dev/null || die "docker run failed for $IMAGE"
|
||||
|
||||
# PASS gate = the migrator's completion log line. FAIL = the container exiting before it (a failed
|
||||
# migration faults the BackgroundService → default StopHost → the host stops → the container exits, so
|
||||
# early-exit IS the reliable migration-failure signal), or the timeout below. We deliberately do NOT
|
||||
# grep logs for error strings: ErsatzTV's startup is noisy (library scans against absent media mounts,
|
||||
# EF connection retries) and a broad error-regex would false-FAIL a good migration.
|
||||
DONE_RE='Done applying database migrations'
|
||||
FAIL_RE='error occurred while|Unhandled exception|SqliteException|BackgroundService failed|An exception occurred|Failed to migrate'
|
||||
|
||||
mig_done=0
|
||||
deadline=$((SECONDS + TIMEOUT))
|
||||
while [ $SECONDS -lt $deadline ]; do
|
||||
logs=$(docker logs "$NAME" 2>&1 || true)
|
||||
if printf '%s' "$logs" | grep -qE "$DONE_RE"; then mig_done=1; break; fi
|
||||
if printf '%s' "$logs" | grep -qiE "$FAIL_RE"; then
|
||||
echo "migration-smoke: FAIL — migration error in logs:"; printf '%s\n' "$logs" | tail -n 40
|
||||
exit 1
|
||||
fi
|
||||
if docker logs "$NAME" 2>&1 | grep -qE "$DONE_RE"; then mig_done=1; break; fi
|
||||
if [ -z "$(docker ps -q --filter name="$NAME" --filter status=running)" ]; then
|
||||
echo "migration-smoke: FAIL — container exited before finishing migrations:"; printf '%s\n' "$logs" | tail -n 40
|
||||
echo "migration-smoke: FAIL — container exited before finishing migrations (a failed migration stops the host):"
|
||||
docker logs "$NAME" 2>&1 | tail -n 40
|
||||
exit 1
|
||||
fi
|
||||
sleep 2
|
||||
|
||||
Reference in New Issue
Block a user