Skip to content

feat(deploy): one-command laptop demo — harness on kind, remote worker as a host container - #195

Merged
pdettori merged 7 commits into
rossoctl:mainfrom
pdettori:feat/demo-remote-worker-174
Aug 31, 2026
Merged

feat(deploy): one-command laptop demo — harness on kind, remote worker as a host container#195
pdettori merged 7 commits into
rossoctl:mainfrom
pdettori:feat/demo-remote-worker-174

Conversation

@pdettori

Copy link
Copy Markdown
Member

Refs #174 — the acceptance criteria. The nine feature vignettes (streaming, abort
mid-stream, dual-ended timeout, reconnect→dedup, presence mirror, docker inspect trust
model, transport swappability) are deliberately deferred to a follow-up, since they are the
bulk of the line count and none of the acceptance boxes.

What this adds

make demo-remote-sandbox goes from no cluster to a passing A/B in one invocation, printing
what it proves at each step. make demo-remote-sandbox-teardown removes the container,
relay, cluster and built image.

laptop
|- kind cluster:  Knative + Redis + harness (ksvc) + sandbox-relay
|                     ^                                    ^
|                     | harness -> relay                   | worker -> relay
|                     | sandbox-relay.default.svc:8443     | host.docker.internal:8443
\- docker run:    remote-worker  --------- dials out ------/

Neither address is inbound to the laptop. The worker publishes no ports — docker run with
no -p at all.

Why a green run would prove nothing, and what stops that

select-sandbox.ts:103 builds candidates = [...pods, ...grpcRecs] and leases
least-loaded-first, so SH_REMOTE_SANDBOX=1 alone does not route to the worker. Two defenses:

  1. Structural — before any remote dispatch, assert the pool selector matches zero
    Running pods
    . A pod cannot win a lease it is not a candidate for.
  2. Fingerprint — the pool is Alpine, the worker image is RHEL, and both directions are
    asserted, so an exec that landed on a pod fails one check or the other:
backend Alpine Red Hat model's own stated reason
in-cluster pod FLAGGED CLEAR "…confirming this system runs Alpine Linux"
remote host container CLEAR FLAGGED "…the system is running Red Hat Enterprise Linux 9.8"

The discriminator is verified before anything relies on it, and the summary prints the
model's reason, so the OS it actually read is visible rather than inferred.

Three of the issue's assumptions had already moved

  • relay-deployment.yaml already sets SH_RELAY_TOKEN — trap 2 is fixed on main.
  • set_ksvc_env patches only the env JSON path, so no volumeMounts are at risk.
  • remote-worker/Dockerfile already builds self-contained from the repo root, so there
    is no hand-rolled Dockerfile and no local Go toolchain. And because the image runs on
    the host and is never kind loaded, its architecture need not match the kind node
    that caveat only ever applied to the pod path.

Two traps found while validating

  • The relay was being OOMKilled before it ever bound :8443. node --import tsx
    compiles the relay's TypeScript at startup: measured 224 MiB idle / 225 MiB peak
    against a 128Mi limit. It failed in the worst way — rollout status reports Ready
    (no readinessProbe, so Running suffices) and the breakage only surfaced as
    "connection refused inside namespace", which reads as a networking fault. Fixed in its
    own commit; independently cherry-pickable.
  • The relay needs ~4s after Running to bind. Probing immediately is a race that looks
    identical to a broken relay. The tunnel probe now retries, restarts kubectl port-forward
    when a failed forward kills it, and proves the relay answers from the host before
    testing container networking — so a dead relay is never misreported as a networking fault.

Also: the leaf endpoint needs a kourier port-forward. Without it every dispatch returns an
empty body, indistinguishable at the verdict layer from an unreachable model. Now asserted
up front, and assert_verdict's hint names both causes it cannot distinguish.

Shared library, so the two proofs cannot drift

The A/B logic lived entirely in relay-leaf-smoke.sh. Rather than copy it, the
worker-topology-agnostic parts moved to lib-relay.sh, sourced by both.
validate_discriminator takes the two os-release texts as strings rather than fetching
them, because the gate reads the worker with kubectl exec and the demo with docker exec
shared assertion, local fetch.

Verification

Check Result
demo-remote-worker.sh --reuse-cluster on Kind 8/8 pass, opposite verdicts, env restored exactly
RELAY_LIVE_SMOKE=1 relay-leaf-smoke.sh (regression on the refactor) 9/9 pass — the 9th is the new selector assertion
CI's find deploy/ -name '*.sh' -exec shellcheck -x -S warning pass, all of deploy/
Leftovers after a run none: no container, relay, or port-forward

Not yet validated locally: the cold-start path (--reuse-cluster omitted, no cluster
present). Every run here reused a warm cluster; the from-scratch invocation delegates cluster
creation to setup-kind.sh, which already creates it when absent, but that specific
end-to-end path has not been exercised on this branch. Worth one clean run before merge.

Container-to-host reachability is probed with the worker's own image via bash /dev/tcp,
escalating only as needed (--add-host, then --address 0.0.0.0 with a warning that the
port becomes briefly LAN-visible), so the common laptop case exposes nothing beyond the host.

Assisted-By: Claude Code

The relay entrypoint is `node --import tsx src/main.ts`, so tsx compiles the
relay's TypeScript in memory at startup: measured 224 MiB steady / 225 MiB peak
while merely idle, before a single exec is relayed. The 128Mi limit was therefore
never survivable -- the container was OOMKilled (exit 137) during startup and
restarted before it ever bound :8443.

It failed in the worst possible way. `kubectl rollout status` reports Ready,
because the Deployment declares no readinessProbe and Running is enough, so
deployment looked clean; the breakage only surfaced on a connection attempt, as
"connection refused inside namespace" -- which reads as a networking fault rather
than a dead relay.

Requests now cover the measured idle footprint; the limit leaves headroom for
concurrent relayed streams.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
relay-leaf-smoke.sh owned the whole A/B proof: the Alpine/RHEL discriminator and
its up-front validation, leaf dispatch, verdict assertion, the Redis presence
checks, and the trap-driven exact restore of the harness ksvc env. The laptop
demo added next needs all of it, differing only in WHERE the worker runs and HOW
its /etc/os-release is read -- so a second copy would let the two proofs drift,
and a drifting copy lets one path keep asserting something the other no longer
does.

Extracted worker-topology-agnostic. validate_discriminator takes the two
os-release texts as strings rather than fetching them, because the gate reads the
worker with `kubectl exec` and the demo with `docker exec`, but the assertion
deciding whether the discriminator is trustworthy must be identical.

Two strengthenings while the logic was being lifted:

- assert_no_pods_match makes the pool-selection trap structurally impossible
  rather than merely detectable. select-sandbox.ts builds
  candidates = [...pods, ...grpcRecs], so asserting the selector matches zero
  Running pods leaves its least-loaded-first leasing nothing to route around the
  worker with.
- diagnose_relay_crash names a relay that is Running but not serving, calling out
  OOMKilled specifically. Without it, that failure surfaced far downstream as an
  unexplained presence-assertion failure.

assert_verdict's "no verdict returned" hint now names both causes it cannot
distinguish -- an unreachable harness endpoint and an unreachable model -- since
blaming only the model sends you to the wrong place.

Verified behaviour-preserving: RELAY_LIVE_SMOKE=1 relay-leaf-smoke.sh passes
9 of 9 on Kind, the 9th being the new selector assertion.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
Refs rossoctl#174 (acceptance criteria; the feature vignettes follow separately).

`make demo-remote-sandbox` goes from no cluster to a passing A/B in one
invocation, printing what it proves at each step. kind + Knative + Redis +
harness + relay run in-cluster; the worker runs as a plain `docker run` on the
host with no published ports, reaching the relay only by dialing OUT through a
port-forward. A worker deployed as a pod -- worker-example.yaml, and the
relay-leaf-smoke.sh gate -- demonstrates the plumbing but not the driver, which
is the point of this demo. `make demo-remote-sandbox-teardown` removes the
container, relay, cluster and built image.

The proof is defended twice, because a single green run proves nothing about
which backend served it:

- Structurally: the pool selector is asserted to match zero Running pods before
  any remote dispatch, so the worker is the only lease candidate there is.
- By fingerprint: the pool is Alpine and the worker image is RHEL, and BOTH
  directions are asserted ("Alpine" CLEAR and "Red Hat" FLAGGED), so an exec that
  landed on a pod fails one check or the other. The summary prints the model's own
  stated reason, so the OS it actually read is visible rather than inferred.

Three things the issue anticipated turned out not to be needed. The relay
manifest already sets SH_RELAY_TOKEN; set_ksvc_env patches only the env JSON path
so no volumeMounts are at risk; and remote-worker/Dockerfile already builds
self-contained from the repo root, so there is no hand-rolled Dockerfile, no local
Go toolchain, and -- because the image runs on the host and is never kind-loaded
-- no architecture matching against the kind node.

Two traps found while validating, both now handled with named diagnostics rather
than silence:

- The relay needs ~4s after Running to finish tsx compilation and bind :8443, so
  probing immediately is a race that looks exactly like a broken relay. The tunnel
  probe retries, restarting kubectl port-forward when a failed forward kills it,
  and proves the relay answers from the host BEFORE testing container networking
  -- so a dead relay is never misreported as a networking fault.
- The leaf endpoint needs a kourier port-forward. Without it every dispatch
  returns an empty body, indistinguishable at the verdict layer from an
  unreachable model. It is now asserted up front.

Container-to-host reachability is probed with the worker's own image and network
config via bash /dev/tcp, then escalated only as needed (--add-host, then
--address 0.0.0.0 with a warning that the port is briefly LAN-visible), so the
common laptop case exposes nothing beyond the host.

Verified on Kind: 8 of 8 assertions pass, with the remote leaf reporting "the
system is running Red Hat Enterprise Linux 9.8" while the pod path reports Alpine.
The harness ksvc env is snapshotted and restored from an EXIT trap, verified to
restore exactly.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Paolo Dettori <dettori@us.ibm.com>

@pdettori pdettori left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Review

Approve on the merits — I'd have used APPROVE but GitHub blocks self-approval, so this is a COMMENT. Nothing here is blocking.

The two defenses are the right shape, and the discriminator-before-reliance ordering is what makes the A/B actually mean something. I verified a few of the PR body's claims independently rather than taking them on trust:

  • set_ksvc_env (upstream lib.sh:416) really does mutate the env array by name via jq and replace it with a JSON-patch op, preserving valueFrom entries — so the "no volumeMounts at risk" claim holds.
  • docker build --load matches existing repo convention (setup-kind.sh:71,241,250, relay-leaf-smoke.sh:163), not a new dependency on buildx.
  • ko() returns 0, so the unguarded assert_verdict calls can't trip set -e and skip the results summary. The [ … ] && abort / [ -n … ] && note lines are all mid-script, so they're exempt from set -e too.
  • The relay memory bump is well-argued and the measurements are in the manifest comment, which is the right place for them.

Commits are cleanly ordered (memory fix → refactor → feat), all DCO-signed, all conventional prefixes. CI is 10/10 green including shellcheck over all of deploy/.

Four suggestions and one nit below. The two worth acting on are the teardown scope (#1) and count_pool_pods failing open (#2) — the second one undercuts the PR's own strongest argument.

One pre-merge item, echoing the PR body's own caveat: the cold-start path (--reuse-cluster omitted, no cluster present) is unexercised on this branch. Worth one clean run before merge, since it's the path make demo-remote-sandbox takes by default and therefore the one a first-time reader will hit.

Assisted-By: Claude Code

Comment thread deploy/knative/demo-remote-worker.sh Outdated
fi
if kind get clusters 2>/dev/null | grep -q "^${CLUSTER_NAME}$"; then
echo "--- deleting kind cluster '$CLUSTER_NAME' ---"
kind delete cluster --name "$CLUSTER_NAME" >/dev/null 2>&1 || true

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

suggestion--teardown deletes a cluster it may not have created.

--reuse-cluster exists precisely to run against a pre-existing cluster, but nothing records whether this invocation created $CLUSTER_NAME. The sharp edge is line 167: the normal-exit cleanup prints

cluster 'sh-knative' left running. Remove everything with:
  ./demo-remote-worker.sh --teardown

So after make demo-remote-sandbox DEMO_ARGS=--reuse-cluster against a long-lived dev cluster, the script actively recommends the command that destroys it. The README's "removes the container, relay, cluster and built image" is accurate, but a reader who reached teardown by following the script's own hint hasn't read the README again.

Cheapest fix is one flag: set CREATED_CLUSTER=1 in the else branch of the step-2 cluster block (the path that runs setup-kind.sh), then gate this kind delete on it — and make the line-167 hint conditional on the same flag, so a reused cluster is never advertised as disposable. --teardown as an explicit standalone invocation could still delete unconditionally, or prompt.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@-

Comment thread deploy/knative/lib-relay.sh Outdated
# Usage: count_pool_pods <selector>
count_pool_pods() {
kubectl get pods -n "$NS" -l "$1" --field-selector=status.phase=Running --no-headers 2>/dev/null \
| wc -l | tr -d ' '

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

suggestion — this makes defense #1 fail open, which is the one place it must not.

kubectl get pods … 2>/dev/null | wc -l returns 0 for both "the selector genuinely matches nothing" and "kubectl failed" — wrong context, transient API error, RBAC, expired credential. In the assert_no_pods_match direction that means a kubectl blip prints

PASS: pool selector '…' matches 0 Running pods -- the remote worker is the only lease candidate

and the remote assertions proceed against an unverified candidate set. That is exactly the vacuous-green outcome the structural defense exists to rule out, and it's the failure mode the comment above assert_no_pods_match calls "structurally impossible rather than merely detectable".

Note the asymmetry: the same helper backing the [ "${POOL_POD_COUNT:-0}" -ge 1 ] precondition fails safe (a kubectl error aborts). Only the zero-direction is unsafe, so the fix belongs here rather than at the call sites.

Something like capturing the query separately so a non-zero kubectl status is distinguishable from an empty result:

count_pool_pods() {
  local out
  out="$(kubectl get pods -n "$NS" -l "$1" --field-selector=status.phase=Running --no-headers 2>/dev/null)" \
    || { echo "ERR"; return 1; }
  printf '%s' "$out" | grep -c . | tr -d ' '
}

…and having assert_no_pods_match abort on ERR rather than treating it as zero. (grep -c . rather than wc -l also avoids counting a trailing empty line as a pod.)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@-

# Native Linux Docker: host-gateway is the bridge IP, which a loopback-only
# port-forward does not answer on. Widen the bind, and say so -- for the demo's
# duration the relay port is reachable from the local network.
PF_ADDRESS_ARGS=(--address 0.0.0.0)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

suggestion — the 0.0.0.0 widening pairs a LAN-reachable relay with a token that is a repo constant.

The warning, the last-resort ordering, and the teardown-on-exit are all handled well, and the README documents it. The residual gap is that RELAY_TOKEN defaults to dev-token (line 56), which is hardcoded in relay-deployment.yaml and therefore public. So for the demo's duration anyone on the same network can Attach to the relay as a sandbox and start receiving the leaf's exec payloads. On native Linux Docker this branch is the normal path, not a rare fallback.

Since the relay's token is a plain env value on the Deployment, this is close to free to close: generate a per-run random token, patch it into the relay before rollout status, and pass the same value to the worker. That also strengthens the trust-model section — docker inspect would then show a credential that is genuinely scoped to this one run, rather than a constant checked into the repo.

At minimum, worth making the existing WARN say that the exposed port accepts a well-known token, so the reader can judge whether their network is somewhere they want that.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@-

Comment thread deploy/knative/demo-remote-worker.sh Outdated
--reuse-cluster) REUSE_CLUSTER=1; shift ;;
--keep) KEEP=1; shift ;;
--teardown) TEARDOWN_ONLY=1; shift ;;
-h|--help) grep '^#' "$0" | sed 's/^# \{0,1\}//'; exit 0 ;;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

nit--help prints more than the usage block.

grep '^#' "$0" matches every column-0 comment in the file, so --help emits the shebang as !/usr/bin/env bash and then, after the usage text, ~10 blocks of internal implementation commentary (# Demo narration: …, # EXIT trap. Restoring …, # Does a connection actually TRAVERSE the tunnel …, # --- 1. Preflight ---, …). Indented comments are excluded, but the section headers and function docs are not.

The header block is carefully written and reads well as help output on its own, so it's worth stopping at the first non-comment line:

-h|--help) sed -n '2,/^[^#]/{/^#/s/^# \{0,1\}//p;}' "$0"; exit 0 ;;

Starting at line 2 also drops the shebang.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@-

#
# Sourced by BOTH the gated conformance smoke (relay-leaf-smoke.sh, worker as an
# in-cluster pod) and the laptop demo (demo-remote-worker.sh, worker as a host
# container). Everything here is worker-topology-agnostic on purpose: the two callers

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

suggestion — nothing enforces the property this file exists to guarantee.

"A drifting copy would let one path keep asserting something the other no longer does" is the right motivation, and extracting the shared assertions is the right fix. But the invariant is currently maintained by convention only: a future edit can re-inline a copy of validate_discriminator or assert_verdict into either caller and nothing notices.

deploy/knative/tests/setup-kind-image.test.sh already establishes the pattern for cluster-free static tests in this repo (mocked PATH, assert_grep / assert_absent on the call log). A short companion test asserting that both relay-leaf-smoke.sh and demo-remote-worker.sh source ./lib-relay.sh, and that neither defines validate_discriminator / assert_verdict / dispatch_pattern locally, would lock the anti-drift property for roughly the cost of this comment — and it runs in CI, where RELAY_LIVE_SMOKE=1 never does.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@-

`count_pool_pods` piped a failed `kubectl get` into `wc -l`, which yields 0 --
indistinguishable from "the selector genuinely matches nothing". So a wrong
context, transient API error, expired credential or missing RBAC made
`assert_no_pods_match` print

    PASS: pool selector '...' matches 0 Running pods

and the remote assertions then ran against an unverified candidate set. That is
exactly the vacuous green the structural defense exists to rule out, and the
failure mode the comment above `assert_no_pods_match` calls "structurally
impossible rather than merely detectable".

The asymmetry mattered: the same helper backing the `-ge 1` precondition fails
safe, because a kubectl error there aborts. Only the zero-direction was unsafe,
so the fix belongs in the helper rather than at the call sites.

`count_pool_pods` now captures the query separately and echoes "ERR" (returning
1) when kubectl fails, and `assert_no_pods_match` aborts on it instead of
treating it as zero. Both `-ge 1` call sites gained an explicit ERR check so a
failed query names its cause rather than exiting bare under `set -e`.
`grep -c .` replaces `wc -l` so a trailing newline is never counted as a pod.

Verified with kubectl forced to fail (aborts), forced to return nothing (still
passes), and returning two pods (aborts).

Refs rossoctl#174

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
The demo defaulted `RELAY_TOKEN` to `dev-token`, which is hardcoded in
relay-deployment.yaml and therefore public. That pairs badly with the
container-to-host fallback: when neither the loopback bind nor `--add-host`
works, the demo re-binds the port-forward to `--address 0.0.0.0`, so for the
demo's duration anyone on the same network could Attach to the relay as a
sandbox -- with a credential read out of the repo -- and start receiving the
leaf's exec payloads. On native Linux Docker that branch is the normal path,
not a rare fallback.

The token is a plain env value on the Deployment, so this is close to free to
close: generate a random token per run, patch it onto the relay before waiting
on the rollout (so the pod that becomes Ready already holds it), and hand the
same value to the worker. `SANDBOX_TOKEN` still pins a value when set.

relay-deployment.yaml is deliberately left alone: packages/knative-server/test/
relay-deployment.test.ts asserts its SH_RELAY_TOKEN equals worker-example.yaml's
SANDBOX_TOKEN, and a later `kubectl apply -f relay-deployment.yaml` (which
relay-leaf-smoke.sh does) reverts the patch, so nothing is left behind for other
callers.

This also strengthens the trust-model section of README-worker.md: what
`docker inspect` shows is now a credential scoped to this one run rather than a
repo constant. The 0.0.0.0 WARN says so too, so the reader can judge the
exposure.

Token generation prefers `openssl rand -hex 16` and falls back to /dev/urandom
via `head -c 16 | od`; `head` leads the pipeline so no stage is killed by
SIGPIPE, which `pipefail` would otherwise surface as a failure. Both paths
verified to yield 32 distinct hex chars under `set -euo pipefail`.

Refs rossoctl#174

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
Two CLI-surface fixes on the demo script.

--teardown deleted a cluster it may never have created. --reuse-cluster exists
precisely to run against a pre-existing cluster, but nothing recorded whether
this invocation created $CLUSTER_NAME. The sharp edge was the normal-exit hint,
which printed

    cluster 'sh-knative' left running. Remove everything with:
      ./demo-remote-worker.sh --teardown

unconditionally -- so after `make demo-remote-sandbox DEMO_ARGS=--reuse-cluster`
against a long-lived dev cluster, the script actively recommended the command
that destroys it. The README's "removes everything" was accurate, but a reader
who reached teardown by following the script's own hint has not re-read it.

Now CREATED_CLUSTER is raised only on the path that actually creates the cluster
-- checked before setup-kind.sh runs, since it reuses an existing cluster rather
than recreating it -- and the exit hint is gated on it: a reused cluster is never
advertised as disposable. `--teardown` itself is a separate process and cannot
know, so it asks before deleting, takes --yes/-y as the non-interactive answer,
and keeps the cluster when there is no terminal to ask on. Everything else it
removes stays unconditional, and the closing message names what survived.

--help printed far more than the usage block, and in the documented invocation
printed nothing at all. `grep '^#' "$0"` matched every column-0 comment, so it
emitted the shebang as `!/usr/bin/env bash` and then ~10 blocks of internal
implementation commentary. Worse, `cd "$(dirname "$0")"` runs before arg
parsing, so a relative "$0" no longer resolved: `make demo-remote-sandbox
DEMO_ARGS=--help` -- the documented entry point -- failed with
`grep: ...: No such file or directory` and exit 2.

Fixed by resolving the script path into SELF before the cd, and printing only
line 2 to the first non-comment line, which also drops the shebang. Verified at
38 lines with no leakage on BSD, GNU and busybox sed, from all four invocation
styles (make, relative, absolute, and from within deploy/knative). The
user-facing hints keep using "$0": those are copy-pasted from the caller's own
cwd, which is what "$0" was relative to.

Refs rossoctl#174

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
… sharing

lib-relay.sh exists so the two remote-sandbox proofs cannot drift, but nothing
enforced it: a future edit could re-inline a copy of validate_discriminator or
assert_verdict into either caller and every existing check would still pass,
while the two proofs quietly stopped asserting the same thing.

lib-relay-shared.test.sh locks the property statically -- both callers source
./lib-relay.sh, neither shadows a shared assertion, each is defined in the
library exactly once -- and additionally exercises count_pool_pods
behaviourally, so the fail-closed fix cannot silently regress. Following
setup-kind-image.test.sh's pattern: no cluster, mocks on PATH.

demo-teardown-scope.test.sh covers the other half of this review round: with no
tty and no --yes, `--teardown` must keep the cluster while still removing the
container, relay and image; with --yes or -y it must delete it; and the exit
hint must stay gated on CREATED_CLUSTER.

The wiring is the part that makes these worth having. No CI job ran
deploy/knative/tests/*.test.sh -- security-scans.yml only shellchecks deploy/ --
so setup-kind-image.test.sh had never executed in CI either, and a new test file
would have locked nothing. `make test-deploy` runs them all under `set -e`, a
node-free `deploy-scripts` job runs that on every PR, and `make test` includes
it locally. Verified that a deliberately failing test file fails the target.

Refs rossoctl#174

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
@pdettori
pdettori merged commit fc757a7 into rossoctl:main Aug 31, 2026
11 checks passed
@pdettori
pdettori deleted the feat/demo-remote-worker-174 branch August 31, 2026 22:29
pdettori added a commit that referenced this pull request Sep 1, 2026
The remote-sandbox path had a one-command script (`make demo-remote-sandbox`, #195) and a
reference section in `README-worker.md`, but nothing you could drive by hand. Those two
serve different purposes: a script optimizes for an unattended pass/fail, while performing
the claim in front of someone needs the commands separated so each can be explained as it
runs. This adds that walkthrough, in the acts-and-callouts shape the existing demo uses.

Every command was executed end-to-end against `fc757a7` before being written down, and the
outputs shown are real. Three things surfaced only by running it:

- **The harness ksvc carries three `ANTHROPIC_*` `secretKeyRef` entries.** `kubectl set env`
  does not work on a Knative `Service` at all, and a blind env-array replace flattens
  `valueFrom` to an empty string — after which the model call fails looking exactly like an
  unreachable endpoint. The flip step therefore shows those three entries *before* touching
  them and upserts by name with jq, mirroring `lib.sh`'s `set_ksvc_env`. Act 2d explains the
  whole patch, since a copy-paste demo is precisely where that trap gets sprung.
- **`docker logs` does not show individual execs** — the worker logs its attach and then only
  anomalies (dedup, req-id reuse, dropped terminal frames). Recorded under "Notes and limits"
  so nobody promises a live exec log they cannot deliver.
- **A planted marker beats the OS fingerprint as a closer.** Act 3 has you write a random
  string into the container by hand, confirm the in-cluster pool returns ENOENT for that path,
  then ask the leaf to read it. A verdict on `/etc/os-release` can be argued with; a string
  the audience watched you create cannot.

Also adds `docs/demos/README.md`, matching the index the other `docs/` genres already carry
(`adrs/`, `specs/`, `plans/`). It states the demo/smoke-test/spec distinction, since the
overlap is the thing most likely to send a future contributor to the wrong genre, and records
the structural conventions both demos follow so the next one does not have to reverse-engineer
them.

Refs #174, #198.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant