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
4 changes: 2 additions & 2 deletions client/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ apiVersion: v2
name: client
description: A unified Helm chart for tracebloc on AKS, EKS, bare-metal, and OpenShift
type: application
version: 1.9.116
appVersion: "1.9.116"
version: 1.9.117
appVersion: "1.9.117"
keywords:
- tracebloc
- kubernetes
Expand Down
41 changes: 37 additions & 4 deletions client/templates/image-refresh-cronjob.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,13 @@ data:

restart_needed=0
annotate_args=""
# Pass-0 diagnostic annotations -- the `stale-pin-<image>` clears and
# finding-sets -- accumulate SEPARATELY from annotate_args and are written in
# their own bounded annotate BEFORE the restart block (#1008 item 1).
# They touch only the stale-pin keys, never `last-refreshed-<image>-digest`,
# so they are safe to land before the rollout; keeping them out of the final
# annotate is what lets them survive a latched-flap tick (see the write site).
stale_pin_args=""
# `kubectl set image` argument lists, accumulated per WORKLOAD (a single
# `set image` call can carry several container=ref pairs, so the
# two-container jobs-manager Deployment is re-imaged in one patch and
Expand Down Expand Up @@ -585,7 +592,7 @@ data:
disabled_stale_key="tracebloc.io/stale-pin-${repo#*/}"
if [ -n "$(get_annotation "$disabled_stale_key" || true)" ]; then
log " clearing ${disabled_stale_key}: this image is no longer pinned by a digest"
annotate_args="$annotate_args ${disabled_stale_key}-"
stale_pin_args="$stale_pin_args ${disabled_stale_key}-"
fi
continue
fi
Expand Down Expand Up @@ -613,7 +620,7 @@ data:
# (@saqlainsyed007 + Bugbot on client#824.)
if [ -n "$(get_annotation "$stale_key" || true)" ]; then
log " clearing a previous ${stale_key}: the pin is current again"
annotate_args="$annotate_args ${stale_key}-"
stale_pin_args="$stale_pin_args ${stale_key}-"
fi
else
log " WARN: PIN IS STALE. values pin ${pin_digest}"
Expand All @@ -624,7 +631,7 @@ data:
log " in values (backend#2458)."
# Queryable after the log ages out, on the same object the refresh
# annotations use, so `kubectl describe` shows pin state beside refresh state.
annotate_args="$annotate_args ${stale_key}=${pin_latest}"
stale_pin_args="$stale_pin_args ${stale_key}=${pin_latest}"
fi
continue
fi
Expand All @@ -637,7 +644,7 @@ data:
unpinned_stale_key="tracebloc.io/stale-pin-${repo#*/}"
if [ -n "$(get_annotation "$unpinned_stale_key" || true)" ]; then
log " clearing ${unpinned_stale_key}: this image is no longer pinned"
annotate_args="$annotate_args ${unpinned_stale_key}-"
stale_pin_args="$stale_pin_args ${unpinned_stale_key}-"
fi

latest="$(get_latest_digest "$repo" "$IMAGE_TAG" "$IMAGE_REGISTRY" || true)"
Expand Down Expand Up @@ -881,6 +888,32 @@ data:
esac
done

# Pass-0 annotations land HERE, before the restart block (#1008 item 1).
# The restart block's #563 flap guard does `WARN + FLAP_KEY + exit 0`
# once refresh-attempt >= MAX_REFRESH_ATTEMPTS -- BEFORE the digest-record
# annotate at the end of the tick. So on a tick that is both off-digest
# (restart_needed=1) and latched, batching the stale-pin writes into that
# final annotate dropped them: a stale-pin CLEAR that never landed leaves a
# FALSE "pin is stale" finding to persist forever (a write-only annotation
# outliving its problem -- the class client#824's clear paths fixed), and it
# is dropped on the exact tick refresh is dead, when the finding matters most.
# These touch only the `stale-pin-<image>` keys, never
# `last-refreshed-<image>-digest`, so writing them before the rollout cannot
# affect the `recorded == latest` skip logic -- unlike the digest record,
# which MUST stay after a successful `rollout status`, since annotating the
# digest before a failed rollout would freeze the workload on the old image
# (@shujaatTracebloc on #1008). NON-FATAL, like the SKIP_KEY clear above
# (backend#2007): a transient failure on a diagnostic annotation must not
# abort the tick before the re-image; a stale value is re-reconciled next tick.
if [ -n "$stale_pin_args" ]; then
log "updating stale-pin annotations:$stale_pin_args"
# shellcheck disable=SC2086 # word-split stale_pin_args intentional
if ! sp_err="$(kubectl annotate deployment -n "$RELEASE_NAMESPACE" "$DEPLOYMENT_NAME" \
$stale_pin_args --overwrite --request-timeout=15s 2>&1 >/dev/null)"; then
log " WARNING: could not update stale-pin annotations on deployment/${DEPLOYMENT_NAME}: ${sp_err:-unknown error}. Continuing -- these are diagnostic bookkeeping and a stale value is re-reconciled on the next tick."
fi
fi

# Order matters: rollout FIRST, annotate AFTER `rollout status`
# succeeds. Annotating first would let a failed rollout silently
# freeze the deployment on the old image (next tick sees
Expand Down
46 changes: 46 additions & 0 deletions client/tests/image_refresh_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1020,3 +1020,49 @@ tests:
- notMatchRegex:
path: data["image-refresh.sh"]
pattern: 'recorded="\$\(get_annotation "\$key" \|\| true\)"'

- it: stale-pin annotations are written BEFORE the restart block, so they survive a latched flap
# Guards #1008 item 1. The #563 flap guard does WARN + FLAP_KEY +
# exit 0 once refresh-attempt >= MAX, BEFORE the final digest-record annotate.
# Batching the stale-pin CLEARS into that final annotate dropped them on a
# tick that is both off-digest and latched -- leaving a FALSE stale-pin
# finding to persist. They now accumulate in their own list and are annotated
# above the restart block. image-refresh-latched-annotate.bats asserts the
# BEHAVIOUR; these lock the code shapes.
template: templates/image-refresh-cronjob.yaml
documentIndex: 0
asserts:
# ALL FOUR stale-pin writes go to their own accumulator, not annotate_args.
# Every key gets a positive (goes to stale_pin_args) AND a negative (does NOT
# ride annotate_args) -- Bugbot Low + @saadqbal on #1039: a key covered by
# neither could be re-batched into annotate_args and dropped on a latched
# tick while both suites stayed green (this guard's own thesis, one level up).
# The two clears:
- matchRegex:
path: data["image-refresh.sh"]
pattern: 'stale_pin_args="\$stale_pin_args \$\{stale_key\}-"'
- matchRegex:
path: data["image-refresh.sh"]
pattern: 'stale_pin_args="\$stale_pin_args \$\{unpinned_stale_key\}-"'
# the disabled-monitor clear (the third clear -- was uncovered):
- matchRegex:
path: data["image-refresh.sh"]
pattern: 'stale_pin_args="\$stale_pin_args \$\{disabled_stale_key\}-"'
# the finding-SET (a lost set means a real staleness goes unreported):
- matchRegex:
path: data["image-refresh.sh"]
pattern: 'stale_pin_args="\$stale_pin_args \$\{stale_key\}=\$\{pin_latest\}"'
# and that accumulator is annotated BEFORE the restart block (ordering)
- matchRegex:
path: data["image-refresh.sh"]
pattern: '(?s)if \[ -n "\$stale_pin_args" \]; then.*if \[ "\$restart_needed" -eq 1 \]; then'
# NONE of the stale-pin keys may ride the final digest-record annotate_args
- notMatchRegex:
path: data["image-refresh.sh"]
pattern: 'annotate_args="\$annotate_args \$\{stale_key\}'
- notMatchRegex:
path: data["image-refresh.sh"]
pattern: 'annotate_args="\$annotate_args \$\{unpinned_stale_key\}'
Comment thread
saqlainsyed007 marked this conversation as resolved.
- notMatchRegex:
path: data["image-refresh.sh"]
pattern: 'annotate_args="\$annotate_args \$\{disabled_stale_key\}'
164 changes: 164 additions & 0 deletions scripts/tests/image-refresh-latched-annotate.bats
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
#!/usr/bin/env bats
# image-refresh writes the Pass-0 stale-pin annotations BEFORE the restart block,
# so they survive a tick that is both off-digest (restart_needed=1) and LATCHED
# (refresh-attempt >= MAX_REFRESH_ATTEMPTS).
#
# #1008 item 1. The #563 flap guard does `WARN + FLAP_KEY + exit 0` once
# the attempt counter reaches MAX -- BEFORE the digest-record annotate at the end
# of the tick. When the stale-pin CLEARS were batched into that final annotate,
# a latched tick dropped them, leaving a FALSE "pin is stale" finding to persist
# forever -- and on the exact tick refresh is dead, when the finding matters most.
# The fix moves the stale-pin writes into their own bounded annotate above the
# restart block; the `last-refreshed` digest record deliberately stays BELOW,
# after a successful rollout (@shujaatTracebloc on #1008).
#
# This asserts BEHAVIOUR: it extracts the shipped tail (the stale-pin annotate +
# the restart block + the final digest annotate) from the RENDERED chart and
# drives it with kubectl and the attempt-counter read stubbed, so re-batching the
# stale-pin writes back into the final annotate reddens.

setup() {
TMP="$(mktemp -d)"
CHART="${BATS_TEST_DIRNAME}/../../client"
helm template t "$CHART" --set clientId=x --set clientPassword=y \
--set storageClass.create=false > "$TMP/rendered.yaml"
python3 - "$TMP/rendered.yaml" "$TMP/tail.sh" <<'PYX'
import sys

try:
import yaml
except ImportError:
sys.exit("[ERROR] PyYAML required (pip install pyyaml)")

MARKER = "already on the pinned digest; no-op"

def walk(o):
if isinstance(o, str) and MARKER in o:
return o
if isinstance(o, dict):
for v in o.values():
r = walk(v)
if r:
return r
if isinstance(o, list):
for v in o:
r = walk(v)
if r:
return r

script = None
for d in yaml.safe_load_all(open(sys.argv[1])):
if not d:
continue
script = walk(d)
if script:
break
assert script, "no rendered image-refresh script found"

lines = script.splitlines()
start = next(i for i, l in enumerate(lines)
if l.strip() == 'if [ -n "$stale_pin_args" ]; then')
# the LAST `log "tick complete"` -- the flap-guard early exits use the same line,
# so the first match would truncate the region mid-restart-block.
end = max(i for i in range(start, len(lines))
if lines[i].strip() == 'log "tick complete"')
region = lines[start:end + 1]
indent = min(len(l) - len(l.lstrip()) for l in region if l.strip())
open(sys.argv[2], "w").write("\n".join(l[indent:] for l in region))
PYX
}
teardown() { rm -rf "$TMP"; }

# Drives the shipped tail with kubectl + the ATTEMPT_KEY read stubbed.
# $1 = STUB_ATTEMPT what get_annotation returns for ATTEMPT_KEY (the flap count)
# $2 = JM_SET_ARGS `set image` args (non-empty => a rollout runs, stubbed OK)
# stale_pin_args and annotate_args are always populated so the test can assert
# which of the two landed.
#
# The kubectl stub records EVERY call to "$TMP/calls.log" rather than stdout,
# because the stale-pin annotate is wrapped in a non-fatal handler that discards
# its stdout (`2>&1 >/dev/null`) -- exactly as a real silent-success annotate
# would. The file captures the call regardless of the caller's redirections;
# assert kubectl invocations against "$TMP/calls.log" and log lines against stdout.
run_tail() {
: > "$TMP/calls.log"
cat > "$TMP/harness.sh" <<EOF
set -eu
CALLS="$TMP/calls.log"
RELEASE_NAMESPACE="tracebloc"
DEPLOYMENT_NAME="jobs-manager"
REQUESTS_PROXY_DEPLOYMENT="t-requests-proxy"
RESOURCE_MONITOR_DAEMONSET="t-resource-monitor"
NODE_AGENTS_NAMESPACE="tracebloc-node-agents"
ATTEMPT_KEY="tracebloc.io/refresh-attempt"
FLAP_KEY="tracebloc.io/refresh-flap-detected"
MAX_REFRESH_ATTEMPTS=3
ROLLOUT_TIMEOUT="10m"
restart_needed=1
stale_pin_args=" tracebloc.io/stale-pin-jobs-manager-"
annotate_args=" tracebloc.io/last-refreshed-jobs-manager-digest=sha256:beef"
jm_set_args="\${2:-}"
rp_set_args=""
rm_set_args=""
STUB_ATTEMPT="\${1:-0}"
log() { printf '%s\n' "\$*"; }
kubectl() { printf 'KUBECTL:%s\n' "\$*" >> "\$CALLS"; }
get_annotation() { case "\$1" in "\$ATTEMPT_KEY") printf '%s' "\$STUB_ATTEMPT" ;; esac; }
$(cat "$TMP/tail.sh")
EOF
sh "$TMP/harness.sh" "${1:-0}" "${2:-}"
}

@test "the harness really extracted the shipped tail (not an empty file)" {
[ -s "$TMP/tail.sh" ] || return 1
grep -q 'stale_pin_args' "$TMP/tail.sh" || return 1
grep -q 'restart_needed' "$TMP/tail.sh" || return 1
}

@test "LATCHED tick (restart_needed=1, attempt>=MAX): stale-pin clear LANDS, digest record does NOT" {
# The acceptance case (#1008 item 1). attempt=3, MAX=3 -> the flap guard
# WARNs, annotates FLAP_KEY, and exit 0s. The stale-pin clear must already have
# been written (before the restart block); the last-refreshed digest record
# must NOT be (its annotate is after the guard and never runs).
run run_tail "3"
[ "$status" -eq 0 ] || return 1
calls="$(cat "$TMP/calls.log")"
# stale-pin clear landed, above the restart block
[[ "$calls" == *"annotate deployment"*"tracebloc.io/stale-pin-jobs-manager-"* ]] || return 1
# the flap guard fired
[[ "$output" == *"FLAP DETECTED"* ]] || return 1
[[ "$calls" == *"tracebloc.io/refresh-flap-detected=3"* ]] || return 1
# the digest record did NOT land (dropped by the exit 0, as designed)
[[ "$calls" != *"last-refreshed-jobs-manager-digest=sha256:beef"* ]] || return 1
}

@test "NON-latched tick (attempt<MAX): stale-pin clear lands AND the digest record lands after the rollout" {
# A healthy re-image tick still writes both, in order: stale-pin first, then
# the counter bump + rollout, then the digest record. Proves the split did not
# drop the digest record on the normal path.
run run_tail "0" "api=docker.io/tracebloc/jobs-manager@sha256:beef"
[ "$status" -eq 0 ] || return 1
calls="$(cat "$TMP/calls.log")"
[[ "$calls" == *"annotate deployment"*"tracebloc.io/stale-pin-jobs-manager-"* ]] || return 1
# the counter is bumped (not latched) and a rollout runs
[[ "$calls" == *"tracebloc.io/refresh-attempt=1"* ]] || return 1
[[ "$calls" == *"set image"* ]] || return 1
# the digest record DOES land on a healthy tick
[[ "$calls" == *"last-refreshed-jobs-manager-digest=sha256:beef"* ]] || return 1
[[ "$output" != *"FLAP DETECTED"* ]] || return 1
}

@test "stale-pin annotate is a SEPARATE call from the digest-record annotate" {
# The two must not be the same annotate: batching is exactly what dropped the
# clears on a latched tick. On a healthy tick both run, as two distinct
# `kubectl annotate` calls -- the stale-pin one carrying the stale-pin key, the
# other carrying last-refreshed.
run run_tail "0" "api=docker.io/tracebloc/jobs-manager@sha256:beef"
[ "$status" -eq 0 ] || return 1
stale="$(grep -c 'annotate.*stale-pin-jobs-manager' "$TMP/calls.log")"
digest="$(grep -c 'annotate.*last-refreshed-jobs-manager-digest' "$TMP/calls.log")"
[ "$stale" -ge 1 ] || return 1
[ "$digest" -ge 1 ] || return 1
# and no single annotate carries both keys
[ "$(grep 'annotate' "$TMP/calls.log" | grep 'stale-pin' | grep -c 'last-refreshed')" -eq 0 ] || return 1
}
5 changes: 5 additions & 0 deletions scripts/tests/image-refresh-stale-pin.bats
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,18 @@ pin_digest="\${1:-}"
STUB_LATEST="\${2:-}"
EXISTING_ANNOTATION="\${3:-}"
annotate_args=""
# #1008 item 1: the stale-pin writes moved into their own accumulator
# (annotated before the restart block so a latched flap can't drop them). The
# branch under test writes here now, so the harness must define + print it.
stale_pin_args=""
log() { printf '%s\n' "\$*"; }
get_latest_digest() { [ -n "\$STUB_LATEST" ] && printf '%s' "\$STUB_LATEST"; }
get_annotation() { [ -n "\$EXISTING_ANNOTATION" ] && printf '%s' "\$EXISTING_ANNOTATION"; }
for _once in 1; do
$(sed 's/^/ /' "$TMP/branch.sh")
done
printf 'ANNOTATE:%s\n' "\$annotate_args"
printf 'STALEPIN:%s\n' "\$stale_pin_args"
EOF
sh "$TMP/harness.sh" "$1" "$2" "${3:-}"
}
Expand Down
Loading