diff --git a/client/Chart.yaml b/client/Chart.yaml index bdaf83cb..de42aa09 100644 --- a/client/Chart.yaml +++ b/client/Chart.yaml @@ -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.109 -appVersion: "1.9.109" +version: 1.9.110 +appVersion: "1.9.110" keywords: - tracebloc - kubernetes diff --git a/client/templates/image-refresh-cronjob.yaml b/client/templates/image-refresh-cronjob.yaml index 2b371ebe..d664eab1 100644 --- a/client/templates/image-refresh-cronjob.yaml +++ b/client/templates/image-refresh-cronjob.yaml @@ -41,33 +41,36 @@ data: # resource-monitor was likewise untouched (and a chart release does not # change its image ref either), so it only ever moved by accident. # - # TWO KNOWN, BOUNDED LIMITATIONS of keeping the annotation (rather than the - # live pod spec) as the source of truth. Both are self-healing at the next - # upstream image change, which at the control plane's release cadence is - # days, and neither can break a running edge — the pods stay offline-safe + # ONE KNOWN, BOUNDED LIMITATION of keeping the annotation (rather than the + # live pod spec) as the DIGEST source of truth. It is self-healing at the + # next upstream image change, which at the control plane's release cadence is + # days, and cannot break a running edge — the pods stay offline-safe # throughout because IfNotPresent does not depend on any of this. # - # 1. HELM RE-RENDER. `helm upgrade --reset-then-reuse-values` (the fleet - # auto-upgrade path) re-renders the templates, which write `repo:tag` - # and so revert an earlier `set image` pin. This tick will NOT re-pin: - # the annotation still records that digest, so `recorded == latest` - # and the loop no-ops. The edge floats on the tag until the next - # upstream release re-pins it. Note this only happens on a chart - # VERSION bump — auto-upgrade compares versions and skips otherwise — - # so it is not an hourly revert. + # PRE-EXISTING SKEW. requests-proxy and jobs-manager may already be + # running different builds of the same image on an edge upgrading INTO + # this version, because nothing reconciled requests-proxy before now. + # This script converges them on the next digest change (both are set + # in the same tick); it does not detect and repair skew that predates + # it, because it compares registry-vs-annotation, never pod-vs-pod. # - # 2. PRE-EXISTING SKEW. requests-proxy and jobs-manager may already be - # running different builds of the same image on an edge upgrading INTO - # this version, because nothing reconciled requests-proxy before now. - # This script converges them on the next digest change (both are set - # in the same tick); it does not detect and repair skew that predates - # it, because it compares registry-vs-annotation, never pod-vs-pod. + # HELM RE-RENDER, by contrast, is HANDLED rather than tolerated (client-runtime#199). + # `helm upgrade --reset-then-reuse-values` (the fleet auto-upgrade path) + # re-renders the templates, which write `repo:tag` and so revert an earlier + # `set image` pin. `recorded == latest` no longer means no-op: the loop reads + # each workload's LIVE image and re-pins the digest whenever the workload is + # off it. The revert therefore lasts ONE tick — the next tick puts the digest + # back — instead of floating on the tag until the next upstream release. It + # is compared on the @sha256 digest, so a registry-prefix rewrite (a mutating + # webhook) is not mistaken for a revert. This only happens on a chart VERSION + # bump anyway — auto-upgrade compares versions and skips otherwise — so it is + # not an hourly revert. # - # Fixing either properly means reconciling against each workload's LIVE - # container image instead of a shared annotation — a declarative reconcile, - # which `set image` makes possible for the first time (`rollout restart` - # was a blind action, which is why the annotation existed at all). That is - # a deliberate follow-up, not an oversight. + # Reconciling against the live spec is what makes that possible — the + # declarative reconcile `set image` enables (`rollout restart` was a blind + # action, which is why the annotation existed at all). The remaining + # pre-existing-skew case would need pod-vs-pod comparison; that is a + # deliberate follow-up, not an oversight. # # Source of truth: annotations on the JOBS-MANAGER deployment metadata # (`tracebloc.io/last-refreshed--digest`) — comparing the registry @@ -91,14 +94,29 @@ data: # # First-tick contract: annotation missing → record the current registry # digest WITHOUT touching the workload (no evidence of drift, no reason to - # churn pods). #569 keeps this deliberately. Pinning on the first tick + # churn pods). #569 keeps this deliberately. Pinning on the FIRST tick # would rewrite `repo:tag` to `repo@digest` on every fresh install — a spec # change, therefore a rollout, for byte-identical content, and for the - # resource-monitor DaemonSet that is a rollout across every node. The cost - # is that a freshly installed edge runs `repo:tag` until the first real - # digest change: still restart-safe offline (IfNotPresent), just not yet - # reproducible. Offline-safety is what #569 is fixing; reproducibility - # follows on the next upstream release. + # resource-monitor DaemonSet that is a rollout across every node. So the + # first tick only RECORDS. The NEXT tick, seeing `recorded == latest` but the + # workload still on `:tag`, pins the digest (client-runtime#199) — so a fresh edge + # becomes reproducible ~one interval post-install, NOT "at the next upstream + # release". Between the two it is still restart-safe offline (IfNotPresent). + # + # COST, stated honestly (@shujaatTracebloc / @LukasWodka on #1008): that + # re-pin is not "one cheap rollout". It enters the shared #563 flap path — + # `rollout status` on the resource-monitor DaemonSet, whose + # `desiredNumberScheduled` counts every node (tolerations: Exists), so it can + # never settle on a fleet with one NotReady/cordoned node; three such ticks + # (~45 min at the default 15m schedule) latch the SHARED MAX_REFRESH_ATTEMPTS + # lockout and stop refresh for ALL control-plane images until a human clears + # ATTEMPT_KEY, while the CronJob stays green. And jobs-manager is + # `strategy: Recreate`, so its extra rollout is full downtime + wait-for-mysql + # for byte-identical content, on every fresh install and again after each + # chart-version bump. The widening is kept deliberately — it also repairs a + # reinstall onto a node whose `:tag` layer is already stale — but that is the + # price, and a follow-up may gate the re-pin on "have we ever applied a digest + # here?" so a genuine fresh install skips the flap path entirely. # # Parsing: awk/sed/grep + jq. jq used only where JSON-with-dotted-keys # or container/env-array filtering motivates it; the rest stays in pure @@ -300,6 +318,55 @@ data: printf '%s\n' "$_json" | jq -r --arg k "$_key" '.metadata.annotations[$k] // empty' } + # The image reference the LIVE workload currently runs for $repo's primary + # container. Used to detect when `helm upgrade --reset-then-reuse-values` + # (the auto-upgrade) has re-rendered the workload back to the + # chart's `repo:tag` and so DISCARDED an earlier `set image repo@digest` + # pin. That revert is invisible to the digest comparison below -- the + # annotation still equals the registry digest, so `recorded == latest` + # reads "unchanged" and never re-pins -- while the workload sits on the + # bare tag (IfNotPresent per tracebloc.controlPlanePullPolicy). On a node + # whose `:tag` layer is stale that silently runs an OLD control-plane image + # (backend#2896-adjacent; it ran a pre-#416 jobs-manager under a sealed + # egress netpol on the stg/prod fleets, client-runtime#199). Container + # names are contractual with the deployment/daemonset templates -- keep in + # sync with the `case` block in the reconcile loop below. An empty result + # (read error / container absent) makes the caller SKIP the re-pin this tick + # and retry -- NOT re-assert (which would burn a #563 flap attempt on a + # healthy edge) and NOT assume agreement. That is the fail-closed stance + # get_annotation and the settled guard already take. + workload_image_for_repo() { + case "$1" in + tracebloc/jobs-manager) + kubectl get deployment -n "$RELEASE_NAMESPACE" "$DEPLOYMENT_NAME" \ + -o jsonpath='{.spec.template.spec.containers[?(@.name=="api")].image}' \ + --request-timeout=15s 2>/dev/null ;; + tracebloc/pods-monitor) + kubectl get deployment -n "$RELEASE_NAMESPACE" "$DEPLOYMENT_NAME" \ + -o jsonpath='{.spec.template.spec.containers[?(@.name=="pods-monitor-container")].image}' \ + --request-timeout=15s 2>/dev/null ;; + tracebloc/resource-monitor) + kubectl get daemonset -n "$NODE_AGENTS_NAMESPACE" "$RESOURCE_MONITOR_DAEMONSET" \ + -o jsonpath='{.spec.template.spec.containers[?(@.name=="tracebloc-resource-monitor")].image}' \ + --request-timeout=15s 2>/dev/null ;; + esac + } + + # The requests-proxy is a SEPARATE deployment that runs the SAME + # tracebloc/jobs-manager image (container `proxy`). `workload_image_for_repo` + # above reads only the jobs-manager `api` container, so the no-op decision + # below cannot see the proxy on its own. Read it here so a proxy left on + # `:tag` -- a tick that pinned `api` then died before the rp rollout, or a + # helm re-render that reverted only the proxy -- is still re-pinned instead + # of being declared "unchanged" forever because `api` happens to match + # (Bugbot on #1008). Empty (read error / absent) makes the caller SKIP this + # tick, same fail-closed stance as `workload_image_for_repo`. + requests_proxy_image() { + kubectl get deployment -n "$RELEASE_NAMESPACE" "$REQUESTS_PROXY_DEPLOYMENT" \ + -o jsonpath='{.spec.template.spec.containers[?(@.name=="proxy")].image}' \ + --request-timeout=15s 2>/dev/null + } + # Skip the whole tick if the deployment isn't currently SETTLED (#546). A rollout # already in progress, or a pod stuck (e.g. Pending on volume binding), means a restart # can't help — it only churns ReplicaSets, and on a single-node local-path cluster that @@ -409,6 +476,23 @@ data: rp_set_args="" rm_set_args="" + # An UNFINISHED re-image attempt (client-runtime#199, Bugbot High on #1008). The + # restart block below increments ATTEMPT_KEY BEFORE the rollout and only + # resets it on a settled one; a rollout that times out exits the tick under + # set -e with the counter still raised. `kubectl set image` has by then + # updated the SPEC to repo@digest, so the live-image check reads "on digest" + # even though the rollout never completed -- and requests-proxy / + # resource-monitor sit OUTSIDE the top-of-tick settled guard, so a stuck + # rollout on either would no-op here forever, the counter raised and stale + # :tag pods still running. Read the counter once up front: a raised value + # forces the no-op branch to re-enter the re-image path so `rollout status` + # is retried -- resolving it (success resets the counter) or advancing it to + # the #563 flap lockout, which SURFACES the stuck rollout rather than hiding + # it. Best-effort: an unreadable/absent counter is treated as 0 (no forced + # retry), since the restart block's own read is the fail-closed authority. + pending_attempt="$(get_annotation "$ATTEMPT_KEY" || true)" + case "$pending_attempt" in ''|*[!0-9]*) pending_attempt=0 ;; esac + # Each entry: "|||". set -- \ "tracebloc/jobs-manager|tracebloc.io/last-refreshed-jobs-manager-digest|${JOBS_MANAGER_PINNED}|${JOBS_MANAGER_PIN:-}" \ @@ -525,11 +609,120 @@ data: fi if [ "$recorded" = "$latest" ]; then - log " digest unchanged since last refresh; no-op" - continue + # The registry digest has not moved since we recorded it -- but that + # alone does NOT prove the workload is running it. A `helm upgrade + # --reset-then-reuse-values` (the auto-upgrade) re-renders the + # Deployment back to `repo:tag` and discards our `set image repo@digest` + # pin; with `recorded == latest` this used to no-op, leaving the workload + # on the bare tag until the NEXT registry publish -- and on a node whose + # `:tag` layer is stale that silently runs an OLD image (client-runtime#199: + # a pre-#416 jobs-manager under a sealed egress netpol). So re-assert the + # pin whenever the live workload is not already on the pinned digest. + # + # The re-pin is BOUNDED TO ONE TICK: it writes the digest back and the + # next tick sees the workload on it and no-ops. It also converges a FRESH + # install to the digest one tick after first observation -- the first-tick + # contract in the header records without re-imaging, and this completes + # it, because staying on `:tag` is exactly the steady-state stale-`:tag` + # exposure this fix closes. + # + # Compare on the @sha256 DIGEST, not the whole image reference: a mutating + # admission webhook that rewrites the registry PREFIX to an internal mirror + # (seen behind hospital proxies) keeps the digest, so a prefix-only rewrite + # must NOT read as a revert -- otherwise the ref never equals the pinned one, + # every tick re-pins, the webhook rewrites it again, and three ticks trip the + # #563 flap lockout for ALL control-plane images (LukasWodka on #1008). A + # genuine revert to `:tag` carries no `@sha256` suffix, so `${ref##*@}` (the + # digest for `repo@sha256:...`, the whole ref otherwise) still mismatches a + # bare `sha256:...` and re-pins. + have="$(workload_image_for_repo "$repo" || true)" + # jobs-manager: the requests-proxy runs this SAME image as its own + # deployment. When it follows this digest (not operator-pinned) it must + # ALSO be on it, or a partial re-pin (api pinned, proxy still on :tag) is + # declared no-op forever off the api match alone and never retried (Bugbot + # on #1008). Any mismatch falls through to the re-image path, whose `case` + # block re-derives BOTH `jm_set_args`/`rp_set_args`. + proxy_follows=0 + rp_have="" + if [ "$repo" = "tracebloc/jobs-manager" ] && [ "$REQUESTS_PROXY_PINNED" != "1" ]; then + proxy_follows=1 + rp_have="$(requests_proxy_image || true)" + fi + # Unreadable live image (read error / container absent): SKIP the re-pin + # this tick and retry, rather than re-assert. Re-asserting on an unreadable + # read would burn a #563 flap attempt on a possibly-healthy edge and log a + # revert that may not have happened -- the same fail-closed stance the + # SKIP_KEY read and the settled guard take. A real `:tag` ref is readable + # and is NOT this case; it falls through and re-pins. + if [ -z "$have" ]; then + log " digest unchanged, but the live ${repo} image is unreadable (API read error / container absent) -- skipping re-pin this tick, will retry when readable" + continue + fi + if [ "$proxy_follows" = "1" ] && [ -z "$rp_have" ]; then + log " digest unchanged, but the live requests-proxy image is unreadable (API read error / container absent) -- skipping re-pin this tick, will retry when readable" + continue + fi + api_on_digest=1 + [ "${have##*@}" = "$latest" ] || api_on_digest=0 + proxy_on_digest=1 + if [ "$proxy_follows" = "1" ]; then + [ "${rp_have##*@}" = "$latest" ] || proxy_on_digest=0 + fi + if [ "$api_on_digest" = "1" ] && [ "$proxy_on_digest" = "1" ]; then + if [ "$pending_attempt" -gt 0 ] && [ "$pending_attempt" -lt "$MAX_REFRESH_ATTEMPTS" ]; then + # Only re-enter the rollout while there is budget left to resolve it. + # Once ATTEMPT_KEY has reached MAX_REFRESH_ATTEMPTS the flap guard below + # annotates FLAP_KEY and `exit 0`s BEFORE any `set image`/`rollout status` + # runs, so a latched forced-retry resolves nothing and surfaces nothing -- + # it only forces restart_needed=1 and skips the rest of the tick, dropping + # the annotation write (first-observation records, stale-pin clears) every + # tick, forever. Gating on `< MAX` lets a latched image fall to the no-op + # branch so the tick completes and its annotations land (@shujaatTracebloc + # on #1008, blocking 1 & 2). + # + # The SPEC reads on-digest, but a prior re-image attempt never reached + # its success-reset (ATTEMPT_KEY is raised): its rollout timed out and + # `set image` had already moved the spec, so this "on digest" can be a + # rollout that never settled -- and for requests-proxy / resource-monitor + # nothing else would catch it (they are outside the settled guard). + # Re-enter the re-image path so `rollout status` runs again: a settled + # workload resets the counter, a stuck one advances it to the #563 flap + # lockout, which surfaces it (Bugbot High on #1008). Re-`set image` with + # the same ref is an idempotent no-op patch, so a genuinely-settled + # workload pays only one fast `rollout status`. + log " workload spec is on the pinned digest, but ATTEMPT_KEY=${pending_attempt} marks an unfinished re-image (a rollout that never settled) -- re-running the rollout to resolve it or surface it via the flap guard" + else + # Latched on-digest: the spec is on the pinned digest but a prior + # re-image never reset ATTEMPT_KEY, and the `< MAX` gate above now + # keeps restart_needed=0 so the flap guard below (the only other + # writer of FLAP_KEY / the MANUAL ATTENTION WARN) never runs on this + # tick. Surface the latch HERE, mirroring that guard, so a + # stopped-and-silent refresh is never inferable only from the + # CronJob's green (#1964): without this the tick would log a bare + # "no-op" on the exact tick refresh is dead for ALL control-plane + # images (@shujaatTracebloc / @LukasWodka / @saadqbal on #1008). + if [ "$pending_attempt" -ge "$MAX_REFRESH_ATTEMPTS" ]; then + log " WARN: workload is on the pinned digest but ${ATTEMPT_KEY}=${pending_attempt} (>= MAX_REFRESH_ATTEMPTS=${MAX_REFRESH_ATTEMPTS}) -- FLAP LATCHED: image refresh is STOPPED for ALL control-plane images and does not auto-resume. MANUAL ATTENTION NEEDED: clear the ${ATTEMPT_KEY} annotation on deployment/${DEPLOYMENT_NAME} to re-arm refresh." + kubectl annotate deployment -n "$RELEASE_NAMESPACE" "$DEPLOYMENT_NAME" \ + "${FLAP_KEY}=${pending_attempt}" --overwrite --request-timeout=15s + fi + log " digest unchanged and workload already on the pinned digest; no-op" + continue + fi + elif [ "$api_on_digest" = "1" ]; then + log " digest unchanged and jobs-manager already on the pinned digest, but" + log " deployment/${REQUESTS_PROXY_DEPLOYMENT} runs '${rp_have}', not digest ${latest}" + log " -- re-pinning the requests-proxy digest" + else + log " digest unchanged, but the workload runs '${have}', not digest ${latest}" + log " (fresh install, or a helm re-render reverted the pin onto :${IMAGE_TAG}) -- re-pinning the digest" + fi + # Fall through to the re-image path (ref + case block) with recorded + # already == latest: the annotate below is an idempotent re-write, and + # restart_needed drives the rollout that puts the digest back on. + else + log " digest changed (${recorded} -> ${latest}); re-image needed" fi - - log " digest changed (${recorded} -> ${latest}); re-image needed" annotate_args="$annotate_args ${key}=${latest}" restart_needed=1 diff --git a/client/tests/image_refresh_test.yaml b/client/tests/image_refresh_test.yaml index 0e48630a..3787fbc7 100644 --- a/client/tests/image_refresh_test.yaml +++ b/client/tests/image_refresh_test.yaml @@ -223,6 +223,21 @@ tests: - matchRegex: path: data["image-refresh.sh"] pattern: "pinned by digest in values" + # Regression guard (Bugbot #1008): the requests-proxy is a SEPARATE + # deployment running the SAME jobs-manager image, so the no-op "already on + # the pinned digest" decision MUST also read the proxy and fall through + # when it is off the digest -- else a partial re-pin (api pinned, proxy + # still on :tag) is declared unchanged forever off the api match alone and + # never retried. Lock the reader and the per-workload digest check in + # place. (The BEHAVIOUR -- that an inverted check reddens -- is asserted in + # scripts/tests/image-refresh-repin-on-revert.bats; this only pins that the + # two pieces still exist in the shipped script.) + - matchRegex: + path: data["image-refresh.sh"] + pattern: 'requests_proxy_image\(\)' + - matchRegex: + path: data["image-refresh.sh"] + pattern: 'proxy_on_digest' # Regression guard: the script must HEAD the manifest with all # four Accept media types in a SINGLE comma-separated Accept # header per the Docker registry v2 spec (some proxies have been @@ -935,3 +950,23 @@ tests: - matchRegex: path: data["image-refresh.sh"] pattern: 'rm_set_args tracebloc-resource-monitor=' + + - it: reconcile re-pins when a helm re-render reverted the workload off the digest + # Guards the client-runtime#199 fix: with `recorded == latest` the loop must + # NOT unconditionally no-op -- it must read the live workload image and + # re-pin when it is not `repo@latest` (a `helm upgrade --reset-then-reuse-values` + # reverted the pin onto the bare :tag, where a stale node cache serves an old + # image). The helper + the fall-through into the re-image path are the fix. + template: templates/image-refresh-cronjob.yaml + documentIndex: 0 + asserts: + - matchRegex: + path: data["image-refresh.sh"] + pattern: 'workload_image_for_repo\(\)' + - matchRegex: + path: data["image-refresh.sh"] + pattern: 'have="\$\(workload_image_for_repo "\$repo"' + # the true no-op now requires BOTH digest-unchanged AND workload-on-digest + - matchRegex: + path: data["image-refresh.sh"] + pattern: 'workload already on the pinned digest; no-op' diff --git a/client/values.yaml b/client/values.yaml index 4dbebf55..564c0962 100644 --- a/client/values.yaml +++ b/client/values.yaml @@ -1718,11 +1718,16 @@ autoUpgrade: # - First observation (annotation absent on a fresh install): record # the current digest without re-imaging. Rewriting repo:tag to # repo@digest for byte-identical content would roll every workload — -# including the DaemonSet on every node — for nothing. The cost is -# that a fresh edge runs repo:tag until the first real digest change: -# still restart-safe offline, just not yet reproducible. -# - Idle-cheap: when the recorded digest matches today's digest, the -# script exits without touching anything. Steady state is one HEAD +# including the DaemonSet on every node — for nothing. The NEXT tick, +# seeing the digest recorded but the workload still on repo:tag, pins +# it (client-runtime#199) — so a fresh edge becomes reproducible ~one interval +# post-install, not at the next upstream release. Restart-safe offline +# throughout. +# - Idle-cheap: when the recorded digest matches today's digest AND the +# workload already runs that digest, the script exits without touching +# anything. If a helm re-render reverted the pin back to repo:tag it +# re-pins that one tick (compared on the @sha256 digest, so a mirror +# prefix rewrite is not mistaken for a revert). Steady state is one HEAD # per image per tick, well under Docker Hub's 100/6h anonymous # pull-rate limit. # - Private mirrors (global.imageRegistry): the script resolves digests diff --git a/scripts/lib/cluster.sh b/scripts/lib/cluster.sh index 079b2a6d..3cd8a432 100755 --- a/scripts/lib/cluster.sh +++ b/scripts/lib/cluster.sh @@ -1188,13 +1188,26 @@ create_cluster() { ensure_cluster_autostart() { if [[ -n "${TRACEBLOC_NO_AUTOSTART:-}" ]]; then return 0; fi - local nodes node + local nodes node _nodes_rc=0 # BOUNDED (client#984, LukasWodka): this is a daemon read on the main install # path, and it ran unbounded while its `docker info` neighbours did not — the gap - # check-style rule 5 could not see until it was widened past `info`. `|| return 0` - # already treats an unreadable engine as "nothing to autostart", so a 124 lands in - # the branch this function was written for. - nodes=$(_bounded "${TB_DOCKER_PROBE_TIMEOUT:-10}" docker ps -a --filter "name=k3d-${CLUSTER_NAME}-" --format '{{.Names}}' 2>/dev/null) || return 0 + # check-style rule 5 could not see until it was widened past `info`. + # + # DO NOT `|| return 0` here (Bugbot Medium, off the client#1011 promotion + # review): this read feeds ONLY the node restart-policy loop below, but the Linux + # docker.service boot-enable further down does NOT depend on the node list. + # Bailing out of the whole function on a 124 left the operator a finished + # install whose docker.service was never enabled on boot — the cluster would + # not come back after a reboot, with no warning. A failed/timed-out read means + # "we couldn't enumerate nodes", so skip the loop (k3d already sets + # --restart unless-stopped at create time, so the policy still holds — the same + # rationale the Windows twin Set-ClusterAutostart states) and fall through to + # the boot-enable step. + nodes=$(_bounded "${TB_DOCKER_PROBE_TIMEOUT:-10}" docker ps -a --filter "name=k3d-${CLUSTER_NAME}-" --format '{{.Names}}' 2>/dev/null) || _nodes_rc=$? + if [[ "$_nodes_rc" -ne 0 ]]; then + nodes="" + log "Could not read k3d nodes for the restart policy (docker ps exit ${_nodes_rc}); leaving k3d's own --restart policy in place and continuing to the boot-enable step." + fi if [[ -n "$nodes" ]]; then for node in $nodes; do docker update --restart unless-stopped "$node" >/dev/null 2>&1 || true diff --git a/scripts/manifest.sha256 b/scripts/manifest.sha256 index c4f30bde..9f3572bc 100644 --- a/scripts/manifest.sha256 +++ b/scripts/manifest.sha256 @@ -7,7 +7,7 @@ a61f5bac3786a3283b5fa08fea9e522f7cea5f7fa44799a2b38b5caea42469d8 scripts/lib/de b569eec2d8ffb9673da287a2a59d249a7dbc7236c98ab6a5062136bcc69a942c scripts/lib/gpu-amd.sh 95209eeca22917db32e3f352af3f394773978709c73f474d1694c0a42dedc7de scripts/lib/setup-macos.sh d9a372308bf53b25fb39b404bd78cf044563425e3eda750f04583d517114a8ea scripts/lib/setup-linux.sh -e1737a4a7d76bb871e07937b1149ea187baf7185977fde4011cd585ed2c7c1b2 scripts/lib/cluster.sh +669274f425058421ba6346d672d5ee339ba3faac33197dd6af3cc15d9f17b54c scripts/lib/cluster.sh 84ed9d9b3ab4633bfaf07b256c066ed43f96a0025ec6b1a34db23fdef75f0f62 scripts/lib/gpu-plugins.sh 320a3d04d7127849c92d372d5c7942f86f19256a4ea8e2f0afe48b3a5149c53a scripts/lib/install-client-helm.sh 1b3e11d06e4be983ec5cecd8f55b16034b76bdb0476f3e141c025d383ddc043a scripts/lib/install-cli.sh diff --git a/scripts/tests/bounded-reads-propagate.bats b/scripts/tests/bounded-reads-propagate.bats index e96ed53d..724bd14b 100644 --- a/scripts/tests/bounded-reads-propagate.bats +++ b/scripts/tests/bounded-reads-propagate.bats @@ -102,7 +102,10 @@ _bounded_capture_read # Why each is out of scope rather than wrong: every one is a best-effort read # whose timeout branch is already a `|| return 0` / `|| true` no-op on a path # that reconciles anyway (the _check_existing_cluster_* drift probes, - # _generate_node_cdi_specs, ensure_cluster_autostart), a yes/no liveness probe + # _generate_node_cdi_specs), or falls through logging the skip without ever + # claiming a machine state it could not read (ensure_cluster_autostart, whose + # timed-out node read now skips only the restart-policy loop and still runs the + # boot-enable), a yes/no liveness probe # that is already tri-state or has no third state to lose (_docker_answers, # _k3d_cluster_running, _assess_runtime_down, _docker_default_runtime_is_nvidia), # or a preflight/install step that reports its own failure to the operator diff --git a/scripts/tests/cluster.bats b/scripts/tests/cluster.bats index 8447dba5..600dbb65 100644 --- a/scripts/tests/cluster.bats +++ b/scripts/tests/cluster.bats @@ -932,6 +932,26 @@ _cc_mocks() { # $1 = "real-handle" to leave _handle_existing_cluster UNstubbed [[ "$output" != *"docker update"* ]] || return 1 } +# A TIMED-OUT nodes read must skip only the node restart-policy loop, NOT abandon +# the Linux docker.service boot-enable below it — that step does not depend on the +# node list, and bailing out left a finished install whose cluster never came back +# after a reboot, with no warning (Bugbot Medium, off the client#1011 promotion review). +@test "ensure_cluster_autostart: nodes read times out -> still enables docker.service, skips node loop, logs" { + OS=Linux + LOG_FILE="$BATS_TEST_TMPDIR/autostart.log" + _bounded() { return 124; } # docker ps -a (nodes read) times out + docker() { record "docker $*"; } # any docker update would be recorded + sudo() { record "sudo $*"; } + systemctl() { record "systemctl $*"; return 1; } # not already enabled on boot + has() { return 0; } + run ensure_cluster_autostart + [ "$status" -eq 0 ] || return 1 + run mock_calls + [[ "$output" != *"docker update"* ]] || return 1 # node loop skipped on the timeout + [[ "$output" == *"sudo systemctl enable docker"* ]] || return 1 # INDEPENDENT boot-enable still ran + grep -q "leaving k3d's own --restart policy in place" "$LOG_FILE" || return 1 # logged to LOG_FILE, not silent +} + # ── bounded create (#426) ──────────────────────────────────────────────────── @test "k3d create is bounded: --wait always pairs with --timeout (#426)" { grep -q -- '--wait --timeout' "$BATS_TEST_DIRNAME/../lib/cluster.sh" diff --git a/scripts/tests/customer-copy-no-ticket-refs.bats b/scripts/tests/customer-copy-no-ticket-refs.bats index 80d55478..0aabf58a 100644 --- a/scripts/tests/customer-copy-no-ticket-refs.bats +++ b/scripts/tests/customer-copy-no-ticket-refs.bats @@ -348,3 +348,140 @@ FX [ "$status" -eq 2 ] || { echo "$output"; return 1; } [[ "$output" == *"derived ZERO copy-emitting bash helpers"* ]] || { echo "$output"; return 1; } } + +# --- second review round (client#1020): escapes, here-document text, one lexer --- + +@test "derivation: an escaped quote around an unbalanced brace inside a string does not move the depth (bash)" { + # Old walker: `\"` toggled quote state, so the `{` counted as a real brace and the + # first helper never balanced (guard error), while a `}` closed the second early. + printf 'say_escaped() {\n echo "open \\"{ deeper\\" now"\n}\nsay_escaped_close() {\n echo "close \\"} early\\" now"\n echo "$*"\n}\nafter_escaped() { echo "$*"; }\n' >> "$WORK/scripts/lib/cluster.sh" + grep -qF 'echo "open \"{ deeper\" now"' "$WORK/scripts/lib/cluster.sh" || return 1 # anchor applied + plant scripts/lib/cluster.sh 'after_escaped "planted after an escaped quote (backend#15)"' + run run_guard "$WORK" --print-vocab bash + [ "$status" -eq 0 ] || { echo "$output"; return 1; } + for fn in say_escaped say_escaped_close after_escaped; do + grep -qx "$fn" <<<"$output" || { echo "missing $fn"; echo "$output"; return 1; } + done + run run_guard + [ "$status" -eq 1 ] || { echo "$output"; return 1; } + [[ "$output" == *"planted after an escaped quote (backend#15)"* ]] || { echo "$output"; return 1; } +} + +@test "derivation: a backtick-escaped or doubled quote around a brace does not move the depth (PowerShell)" { + printf 'function Say-Escaped {\n Write-Host "he said `"go { deeper`" now"\n}\nfunction Say-Doubled {\n Write-Host "he said ""go } early"" now"\n}\nfunction After-Escaped($m) { Write-Host $m }\n' >> "$WORK/scripts/install-k8s.ps1" + grep -qF 'Write-Host "he said `"go { deeper`" now"' "$WORK/scripts/install-k8s.ps1" || return 1 # anchor applied + plant scripts/install-k8s.ps1 'After-Escaped "planted after an escaped quote (RFC-9908)"' + run run_guard "$WORK" --print-vocab ps + [ "$status" -eq 0 ] || { echo "$output"; return 1; } + for fn in Say-Escaped Say-Doubled After-Escaped; do + grep -qx "$fn" <<<"$output" || { echo "missing $fn"; echo "$output"; return 1; } + done + run run_guard + [ "$status" -eq 1 ] || { echo "$output"; return 1; } + [[ "$output" == *"planted after an escaped quote (RFC-9908)"* ]] || { echo "$output"; return 1; } +} + +@test "mutation: a # inside a PRINTED here-document body is text the customer reads, not a comment" { + printf "help_hash() {\n cat <<'HELP'\n # migration required, see backend#16\n Some line # tracked in backend#17\nHELP\n}\n" >> "$WORK/scripts/lib/cluster.sh" + grep -q 'tracked in backend#17' "$WORK/scripts/lib/cluster.sh" || return 1 # anchor applied + run run_guard + [ "$status" -eq 1 ] || { echo "$output"; return 1; } + [[ "$output" == *"migration required, see backend#16"* ]] || { echo "$output"; return 1; } + [[ "$output" == *"tracked in backend#17"* ]] || { echo "$output"; return 1; } + [[ "$output" == *"2 user-visible line(s)"* ]] || { echo "$output"; return 1; } +} + +@test "a # inside a here-document that GENERATES A FILE is that file's comment; its other lines are still copy" { + # The values.yaml the installer writes carries the rationale for its defaults + # as YAML comments -- out of scope like every other comment. + printf 'write_values() {\n cat < "$1"\n# rationale for this default (backend#19)\nreplicas: 1 # see RFC-9909\nEOF\n}\n' >> "$WORK/scripts/lib/cluster.sh" + grep -q 'rationale for this default (backend#19)' "$WORK/scripts/lib/cluster.sh" || return 1 # anchor applied + run run_guard + [ "$status" -eq 0 ] || { echo "$output"; return 1; } + # ...but a non-comment line of the generated file is text the customer can open. + printf 'write_values_token() {\n cat < "$1"\nnote: planted in a generated file (backend#20)\nEOF\n}\n' >> "$WORK/scripts/lib/cluster.sh" + run run_guard + [ "$status" -eq 1 ] || { echo "$output"; return 1; } + [[ "$output" == *"planted in a generated file (backend#20)"* ]] || { echo "$output"; return 1; } + [[ "$output" != *"backend#19"* ]] || { echo "$output"; return 1; } +} + +@test "a # inside an ASSIGNED PowerShell here-string is that file's comment; its other lines are still copy" { + printf 'function Write-Values {\n $values = @"\n# rationale for this default (backend#21)\nreplicas: 1\n"@\n Set-Content -Path $p -Value $values\n}\n' >> "$WORK/scripts/install-k8s.ps1" + grep -q 'rationale for this default (backend#21)' "$WORK/scripts/install-k8s.ps1" || return 1 # anchor applied + run run_guard + [ "$status" -eq 0 ] || { echo "$output"; return 1; } + printf 'function Write-Values-Token {\n $values = @"\nnote: planted in an assigned here-string (backend#23)\n"@\n}\n' >> "$WORK/scripts/install-k8s.ps1" + run run_guard + [ "$status" -eq 1 ] || { echo "$output"; return 1; } + [[ "$output" == *"planted in an assigned here-string (backend#23)"* ]] || { echo "$output"; return 1; } +} + +@test "mutation: the org's RFC-- form with three digits is caught" { + plant scripts/lib/cluster.sh 'warn "planted short rfc (RFC-BACKEND-664)"' + run run_guard + [ "$status" -eq 1 ] || { echo "$output"; return 1; } + [[ "$output" == *"RFC-BACKEND-664"* ]] || { echo "$output"; return 1; } +} + +@test "a here-document body line that starts with an emitter word is reported once, not twice" { + printf "help_once() {\n cat <<'HELP'\n echo is what this prints, see backend#18\nHELP\n}\n" >> "$WORK/scripts/lib/cluster.sh" + grep -q 'echo is what this prints, see backend#18' "$WORK/scripts/lib/cluster.sh" || return 1 # anchor applied + run run_guard + [ "$status" -eq 1 ] || { echo "$output"; return 1; } + [ "$(grep -c 'backend#18' <<<"$output")" -eq 1 ] || { echo "$output"; return 1; } + [[ "$output" == *"1 user-visible line(s)"* ]] || { echo "$output"; return 1; } +} + +@test "derivation: a PowerShell here-string closed by \"@.Trim() does not swallow every later function" { + # The old closer rule wanted the closer ALONE on its line, so `"@.Trim()` in + # install-k8s.ps1 left the here-string open for ~2,750 lines and hid eleven + # emitting helpers from the vocabulary (a silent miss). + printf 'function Get-Script {\n return @"\necho hi\n"@.Trim()\n}\nfunction After-Trim($m) { Write-Host $m }\n' >> "$WORK/scripts/install-k8s.ps1" + grep -qF '"@.Trim()' "$WORK/scripts/install-k8s.ps1" || return 1 # anchor applied + plant scripts/install-k8s.ps1 'After-Trim "planted after a trimmed here-string (RFC-9910)"' + run run_guard "$WORK" --print-vocab ps + [ "$status" -eq 0 ] || { echo "$output"; return 1; } + grep -qx After-Trim <<<"$output" || { echo "$output"; return 1; } + ! grep -qx Get-Script <<<"$output" || { echo "Get-Script emits nothing"; echo "$output"; return 1; } + run run_guard + [ "$status" -eq 1 ] || { echo "$output"; return 1; } + [[ "$output" == *"planted after a trimmed here-string (RFC-9910)"* ]] || { echo "$output"; return 1; } +} + +@test "one lexer: the quote/comment walk and every here-document rule (open AND close) are defined once" { + # The census names each shared rule, opener and closer alike: the closer is the + # half that was wrong before (a second, anchored closer rule left ~2,750 lines + # of install-k8s.ps1 as here-string body), so a second copy of it must redden + # this test too (Bugbot on client#1022). + for fn in lex code_only heredoc_delim herestring_closer closes; do + [ "$(grep -c "function $fn(" "$GUARD")" -eq 1 ] || { echo "$fn defined $(grep -c "function $fn(" "$GUARD") times"; return 1; } + done + [ "$(grep -c 'sub(/\.\*<<-?' "$GUARD")" -eq 1 ] || return 1 + # ...and the closer TEST is spelled exactly once, inside closes(): a state + # machine re-spelling `"^[ \t]*" closer` inline is a second closer rule. + [ "$(grep -cE '"\^\[ \\t\]\*" (heredoc|closer)' "$GUARD")" -eq 1 ] || { echo "an inline closer regex exists outside closes()"; return 1; } +} + +# --- Bugbot round on the follow-up: redirect classification, scratch dir ------ + +@test "a printed here-document with a stderr or /dev redirect is still text, not a generated file" { + printf "help_quiet() {\n cat <<'HELP' 2>/dev/null\n # see backend#24 before upgrading\nHELP\n}\nhelp_err() {\n cat <<'HELP' >/dev/stderr\n # see backend#25 before upgrading\nHELP\n}\nhelp_fd() {\n cat <<'HELP' >&2\n # see backend#26 before upgrading\nHELP\n}\n" >> "$WORK/scripts/lib/cluster.sh" + grep -q 'see backend#26 before upgrading' "$WORK/scripts/lib/cluster.sh" || return 1 # anchor applied + run run_guard + [ "$status" -eq 1 ] || { echo "$output"; return 1; } + for t in 'backend#24' 'backend#25' 'backend#26'; do + [[ "$output" == *"$t before upgrading"* ]] || { echo "missing $t"; echo "$output"; return 1; } + done + [[ "$output" == *"3 user-visible line(s)"* ]] || { echo "$output"; return 1; } + # ...while an explicit stdout-to-file redirect (`1>`) is a generated file. + printf 'write_one() {\n cat < "$1"\n# rationale (backend#27)\nEOF\n}\n' >> "$WORK/scripts/lib/cluster.sh" + run run_guard + [[ "$output" != *"backend#27"* ]] || { echo "$output"; return 1; } +} + +@test "fail closed: a scratch directory that cannot be created is a guard error, never a cleanup of /" { + TMPDIR="$WORK/does-not-exist" run run_guard + [ "$status" -eq 2 ] || { echo "$output"; return 1; } + [[ "$output" == *"could not create a scratch directory"* ]] || { echo "$output"; return 1; } +} diff --git a/scripts/tests/customer-copy-no-ticket-refs.sh b/scripts/tests/customer-copy-no-ticket-refs.sh index 8088ebf0..23903e9b 100755 --- a/scripts/tests/customer-copy-no-ticket-refs.sh +++ b/scripts/tests/customer-copy-no-ticket-refs.sh @@ -28,10 +28,16 @@ # plus those primitives themselves and PowerShell's `throw`. # # A line whose FIRST command word is in that vocabulary and which carries a -# `backend#` or `RFC-` token is an offender. A trailing `# comment` -# on such a line (whitespace, `#`, no quote after it) is stripped first: the -# customer does not see it. Known limit: a string continued onto a second -# line is not seen either — its first word is not an emitter. +# `#` or `RFC-[-]` token is an offender, and so is every line +# of a here-document / here-string body (help text, multi-line notices). A +# trailing `# comment` on a CODE line (whitespace, `#`, outside quotes) is +# stripped first: the customer does not see it. Inside a here-document body a +# `#` is text the customer reads, so nothing is stripped there -- unless the +# here-document GENERATES A FILE (redirected into one, or assigned to a +# variable): then its `#` lines are that file's comments, out of scope like +# every other comment, and only its non-comment lines are copy. Known limit: a +# string continued onto a second line is not seen — its first word is not an +# emitter. # # Usage: customer-copy-no-ticket-refs.sh [REPO_ROOT] [--print-vocab bash|ps] # Exit 0 = clean, 1 = offenders found, 2 = the guard itself could not check @@ -54,13 +60,75 @@ MANIFEST="$ROOT/scripts/manifest.sha256" BOOTSTRAPS="scripts/install.sh scripts/install.ps1" # Internal tracker identifiers, REPO-AGNOSTIC (Saqlain, client#1020): `client#564`, # `engine#972`, `e2e#459`, `.github#306`, `rfcs#80` are as internal as `backend#889`, -# and the org's RFC form is `RFC-BACKEND-0007` as well as `RFC-0001`. The leading -# character class keeps shell parameter expansion out of it: `${x#0}` is preceded -# by `{`, `$#` has no name, `${a[@]#1}` is preceded by `]`. -TOKEN_RE='(^|[^{[$A-Za-z0-9_.-])[A-Za-z.][A-Za-z0-9._-]*#[0-9]+|RFC-([A-Z]+-)?[0-9]{4}' +# and the org's RFC form is `RFC-BACKEND-0007` as well as `RFC-0001`. The RFC number +# is THREE OR MORE digits, not exactly four: `RFC-BACKEND-664` is a real one this +# repo cites (Saqlain, client#1020, second round). The leading character class +# keeps shell parameter expansion out of it: `${x#0}` is preceded by `{`, `$#` has +# no name, `${a[@]#1}` is preceded by `]`. +TOKEN_RE='(^|[^{[$A-Za-z0-9_.-])[A-Za-z.][A-Za-z0-9._-]*#[0-9]+|RFC-([A-Z]+-)?[0-9]{3,}' guard_error() { echo "[GUARD ERROR] $*" >&2; echo " 'could not check' is a finding, never 'clean'." >&2; exit 2; } +# ---- 0. the ONE lexer every awk program below includes ------------------------------ +# Three awk programs here walk quotes, comments and here-document openers: the +# vocabulary derivation, the here-document body lister and the comment stripper. +# They used to carry three verbatim copies of that walk, so a fix to one could +# silently miss the others (Saqlain, client#1020, second round). This string is +# prepended to each program; every function in it reads the -v LANG=bash|ps the +# caller passes. +# +# lex(line, keep) the code part of LINE: a trailing comment removed and, +# unless KEEP, string CONTENTS removed too (the quote +# marks stay). Escapes are honoured per language -- bash: +# a backslash escapes the next character outside single +# quotes; PowerShell: a backtick does, and a doubled +# quote inside a string is a literal quote -- so a brace +# inside `"he said \"go { deeper\" now"` stays inside the +# string instead of opening one (Saqlain, client#1020, +# second round: it used to abort the derivation, or +# close a helper early and hide every later one). +# code_only(line) lex(line, 0). +# heredoc_delim(code, raw) "" when CODE opens no here-document; else the +# delimiter, read from the RAW line because code_only +# has already emptied a QUOTED delimiter (<<'HELP' reads +# as two bare quote marks after stripping); "?" when the +# delimiter cannot be read. `(^|[^<])` keeps a here-STRING +# (`<<< "$a"`) from reading as a quoted here-document. +# herestring_closer(code) PowerShell: the closer of a here-string CODE opens +# (`"@` / `'@`), else "". +# closes(line, closer) does LINE end the open here-document / here-string? +# bash: the delimiter alone on its line; PowerShell: the +# closer at the start of the line (`"@.Trim()` closes). +AWK_LEX=' + function lex(line, keep, out, i, c, q, esc, n) { + out = ""; q = ""; esc = (LANG == "ps") ? "`" : "\\"; n = length(line) + for (i = 1; i <= n; i++) { + c = substr(line, i, 1) + if (q != "") { + if (c == esc && q == "\"") { if (keep) out = out c substr(line, i + 1, 1); i++; continue } + if (c == q && LANG == "ps" && substr(line, i + 1, 1) == q) { if (keep) out = out c c; i++; continue } + if (c == q) { q = ""; out = out c; continue } + if (keep) out = out c + continue + } + if (c == esc) { if (keep) out = out c substr(line, i + 1, 1); i++; continue } + if (c == "\"" || c == "\047") { q = c; out = out c; continue } + if (c == "#" && (i == 1 || substr(line, i - 1, 1) ~ /[ \t;]/)) break + out = out c + } + return out + } + function code_only(line) { return lex(line, 0) } + function heredoc_delim(code, raw, h, q2) { + if (code !~ /(^|[^<])<<-?[ \t]*([\047"]?[A-Za-z_]|[\047"][\047"])([^<]|$)/) return "" + h = raw; sub(/.*<<-?[ \t]*/, "", h); q2 = substr(h, 1, 1) + if (q2 == "\047" || q2 == "\"") { h = substr(h, 2); sub(q2 ".*$", "", h) } else { sub(/[^A-Za-z0-9_].*$/, "", h) } + return (h ~ /^[A-Za-z_][A-Za-z0-9_]*$/) ? h : "?" + } + function herestring_closer(code) { return (code ~ /@"[ \t]*$/) ? "\"@" : (code ~ /@\047[ \t]*$/) ? "\047@" : "" } + function closes(line, closer) { return (LANG == "ps") ? (line ~ ("^[ \t]*" closer)) : (line ~ ("^[ \t]*" closer "[ \t]*$")) } +' + # ---- 1. the shipped set -------------------------------------------------------- [ -r "$MANIFEST" ] || guard_error "cannot read $MANIFEST" shipped="" @@ -94,21 +162,8 @@ done # Write-Warning|Write-Error|Write-Output`. derive_emitters() { local lang="$1" file="$2" known="${3:-}" - awk -v LANG="$lang" -v FILE="$file" -v KNOWN="$known" ' + awk -v LANG="$lang" -v FILE="$file" -v KNOWN="$known" "$AWK_LEX"' function fail(msg) { failed = 1; printf("DERIVE ERROR %s:%d: %s\n", FILE, NR, msg) > "/dev/stderr"; exit 3 } - # Remove string CONTENTS and a trailing comment, keeping quote marks, so braces - # and `#` inside strings/comments cannot move the depth or fake an emitter. - function code_only(line, out, i, c, q) { - out = ""; q = "" - for (i = 1; i <= length(line); i++) { - c = substr(line, i, 1) - if (q != "") { if (c == q) { q = ""; out = out c } ; continue } - if (c == "\"" || c == "\047") { q = c; out = out c; continue } - if (c == "#" && (i == 1 || substr(line, i-1, 1) ~ /[ \t;]/)) break # not after `{`/`$`: ${#arr[@]}, $# are code - out = out c - } - return out - } function count(s, ch, n, i) { n = 0; for (i = 1; i <= length(s); i++) if (substr(s, i, 1) == ch) n++; return n } BEGIN { if (LANG == "bash") { DEF = "^[ \t]*(function[ \t]+)?[A-Za-z_][A-Za-z0-9_]*[ \t]*\\(\\)|^[ \t]*function[ \t]+[A-Za-z_][A-Za-z0-9_]*[ \t]*(\\{|$)"; EMIT = "(^|[^A-Za-z0-9_.-])(echo|printf" (KNOWN != "" ? "|" KNOWN : "") ")([^A-Za-z0-9_.-]|$)" } @@ -117,7 +172,7 @@ derive_emitters() { } { line = $0 - if (heredoc != "") { if (line ~ ("^[ \t]*" heredoc "[ \t]*$")) heredoc = ""; next } + if (heredoc != "") { if (closes(line, heredoc)) heredoc = ""; next } code = code_only(line) if (name == "") { if (code !~ DEF) next @@ -130,19 +185,10 @@ derive_emitters() { } depth += count(code, "{") - count(code, "}") body = body "\n" code - # Here-document: the OPERATOR must be in code (not inside a string), but the - # delimiter is read from the RAW line, because code_only has already emptied - # a QUOTED delimiter (<` at the first `#` that is +# OUTSIDE quotes and starts a comment (line start, or after whitespace / `;`). +# Quote-aware through the shared lexer, so an apostrophe in the comment (`# we +# don't …`) no longer keeps the comment in scope, and a `#` inside a quoted string +# (`echo "issue #5"`) is never a comment (Saqlain, client#1020: the old `[^"']*$` +# stripper failed on exactly that pair). Input lines carry grep's `NNN:` prefix; +# the text after it is walked, so a comment at column 1 (`1022:# …`) is still one. strip_trailing_comment() { - # Input lines carry grep's `NNN:` prefix; walk the text after it, so a comment - # at column 1 (`1022:# …`) is still a comment. - awk '{ + awk -v LANG="$1" "$AWK_LEX"'{ prefix = ""; text = $0 if (match($0, /^[0-9]+:/)) { prefix = substr($0, 1, RLENGTH); text = substr($0, RLENGTH + 1) } - out = ""; q = "" - for (i = 1; i <= length(text); i++) { - c = substr(text, i, 1) - if (q != "") { if (c == q) q = ""; out = out c; continue } - if (c == "\"" || c == "\047") { q = c; out = out c; continue } - if (c == "#" && (i == 1 || substr(text, i-1, 1) ~ /[ \t;]/)) break - out = out c - } - print prefix out + print prefix lex(text, 1) }' } @@ -241,77 +279,109 @@ strip_trailing_comment() { # bash here-document (LANG=bash) or a PowerShell here-string (LANG=ps). # Bash: operator detected on quote-stripped code (so a `<<` inside a string does # not count, and `<<<` here-strings are excluded); delimiter taken from the raw -# line, quoted or not -- the same rule derive_emitters applies. +# line, quoted or not -- the same shared rule the derivation applies. # PowerShell: an opener is an at-sign followed by a double or single quote at the # end of a line (Write-Host, throw, or an assignment), the closer is that quote # followed by an at-sign at the start of a line # (Bugbot on client#1020, fourth round: Print-Help and the data-directory -# `throw` are here-strings a customer reads). Assignments are scanned too -- -# over-inclusive on purpose, the guard accepts that bias. +# `throw` are here-strings a customer reads). +# +# TWO KINDS OF BODY. A here-document the installer PRINTS (`cat <<'HELP'`, +# `warn < "$values_file"` or `cat > "$f" <&2`), assigned +# to a variable (`x=$(cat <`/`>>`, or `1>`. Not `2>` (only + # stderr moves, the body still prints), not `>&2` (a stream), not a + # /dev/ pseudo-file (`>/dev/stderr` prints, `>/dev/null` writes no file). + # Any of those leaves the body classified as printed text (Bugbot on + # client#1022: `2>/dev/null` used to read as a generated file). + rest = code + while (match(rest, />>?/)) { + pre = (RSTART > 1) ? substr(rest, RSTART - 1, 1) : "" + tgt = substr(rest, RSTART + RLENGTH); sub(/^[ \t]*/, "", tgt) + rest = substr(rest, RSTART + RLENGTH) + if (pre ~ /[0-9]/ && pre != "1") continue + if (tgt == "" || tgt ~ /^[&>]/ || tgt ~ /^\/dev\//) continue + return 1 } - ' "$file" - return - fi - awk ' - function code_only(line, out, i, c, q) { - out = ""; q = "" - for (i = 1; i <= length(line); i++) { - c = substr(line, i, 1) - if (q != "") { if (c == q) { q = ""; out = out c } ; continue } - if (c == "\"" || c == "\047") { q = c; out = out c; continue } - if (c == "#" && (i == 1 || substr(line, i-1, 1) ~ /[ \t;]/)) break - out = out c - } - return out + return 0 } - BEGIN { heredoc = "" } + function file_text(line) { sub(/(^|[ \t])#.*$/, "", line); return line } + BEGIN { closer = ""; is_file = 0 } { - if (heredoc != "") { if ($0 ~ ("^[ \t]*" heredoc "[ \t]*$")) heredoc = ""; else printf("%d:%s\n", NR, $0); next } + if (closer != "") { + if (closes($0, closer)) closer = ""; else printf("%d:%s\n", NR, is_file ? file_text($0) : $0) + next + } code = code_only($0) - if (code ~ /(^|[^<])<<-?[ \t]*([\047"]?[A-Za-z_]|[\047"][\047"])([^<]|$)/) { - h = $0; sub(/.*<<-?[ \t]*/, "", h); q2 = substr(h, 1, 1) - if (q2 == "\047" || q2 == "\"") { h = substr(h, 2); sub(q2 ".*$", "", h) } else { sub(/[^A-Za-z0-9_].*$/, "", h) } - if (h ~ /^[A-Za-z_][A-Za-z0-9_]*$/) heredoc = h + if (LANG == "ps") { closer = herestring_closer(code) } else { + h = heredoc_delim(code, $0) + if (h == "?") { printf("LEX ERROR %s:%d: here-document with an unreadable delimiter\n", FILE, NR) > "/dev/stderr"; exit 3 } + closer = h } + if (closer != "") is_file = generates_a_file(code) } ' "$file" } +# Template + fail-closed: a bare `mktemp -d` can fail (BSD mktemp, an unwritable +# TMPDIR) and leave tmpd EMPTY, and the cleanup below would then expand to +# `rm -f /*` (Bugbot on client#1022). The trap is armed only once the directory +# exists. +tmpd="$(mktemp -d "${TMPDIR:-/tmp}/copyrefs.XXXXXX")" && [ -d "$tmpd" ] || guard_error "could not create a scratch directory under ${TMPDIR:-/tmp}" +trap 'rm -f "$tmpd"/*; rmdir "$tmpd" 2>/dev/null' EXIT offenders=0 for f in $shipped; do case "$f" in - *.sh) line_re="$bash_line_re" ;; - *.ps1) line_re="$ps_line_re" ;; + *.sh) lang=bash; line_re="$bash_line_re" ;; + *.ps1) lang='ps'; line_re="$ps_line_re" ;; *) guard_error "shipped file with an unknown language, cannot pick a vocabulary: $f" ;; esac - # THREE STAGES, EACH THROUGH A FILE WITH ITS OWN STATUS -- never one pipeline. - # Under `pipefail` a pipeline's status is the RIGHTMOST non-zero one, so + # STAGED, EACH THROUGH A FILE WITH ITS OWN STATUS -- never one pipeline. Under + # `pipefail` a pipeline's status is the RIGHTMOST non-zero one, so # `grep | sed | grep` turned a first-grep failure (2: a bad line regex, an # unreadable path) into the trailing grep's no-match (1) and reported the # file clean (Bugbot on client#1020). Each stage's own exit code is checked # before the next runs; only grep's 1 (no match) may pass. - stage1="$(mktemp)"; stage2="$(mktemp)" - grep -nE "$line_re" "$ROOT/$f" >"$stage1"; rc=$? - [ "$rc" -le 1 ] || { rm -f "$stage1" "$stage2"; guard_error "grep failed ($rc) selecting copy lines in $f"; } - # HERE-DOCUMENT / HERE-STRING BODIES ARE COPY TOO: `cat <<'HELP' … HELP` and + code_lines="$tmpd/code"; body_lines="$tmpd/body"; scan="$tmpd/scan" + # (a) code lines that start with an emitter + grep -nE "$line_re" "$ROOT/$f" >"$code_lines"; rc=$? + [ "$rc" -le 1 ] || guard_error "grep failed ($rc) selecting copy lines in $f" + # (b) HERE-DOCUMENT / HERE-STRING BODIES ARE COPY TOO: `cat <<'HELP' … HELP` and # `Write-Host @" … "@` are how the installers print help and multi-line notices, # and none of those body lines starts with an emitter, so the grep above never - # sees them. Append every body line (same operator/delimiter rule as the derivation). - case "$f" in - *.sh) heredoc_body_lines bash "$ROOT/$f" >>"$stage1" || { rm -f "$stage1" "$stage2"; guard_error "could not list here-document bodies in $f"; } ;; - *.ps1) heredoc_body_lines ps "$ROOT/$f" >>"$stage1" || { rm -f "$stage1" "$stage2"; guard_error "could not list here-string bodies in $f"; } ;; - esac - strip_trailing_comment <"$stage1" >"$stage2" || { rm -f "$stage1" "$stage2"; guard_error "comment stripping failed in $f"; } - hits="$(grep -E "$TOKEN_RE" "$stage2")"; rc=$? - rm -f "$stage1" "$stage2" + # sees them. + heredoc_body_lines "$lang" "$ROOT/$f" >"$body_lines" || guard_error "could not list here-document bodies in $f" + # (c) a body line is TEXT the customer reads in full: it is never also a code + # line, and no comment is stripped from it. A body line that happened to start + # with an emitter word (`echo` in a usage text) used to be taken by BOTH (a) and + # (b) -- printed twice, counted twice -- and `# …` in a body line used to be + # stripped as a comment although the customer reads it (Saqlain, client#1020, + # second round). The body list is read in BEGIN, not via NR==FNR, so an empty + # body list cannot make every code line read as a body line. + awk -F: -v BODY="$body_lines" 'BEGIN { while ((getline l < BODY) > 0) { split(l, a, ":"); body[a[1]] = 1 } } !($1 in body)' "$code_lines" >"$scan" || guard_error "could not separate code lines from here-document bodies in $f" + strip_trailing_comment "$lang" <"$scan" >"$code_lines" || guard_error "comment stripping failed in $f" + cat "$body_lines" >>"$code_lines" || guard_error "could not append here-document bodies in $f" + # (d) the identifier scan + hits="$(grep -E "$TOKEN_RE" "$code_lines")"; rc=$? [ "$rc" -le 1 ] || guard_error "grep failed ($rc) scanning $f for tracker identifiers" if [ -n "$hits" ]; then printf '%s\n' "$hits" | sed "s|^|$f:|" @@ -320,7 +390,7 @@ for f in $shipped; do done if [ "$offenders" -gt 0 ]; then - echo "[FAIL] $offenders user-visible line(s) carry an internal tracker identifier (# / RFC- / RFC--)." >&2 + echo "[FAIL] $offenders user-visible line(s) carry an internal tracker identifier (# / RFC- / RFC--)." >&2 echo " A customer cannot open those; say why in words instead." >&2 exit 1 fi diff --git a/scripts/tests/gate-default-prose-mutations.sh b/scripts/tests/gate-default-prose-mutations.sh index 7175da0d..e835eb0d 100755 --- a/scripts/tests/gate-default-prose-mutations.sh +++ b/scripts/tests/gate-default-prose-mutations.sh @@ -43,6 +43,57 @@ mkfixture() { # $1 = destination root done } +# A CONSISTENT MIXED-POLARITY fixture. narrowEdgeuserByEnv shipping +# `true` for dev and `false` for stg/prod, with EVERY source that names the gate -- +# the chart, the schema description, the helper comment, the runbook -- stating +# exactly that. The real chart ships it true everywhere, so like case +# (a-on) this patches a THROWAWAY copy, never the shipped chart. All sources must +# AGREE, or the guard reddens on the stale one instead of on the span this exercises. +# The runbook sentence is the point: `true for dev, false for stg` comma-joined, so a +# greedy `[\w,\s]*` span runs from `true ... for` across `false` into `stg`, while the +# shipped `_SPAN` (which refuses to cross the opposite polarity word or a second `for`) +# reads it the way a human does. Cases (c) and (c-span) share this one builder so the +# GREEN assertion and its `_SPAN` mutation stand on the SAME fixture. +mixfixture() { # $1 = destination root + local d="$1" + mkfixture "$d" + python3 - "$d/client/values.yaml" <<'PY2' +import re, sys +p = sys.argv[1]; s = open(p).read() +m = re.search(r"narrowEdgeuserByEnv:\n(?: \w+: \w+\n)+", s) +assert m, "fixture lost the narrowEdgeuserByEnv block" +block = m.group(0) +patched = block.replace(" stg: true\n", " stg: false\n").replace( + " prod: true\n", " prod: false\n") +assert patched.count(": false\n") == 2, "expected stg+prod to flip to false in the fixture" +open(p, "w").write(s[: m.start()] + patched + s[m.end() :]) +PY2 + python3 - "$d/client/values.schema.json" <<'PY2' +import sys +p = sys.argv[1]; s = open(p).read() +old = "TRUE for dev, stg and prod" +assert s.count(old) == 1, f"schema anchor matched {s.count(old)} times, not 1" +open(p, "w").write(s.replace(old, "TRUE for dev, FALSE for stg and prod")) +PY2 + python3 - "$d/client/templates/_helpers.tpl" <<'PY2' +import sys +p = sys.argv[1]; s = open(p).read() +old = "BAKED ON FOR dev, stg AND prod" +assert s.count(old) == 1, f"helper anchor matched {s.count(old)} times, not 1" +open(p, "w").write(s.replace(old, "BAKED ON FOR dev, OFF FOR stg AND prod")) +PY2 + python3 - "$d/client/MIGRATION.md" <<'PY2' +import sys +p = sys.argv[1]; s = open(p).read() +old = "`rotateMysqlRootByEnv` were added in `1.9.71`." +assert s.count(old) == 1, f"fixture anchor matched {s.count(old)} times, not 1" +open(p, "w").write(s.replace( + old, + "`narrowEdgeuserByEnv` is `true` for `dev`, `false` for `stg` and `prod`, " + "which is what this fixture ships.")) +PY2 +} + run_case() { # $1 label, $2 expected rc, $3 expected substring, $4 fixture root local label="$1" want_rc="$2" want="$3" d="$4" out rc set +e @@ -228,7 +279,7 @@ assert m, "fixture lost the narrowEdgeuserByEnv block" block = m.group(0) patched = block.replace(" stg: true\n", " stg: false\n").replace( " prod: true\n", " prod: false\n") -assert patched.count(": false\n") >= 2, "expected stg+prod to flip to false in the fixture" +assert patched.count(": false\n") == 2, "expected stg+prod to flip to false in the fixture" open(p, "w").write(s[: m.start()] + patched + s[m.end() :]) PY2 python3 - "$D/client/MIGRATION.md" <<'PY2' @@ -243,26 +294,38 @@ open(p, "w").write(s.replace( PY2 run_case "an ON-polarity claim in LIST form is caught (fixture ships stg/prod false)" 1 "MIGRATION.md" "$D" -# (c) THE FALSE-POSITIVE GUARD: a CORRECT claim must stay GREEN. Now that every -# gate ships true for every env, the correct claim is the all-true LIST form; the -# guard must read it as agreement, not misfire on the list span. (Before the -# rollout completed this case stated BOTH polarities in one sentence -- `true` for -# dev, `false` for stg/prod -- to prove a greedy list span did not cross the other -# polarity word and mis-report it; that mixed sentence is no longer a correct -# statement of any real gate, so the check is now the all-true claim. It is still -# the thing standing between the guard and a wall of false findings.) -D="$TMP/mixed"; mkfixture "$D" -python3 - "$D/client/MIGRATION.md" <<'PY2' +# (c) THE FALSE-POSITIVE GUARD, and the reason _SPAN refuses to cross the other +# polarity word. A CORRECT MIXED-POLARITY claim -- `true` for one env and `false` +# for others in ONE sentence -- must stay GREEN: a reader parses "true for dev, +# false for stg and prod" as dev-on / stg-prod-off, and so must the guard. This is +# the case that was lost when the chart went all-true: after every `*ByEnv` gate was +# baked `true` everywhere, an all-true sentence was substituted here, and with nothing left in +# the suite crossing an opposite-polarity word a greedy `_SPAN` would have stayed +# green. mixfixture rebuilds the mixed reality on a throwaway copy, so the sentence +# is correct against THAT chart -- forever, regardless of what the real chart ships. +D="$TMP/mixed"; mixfixture "$D" +run_case "a CORRECT mixed-polarity claim (true dev, false stg/prod) is NOT a finding" 0 \ + "no document contradicts" "$D" + +# (c-span) ...AND THE MUTATION THAT PROVES _SPAN IS LOAD-BEARING (repo +# CLAUDE.md rule 9). On the SAME mixfixture, revert _SPAN in the fixture's OWN +# guard copy to the greedy `[\w,\s]*` it replaced (saqlainsyed007 on #900). The span +# now runs from `true for dev,` across `false` to `stg`, so env stg (shipped false +# here) matches a TRUE_CLAIM and the guard reports "says 'true' for stg" -- one brick +# of the 18-wide false-finding wall the refusal-to-cross was added to stop. Green +# above and red here on one fixture, driven by the real _SPAN and its real reversion, +# not a reimplementation. +D="$TMP/mixedmut"; mixfixture "$D" +python3 - "$D/scripts/tests/gate-default-prose-agreement.sh" <<'PY2' import sys p = sys.argv[1]; s = open(p).read() -old = "`rotateMysqlRootByEnv` were added in `1.9.71`." -assert s.count(old) == 1, f"fixture anchor matched {s.count(old)} times, not 1" +old = r"_SPAN = r'(?:(?!{opp}\b|for\b)[\w,\s])*'" +assert s.count(old) == 1, f"_SPAN anchor matched {s.count(old)} times, not 1" open(p, "w").write(s.replace( - old, - "`rotateMysqlRootByEnv` were added in `1.9.71`.\n\n`narrowEdgeuserByEnv` is `true` for `dev`, `stg` " - "and `prod`, which is what the chart ships.\n\n")) + old, r"_SPAN = r'[\w,\s]*' # mutation: greedy span crosses the other polarity")) PY2 -run_case "a correct all-true LIST claim is NOT a finding" 0 "no document contradicts" "$D" +run_case "a greedy _SPAN misfires on the same mixed claim (says 'true' for stg)" 1 \ + "says 'true' for stg" "$D" # Re-established here rather than relying on the `$D` set above: the cases # inserted between that setup and this assertion silently repointed `$D`, and diff --git a/scripts/tests/image-refresh-repin-on-revert.bats b/scripts/tests/image-refresh-repin-on-revert.bats new file mode 100644 index 00000000..9ebfb874 --- /dev/null +++ b/scripts/tests/image-refresh-repin-on-revert.bats @@ -0,0 +1,237 @@ +#!/usr/bin/env bats +# image-refresh RE-PINS the digest when a helm re-render reverted the workload +# to `repo:tag`, instead of no-op'ing off the annotation alone. +# +# client-runtime#199. `recorded == latest` proves the REGISTRY digest has not moved; it +# does NOT prove the workload is running it. `helm upgrade --reset-then-reuse-values` +# (the fleet auto-upgrade) re-renders the Deployment back to `repo:tag` and discards +# an earlier `set image repo@digest` pin -- and on a node whose `:tag` layer is +# stale that silently runs an OLD control-plane image (client-runtime#199). So the +# loop reads each workload's LIVE image and re-pins whenever it is off the digest. +# +# These assert BEHAVIOUR, not text presence: the earlier helm-unittest checks that +# `workload_image_for_repo`/`have=`/`proxy_on_digest` merely APPEAR in the script +# still pass if the comparison is inverted. This extracts the shipped branch from +# the RENDERED chart and drives it with the registry + live-workload reads stubbed, +# so an inverted comparison 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/branch.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 script containing the re-pin branch" + +lines = script.splitlines() +start = next(i for i, l in enumerate(lines) + if l.strip() == 'if [ "$recorded" = "$latest" ]; then') +# The branch ends at the `esac` that closes the `case "$repo" in` re-image block +# (no nested `case`, so the first `esac` after it closes it). +case_at = next(i for i in range(start, len(lines)) + if lines[i].strip() == 'case "$repo" in') +end = next(i for i in range(case_at, len(lines)) if lines[i].strip() == "esac") +body = "\n".join(l[6:] if l.startswith(" " * 6) else l.lstrip() + for l in lines[start:end + 1]) +open(sys.argv[2], "w").write(body) +PYX +} +teardown() { rm -rf "$TMP"; } + +# Runs the shipped re-pin branch with the registry HEAD (already known: recorded +# == latest) and the two LIVE-image reads stubbed. +# $1 = STUB_API what workload_image_for_repo returns ("" = unreadable) +# $2 = STUB_PROXY what requests_proxy_image returns ("" = unreadable) +# $3 = RP_PINNED "1" opts the requests-proxy out of following the digest +# $4 = PENDING the ATTEMPT_KEY value carried in (0 = no unfinished re-image) +# +# The branch is wrapped in a ONE-ITERATION loop so its `continue` statements run +# as they ship, rather than being stripped (which would change control flow). +run_branch() { + cat > "$TMP/harness.sh" <= MAX) is a no-op, not a forced re-run" { + # Once ATTEMPT_KEY has reached MAX_REFRESH_ATTEMPTS the flap guard downstream + # annotates FLAP_KEY and exit 0s BEFORE any set image / rollout status, so a + # forced re-run there resolves nothing and, worse, skips the tick's annotation + # write forever. The branch must fall to the no-op path instead + # (@shujaatTracebloc on #1008, blocking 1 & 2). MAX is 3, so pending=3 latches. + run run_branch "docker.io/tracebloc/jobs-manager@sha256:aaa" \ + "docker.io/tracebloc/jobs-manager@sha256:aaa" "0" "3" + [ "$status" -eq 0 ] || return 1 + [[ "$output" == *"; no-op"* ]] || return 1 + [[ "$output" == *"RESTART:0"* ]] || return 1 + [[ "$output" != *"unfinished re-image"* ]] || return 1 +} + +@test "a LATCHED tick still SURFACES the stopped refresh (WARN + FLAP_KEY), not a bare no-op" { + # With the `< MAX` gate a latched tick keeps restart_needed=0 and never enters + # the downstream flap guard -- the only other writer of FLAP_KEY and the MANUAL + # ATTENTION WARN. So refresh is dead for ALL control-plane images while the + # CronJob stays green, and #1964 forbids "images did not update" being + # inferable only from the Job's colour. The latched arm must itself emit the + # WARN naming the refresh-attempt clear and annotate FLAP_KEY before the no-op + # (@shujaatTracebloc / @LukasWodka / @saadqbal on #1008). MAX is 3. + run run_branch "docker.io/tracebloc/jobs-manager@sha256:aaa" \ + "docker.io/tracebloc/jobs-manager@sha256:aaa" "0" "3" + [ "$status" -eq 0 ] || return 1 + [[ "$output" == *"FLAP LATCHED"* ]] || return 1 + [[ "$output" == *"MANUAL ATTENTION NEEDED"* ]] || return 1 + [[ "$output" == *"clear the tracebloc.io/refresh-attempt annotation"* ]] || return 1 + [[ "$output" == *"KUBECTL:annotate deployment"*"tracebloc.io/refresh-flap-detected=3"* ]] || return 1 +} + +@test "a NON-latched no-op (pending=MAX; a clean on-digest + # tick (pending=0) and a bounded-attempt tick must not annotate FLAP_KEY. + run run_branch "docker.io/tracebloc/jobs-manager@sha256:aaa" \ + "docker.io/tracebloc/jobs-manager@sha256:aaa" "0" "0" + [ "$status" -eq 0 ] || return 1 + [[ "$output" == *"; no-op"* ]] || return 1 + [[ "$output" != *"FLAP LATCHED"* ]] || return 1 + [[ "$output" != *"KUBECTL:"* ]] || return 1 +} + +@test "api on digest but proxy reverted re-pins the PROXY, not the api" { + run run_branch "docker.io/tracebloc/jobs-manager@sha256:aaa" \ + "docker.io/tracebloc/jobs-manager:dev" "0" + [ "$status" -eq 0 ] || return 1 + [[ "$output" == *"re-pinning the requests-proxy digest"* ]] || return 1 + [[ "$output" == *"RESTART:1"* ]] || return 1 + [[ "$output" == *"proxy=docker.io/tracebloc/jobs-manager@sha256:aaa"* ]] || return 1 + # the fall-through re-derives BOTH set args; re-setting the api to the digest + # it already runs is an idempotent no-op patch (no rollout), which is why the + # proxy-only revert is repaired without special-casing it. + [[ "$output" == *"api=docker.io/tracebloc/jobs-manager@sha256:aaa"* ]] || return 1 +} + +@test "an unreadable api image SKIPS the re-pin this tick (no restart, no churn)" { + run run_branch "" "" "1" + [ "$status" -eq 0 ] || return 1 + [[ "$output" == *"unreadable"* ]] || return 1 + [[ "$output" == *"skipping re-pin this tick"* ]] || return 1 + [[ "$output" == *"RESTART:0"* ]] || return 1 +} + +@test "an unreadable requests-proxy image SKIPS the re-pin this tick too" { + run run_branch "docker.io/tracebloc/jobs-manager@sha256:aaa" "" "0" + [ "$status" -eq 0 ] || return 1 + [[ "$output" == *"requests-proxy image is unreadable"* ]] || return 1 + [[ "$output" == *"RESTART:0"* ]] || return 1 +} + +@test "a registry-prefix rewrite (mutating webhook) is NOT mistaken for a revert" { + # The webhook keeps the @sha256 suffix; comparing on the digest must read this + # as already-pinned, or every tick re-pins and three ticks trip the #563 flap + # lockout for all images (LukasWodka on #1008). + run run_branch "mirror.internal/tracebloc/jobs-manager@sha256:aaa" \ + "mirror.internal/tracebloc/jobs-manager@sha256:aaa" "0" + [ "$status" -eq 0 ] || return 1 + [[ "$output" == *"no-op"* ]] || return 1 + [[ "$output" == *"RESTART:0"* ]] || return 1 +}