diff --git a/.gitea/workflows/ci.yaml b/.gitea/workflows/ci.yaml index 8367bb4..4f7fc8d 100644 --- a/.gitea/workflows/ci.yaml +++ b/.gitea/workflows/ci.yaml @@ -464,7 +464,24 @@ jobs: build: needs: - [lint-actionlint, lint-shellcheck, lint-compose, lint-prettier, lint-ruff, lint-yaml, lint-dockerfiles, validate] + # scan-deps and the two test jobs were missing here, so a commit with a + # known-vulnerable dependency or a failing test still moved the :prod tag. + # The deploy was blocked either way - it requires the whole workflow to + # have succeeded - but the tag had already moved, and the next deploy to + # run resolved it. Publishing and passing the checks are the same gate. + [ + lint-actionlint, + lint-shellcheck, + lint-compose, + lint-prettier, + lint-ruff, + lint-yaml, + lint-dockerfiles, + scan-deps, + test-backend, + test-frontend, + validate, + ] if: github.event_name != 'pull_request' && (github.ref_name == 'main' || github.ref_name == 'dev') runs-on: [self-hosted, linux, arch, homelab] timeout-minutes: 60 @@ -480,12 +497,20 @@ jobs: id: services shell: bash run: | + set -euo pipefail base="${{ github.event.before }}" if [ -z "$base" ] || [ "$base" = "0000000000000000000000000000000000000000" ]; then base="$(git rev-list --max-parents=0 HEAD)" fi - mapfile -t changed_files < <(git diff --name-only "$base" "${GITHUB_SHA}") + # A failed diff used to leave changed_files empty, which reads exactly + # like "nothing to build": the job went green having built nothing and + # the tag never moved. The status is checked, not assumed. + if ! changed="$(git diff --name-only "$base" "${GITHUB_SHA}")"; then + echo "::error::cannot diff ${base}..${GITHUB_SHA}" + exit 1 + fi + mapfile -t changed_files <<<"$changed" services=() @@ -535,30 +560,57 @@ jobs: - name: Log in to registry if: steps.services.outputs.services != '' shell: bash + # Through env, not by substitution into the script. A secret written + # into a run: block is pasted into the shell source before bash parses + # it, so a password containing a quote, a backtick or $(...) becomes + # code that runs. Masking the value in the log does not prevent that. + env: + REGISTRY_USERNAME: ${{ secrets.REGISTRY_USERNAME }} + REGISTRY_PASSWORD: ${{ secrets.REGISTRY_PASSWORD }} run: | - echo "${{ secrets.REGISTRY_PASSWORD }}" | docker login "${REGISTRY}" \ - -u "${{ secrets.REGISTRY_USERNAME }}" \ + set -euo pipefail + printf '%s' "$REGISTRY_PASSWORD" | docker login "${REGISTRY}" \ + -u "$REGISTRY_USERNAME" \ --password-stdin - name: Build and push changed images if: steps.services.outputs.services != '' shell: bash run: | + # This step was the one run: block in the workflow without it, and it + # is the one that cannot afford it: a docker push that failed partway + # through the loop used to be followed by more pushes, the loop's exit + # status came from the last one, and the job went green with half the + # images missing from the registry. + set -euo pipefail IFS=, read -r -a services <<< "${{ steps.services.outputs.services }}" + # Tags for this push. The commit-pinned name is the point of this + # step: the deploy resolves it in preference to :prod, so a deploy + # that sat in the queue behind a later push still gets the build of + # the commit CI validated, instead of whatever :prod points at by the + # time it runs. See render_pinned in deploy-lib.sh. + commit_tag="" + if [ "${GITHUB_REF_NAME}" = "main" ]; then + commit_tag="sha-${GITHUB_SHA:0:12}" + fi + + set_tags() { + tags=() + case "${GITHUB_REF_NAME}" in + main) tags+=("main" "prod") ;; + dev) tags+=("dev") ;; + esac + if [ -n "$commit_tag" ]; then + tags+=("$commit_tag") + fi + } + for service in "${services[@]}"; do case "$service" in dtek_notif) image="${REGISTRY}/forust/dtek-notif" - tags=() - case "${GITHUB_REF_NAME}" in - main) - tags+=("main" "prod") - ;; - dev) - tags+=("dev") - ;; - esac + set_tags build_args=() for tag in "${tags[@]}"; do build_args+=(-t "${image}:${tag}") @@ -573,15 +625,7 @@ jobs: ;; errorpages) image="${REGISTRY}/forust/error-pages" - tags=() - case "${GITHUB_REF_NAME}" in - main) - tags+=("main" "prod") - ;; - dev) - tags+=("dev") - ;; - esac + set_tags build_args=() for tag in "${tags[@]}"; do build_args+=(-t "${image}:${tag}") @@ -595,15 +639,7 @@ jobs: done ;; userbot) - tags=() - case "${GITHUB_REF_NAME}" in - main) - tags+=("main" "prod") - ;; - dev) - tags+=("dev") - ;; - esac + set_tags for target in runtime panel; do case "$target" in runtime) @@ -638,15 +674,7 @@ jobs: image="${REGISTRY}/forust/xdfnx-homepage" ;; esac - tags=() - case "${GITHUB_REF_NAME}" in - main) - tags+=("main" "prod") - ;; - dev) - tags+=("dev") - ;; - esac + set_tags build_args=() for tag in "${tags[@]}"; do build_args+=(-t "${image}:${tag}") @@ -672,15 +700,7 @@ jobs: image="${REGISTRY}/forust/webinar-checker" ;; esac - tags=() - case "${GITHUB_REF_NAME}" in - main) - tags+=("main" "prod") - ;; - dev) - tags+=("dev") - ;; - esac + set_tags build_args=() for tag in "${tags[@]}"; do build_args+=(-t "${image}:${tag}") @@ -696,3 +716,42 @@ jobs: ;; esac done + + # Every image the tree names has to carry the commit-pinned name, not only + # the ones this push rebuilt. A push that touches nothing but manifests + # builds nothing, and its deploy would then find no commit-pinned tag to + # resolve and quietly fall back to the moving :prod - which is the whole + # failure the commit-pinned name exists to remove. + # + # Re-tagging copies the manifest list and transfers no layers, so pinning + # six images that already exist costs six registry writes. + # + # The list is derived from the tree rather than written out here, so an + # image added to a manifest is covered without a second place to update. + - name: Pin the commit name on the images this push did not rebuild + if: github.ref_name == 'main' + shell: bash + run: | + set -euo pipefail + commit_tag="sha-${GITHUB_SHA:0:12}" + mapfile -t repos < <( + git grep -hoE 'gcr\.forust\.xyz/forust/[A-Za-z0-9._-]+' -- '*.yaml' '*.yml' \ + | sort -u + ) + if [ "${#repos[@]}" -eq 0 ]; then + echo "No own images referenced by the tree." + exit 0 + fi + echo "pinning ${#repos[@]} image(s) to $commit_tag" + for repo in "${repos[@]}"; do + if docker buildx imagetools inspect "$repo:$commit_tag" >/dev/null 2>&1; then + echo " already built by this push: ${repo##*/}" + continue + fi + if ! docker buildx imagetools inspect "$repo:prod" >/dev/null 2>&1; then + echo " WARNING: ${repo##*/} has no :prod to pin and no build produced it" + continue + fi + docker buildx imagetools create --tag "$repo:$commit_tag" "$repo:prod" + echo " pinned ${repo##*/}" + done diff --git a/.gitea/workflows/deploy-lib.sh b/.gitea/workflows/deploy-lib.sh index 1f0de86..80d2cc7 100644 --- a/.gitea/workflows/deploy-lib.sh +++ b/.gitea/workflows/deploy-lib.sh @@ -242,11 +242,49 @@ registry_digest() { | head -1 || true } +# The commit this deploy is for: what CI validated, or - on a manual dispatch, +# whatever stage_preflight just checked out. +deploy_commit() { + local c="${DEPLOY_SHA:-}" + [ -n "$c" ] || c="$(git -C "$REPO" rev-parse HEAD 2>/dev/null || true)" + printf '%.12s' "${c:-}" +} + +# Resolves one of our image refs to the digest THIS commit's build produced. +# +# A manifest naming `:prod` names a pointer, not a version, and the deploy +# resolves it when the apply runs - which is not when CI ran it. Deploy runs are +# queued rather than cancelled (see deploy.yaml), so two pushes in a row leave +# the first deploy resolving the second push's build: the right manifests with +# the wrong code, and nothing anywhere reports it. ci therefore publishes every +# image it ships under `sha-`, a name that cannot move, and that is +# the name resolved here. +# +# The fallback to the plain tag is for an image this pipeline never built. It +# reports itself, because a fallback nobody sees is the failure this removes. +pinned_digest() { + local ref="$1" commit pinned + commit="$(deploy_commit)" + if [ -n "$commit" ]; then + pinned="$(registry_digest "${ref%:*}:sha-$commit")" + if [ -n "$pinned" ]; then + printf '%s' "$pinned" + return 0 + fi + fi + pinned="$(registry_digest "$ref")" + if [ -n "$pinned" ]; then + echo "WARNING: ${ref} carries no sha-${commit:-} tag; resolved the moving tag instead" >&2 + fi + printf '%s' "$pinned" +} + # Rewrites our own images to immutable digests on the way into the cluster. # Reads a manifest stream on stdin, writes the pinned stream to stdout. # -# A digest is not knowable when a manifest is written, so it is resolved here, at -# apply time, and never committed: git keeps a readable `:prod` tag. That is what +# A digest is not knowable when a manifest is written, so it is never committed: +# git keeps a readable `:prod` tag and the exact bytes are chosen here, at apply +# time, from the tag ci published for the commit being deployed. That is what # makes rollback mean something. `kubectl rollout undo` restores the previous # ReplicaSet's pod template verbatim, and a template naming a digest restores the # exact bytes that were serving before. A template naming a moving tag does not — @@ -271,7 +309,7 @@ render_pinned() { while read -r ref; do [ -n "$ref" ] || continue - digest="$(registry_digest "$ref")" + digest="$(pinned_digest "$ref")" if [ -z "$digest" ]; then echo "ERROR: cannot resolve ${ref} in the registry; applying nothing." >&2 echo " The build job has to push that tag before the deploy resolves it." >&2 @@ -339,7 +377,7 @@ restart_stale_images() { while read -r ns target image; do [ -n "${target:-}" ] || continue if [ -z "${digests[$image]:-}" ]; then - digests[$image]="$(registry_digest "$image")" + digests[$image]="$(pinned_digest "$image")" fi want="${digests[$image]}" if [ -z "$want" ]; then diff --git a/.gitea/workflows/deploy.yaml b/.gitea/workflows/deploy.yaml index 2ce9f83..65f883a 100644 --- a/.gitea/workflows/deploy.yaml +++ b/.gitea/workflows/deploy.yaml @@ -73,8 +73,34 @@ jobs: needs: [validate] runs-on: [self-hosted, linux, arch, homelab, prod] # Apply only, no verification, so this is just the work itself: snapshot, - # then up to three sequential `helm upgrade --atomic --timeout 10m`, then the - # apply loop. Verification has its own job and its own budget. + # then sequential `helm upgrade --atomic --timeout 10m`, then the apply loop. + # Verification has its own job and its own budget. + # + # 45 is roughly four times the measured cost of the stage, which is + # deliberately not raised on a theory: + # + # helm, healthy 3 no-op upgrades ~3-5 min + # helm, one release bad --atomic spends its 10m, ~10-15 min + # then rolls that one back + # apply loop ~40 manifests, 4 of which ~1 min + # resolve an image digest + # restart_stale_images 7.6s to find 8 workloads, ~0.5 min + # 9.8s to resolve their digests + # + # The helm figure is one release, not three: `set -e` aborts + # upgrade_helm_releases on the first failure, so a broken release costs + # 10m and the other two are never attempted. Multiplying 10m by three + # overstates the worst case by 20 minutes. + # + # The 45 minutes this was last raised to 45 were still not enough, and the + # job logs for those runs no longer exist, so what actually consumed the + # budget is not known - the two measurable candidates above account for + # ~15 of it. The one unbounded thing left in this stage is + # `docker manifest inspect` at deploy-lib.sh:236, which has no timeout + # against a registry with a known hang mode. Bound it, and make the stage + # announce what it is working on, before spending any of that on a larger + # ceiling: a stage that is killed with a diagnosable last line is a bug + # report, one that vanishes is not. timeout-minutes: 45 steps: - name: Checkout repository @@ -112,7 +138,22 @@ jobs: needs.apply-k8s.result != 'skipped' && needs.apply-compose.result != 'skipped' runs-on: [self-hosted, linux, arch, homelab, prod] - # ceil(changed_workloads / 8) waves of ROLLOUT_TIMEOUT each, plus rollback. + # Not raised, because the arithmetic does not close. + # + # 32 workloads are under management and the wave width is 8, so the verify + # itself is 4 waves of ROLLOUT_TIMEOUT (300s) = 20 minutes worst case, when + # every rollout times out rather than converging. That is already 20 of 30. + # + # The other 10 would have to absorb rollback, and rollback_workloads is a + # serial `while read` loop at 300s per failed workload. 10 minutes buys two. + # Any larger number is buying a bigger multiple of an unbounded term rather + # than covering a known cost: 60 minutes buys eight, and 60 minutes is + # therefore not a bound, it is a guess with two digits. + # + # The number becomes derivable the moment rollback uses the same wave width + # as the verify: 32 failures then cost 4 waves = 20 minutes instead of 160, + # and 45 covers verify plus rollback at full width. That change is to the + # recovery path and is not folded into a timeout edit. timeout-minutes: 30 steps: - name: Checkout repository