Skip to content

test: cross-repo fresh-shell last-mile E2E harness (install -> CLI -> cluster info -> push) - #214

Merged
LukasWodka merged 3 commits into
developfrom
test/install-journey-737-fresh-shell-e2e
Jul 7, 2026
Merged

test: cross-repo fresh-shell last-mile E2E harness (install -> CLI -> cluster info -> push)#214
LukasWodka merged 3 commits into
developfrom
test/install-journey-737-fresh-shell-e2e

Conversation

@LukasWodka

Copy link
Copy Markdown
Contributor

Part of #737

Summary

Adds the two-leg install-journey harness that closes the gap which let a PATH-persistence regression ship green: no existing test opens a fresh shell after install and asserts the documented next command works. distro-prereqs and e2e-cluster both assert in the same shell that ran the installer, so a binary that lands somewhere a new terminal can't see is invisible to them. This harness reproduces "customer opens a new terminal and types the next command."

Related

  • Part of tracebloc/backend#737 (keystone last-mile E2E)
  • Guards the PATH-persistence class fixed on cli's fix/install-path-persist branch
  • Leg 2 reproduces the namespace/context incident addressed by client#208 (installer sets the kube context — already merged)
  • A thin cli-side caller (separate PR) points Leg 1 at a cli PR's install.sh for pre-merge cross-repo coverage

What's in here

Leg 1 — scripts/tests/path-persist.sh (cheap, wide, no cluster, no creds)
Runs in a plain distro container. Installs the tracebloc CLI via cli's install.sh (ref configurable via TRACEBLOC_CLI_REF — URL or local path), then for each shell present among bash / zsh / fish spawns a fresh login AND non-login shell and asserts command -v tracebloc resolves and tracebloc version runs. A fresh non-login bash reads ~/.bashrc, not~/.profile / ~/.bash_profile — so this is the cell that goes red on the pre-fix installer and green on the fixed one. Prints a PASS/FAIL line per shell x mode cell.

Leg 2 — scripts/tests/e2e-journey.sh (full journey on a real cluster, amd64, no secrets)
Extends the e2e-cluster.sh pattern (sources common.sh / setup-linux.sh / cluster.sh, isolated CLUSTER_NAME, TRACEBLOC_NO_AUTOSTART=1):

  1. create_cluster() + wait nodes Ready.
  2. Install the CLI via install.sh.
  3. Apply a credential-free stub matching the CLI's real discovery contract (internal/cluster/discover.go): a *-jobs-manager Deployment carrying app.kubernetes.io/name=client + app.kubernetes.io/managed-by=Helm + instance/version/chart labels (image registry.k8s.io/pause:3.9), plus an ingestor ServiceAccount (so the cluster info TokenRequest path doesn't exit 5). Point the kubeconfig context's namespace at it and assert tracebloc cluster info(a) succeeds and (b) succeeds from a fresh login + non-login shell.
  4. tracebloc dataset push --dry-run smoke on a tiny sample CSV (offline-validatable; no creds).
  5. Teardown via EXIT trap.

Every long step runs under a watchdog timeout so a hang FAILS (exit 124 -> hard error) instead of spinning to the GitHub ceiling.

Note on the stub labels: the issue's shorthand says app: manager; the CLI actually discovers on app.kubernetes.io/name=client,app.kubernetes.io/managed-by=Helm + a *-jobs-manager Deployment name. The manifest uses the real selector so the assertion exercises the actual code path, and keeps app: manager as a cosmetic extra label. Documented inline.

Note on context-on-default: client#208 (installer sets the kube context) is merged, so the supported state is context-namespace == workspace namespace — that's the core assertion. The opposite case (context left on default, CLI auto-discovers across namespaces) needs a CLI change that is not merged yet, so it runs as a non-fatal pending probe, flippable to a hard assertion via TB_EXPECT_NS_AUTODISCOVER=1 once that lands.

CI (.github/workflows/installer-tests.yaml)

  • path-persistdistro matrix (ubuntu:22.04 / 24.04, debian:12, fedora:latest, almalinux:9, opensuse/leap:15.6, alpine:3), fail-fast: false, one fresh container per distro; the script iterates shell x mode inside. Runs on the same scripts/** paths as the rest of the file.
  • e2e-journey — amd64, gated to nightly schedule + workflow_dispatch + the e2e PR label (mirrors cli's e2e.yml label gating), to control cluster cost.
  • Both new scripts added to the static job's shellcheck list (error + advisory passes).

Type of change

  • Tech-debt / refactor (test infrastructure)
  • Feature
  • Bug fix
  • Docs
  • Security / hardening
  • Breaking change

Test plan — Verified locally / Needs CI

Verified locally (green):

  • bash -n on path-persist.sh and e2e-journey.sh — both parse.
  • shellcheck --severity=error (the CI gate) on both new scripts — no findings; --severity=warning advisory pass — also clean.
  • actionlint on installer-tests.yamlclean (also runs shellcheck on the inline run blocks).
  • yaml.safe_load on the workflow — parses.
  • Isolated sanity-check of the fresh-shell assertion mechanism (capture command -v output, check non-empty + version exit code): positive case resolves a real on-PATH binary, negative control (PATH stripped) correctly yields empty -> the guard would fail. Confirms the assert logic.

Needs CI (requires GitHub runners / Docker — NOT run locally, not claimed to pass):

  • The full path-persist distro x shell x mode matrix (needs per-distro containers + zsh/fish install).
  • e2e-journey end to end (needs a real k3d cluster on a Linux runner with Docker, plus a reachable cli install.sh).
  • The "red on the pre-fix installer / green on the fixed one" proof — needs the matrix to actually run against both installer refs.

The default TRACEBLOC_CLI_REF currently points at cli's fix/install-path-persist raw install.sh (with an inline TODO(cli#61) to switch to releases/latest/download/install.sh once the fix ships in a public release — otherwise the guard would test the old installer from the latest release and report a false red).

Checklist

  • Tests added (this PR is the tests)
  • No secrets / credentials in the diff (stub is credential-free)
  • Customer identifiers scrubbed — generic phrasing only in added lines
  • Docs updated if behavior or config changed (N/A — test-only)

@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

👋 Heads-up — Code review queue is at 17 / 8

Above the WIP limit. The team convention is to review existing PRs before opening new work.

Open PRs currently in Code review (oldest first):

Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.)

LukasWodkaand others added 2 commits July 3, 2026 17:27
Adds the two-leg install-journey harness that closes the gap which let a
PATH-persistence regression ship green: no existing test opens a fresh
shell after install and asserts the documented next command works.
Leg 1 — scripts/tests/path-persist.sh: runs in a plain distro container,
installs the tracebloc CLI via cli/install.sh (configurable ref via
TRACEBLOC_CLI_REF), then for each shell among bash/zsh/fish spawns a fresh
login AND non-login shell and asserts `command -v tracebloc` resolves and
`tracebloc version` runs. A fresh non-login bash reads ~/.bashrc (not
~/.profile), so this catches the whole PATH-persistence class — red on the
pre-fix installer, green on the fixed one.
Leg 2 — scripts/tests/e2e-journey.sh: extends the e2e-cluster pattern.
Brings the cluster up via create_cluster(), installs the CLI, applies a
credential-free stub matching the CLI's real discovery contract (a
*-jobs-manager Deployment with the chart's hallmark labels + an `ingestor`
ServiceAccount), points the kubeconfig context's namespace at it, and
asserts `tracebloc cluster info` succeeds AND resolves from a fresh shell,
then `dataset push --dry-run` on a tiny sample CSV. Long steps run under a
watchdog timeout so a hang fails instead of spinning. The context-on-default
namespace auto-discover sub-assertion is gated pending the CLI change.
CI: new path-persist job (distro matrix, like distro-prereqs, fail-fast
false) and e2e-journey job (amd64, nightly + `e2e` label only, mirroring
cli's e2e.yml gating). Both new scripts added to the static shellcheck list.
Part of #737.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cli#61's PATH-persistence fix shipped in cli v0.3.1 and is in every
release since (verified: the served releases/latest install.sh persists
PREFIX to ~/.bashrc / ~/.zshrc / fish config). Switch both legs' default
TRACEBLOC_CLI_REF from the now-merged fix/install-path-persist branch raw
URL to https://github.com/tracebloc/cli/releases/latest/download/install.sh
and drop the TODO(cli#61) — the guard now exercises the shipped installer
and would go red if a future release regressed the PATH class.
Re-validated the e2e-journey stub against the current CLI discovery
contract (internal/cluster/discover.go @ v0.5.1): selector
app.kubernetes.io/name=client,app.kubernetes.io/managed-by=Helm, a
*-jobs-manager Deployment, instance/version labels, and the `ingestor`
SA for the cluster-info TokenRequest — unchanged, stub still valid.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@saadqbal
saadqbalforce-pushed the test/install-journey-737-fresh-shell-e2e branch from 3fa1a8f to 1b98ba1CompareJuly 3, 2026 12:29
The path-persist matrix ran `docker run <distro> bash path-persist.sh` as the
container entrypoint. Alpine's base image ships only busybox `sh` — no bash — so
the container died at init (exit 127, "bash: executable file not found") before a
single line of the script ran. The Alpine cell was never testing the installer;
it was failing at container startup. The script's own apk-aware _pm_install can't
help — you need bash to reach the code that installs bash.
Wrap the entrypoint in a POSIX `sh -c` that ensures bash then execs the script.
`command -v bash` short-circuits on every glibc distro (bash already present); the
apk branch fires only on Alpine. Now the Alpine cell exercises the real installer.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodka
LukasWodka requested a review from saadqbalJuly 7, 2026 11:05
@saadqbal
saadqbal requested review from divyasinghds and removed request for saadqbalJuly 7, 2026 11:06
@saadqbal
saadqbal marked this pull request as ready for review July 7, 2026 11:07
@LukasWodka
LukasWodka merged commit 947f79e into developJul 7, 2026
32 checks passed
@aptracebloc

Copy link
Copy Markdown
Contributor

cc @LukasWodka@saadqbal — post-merge review of this PR. The design and documentation are genuinely excellent (cost-tiered legs, real discovery contract in the stub, honest test plan), but there are two significant issues that should get a follow-up PR, one of which defeats Leg 2 entirely.

🔴 Critical: guard() swallows every failure — e2e-journey.sh can never go red

if! timeout --kill-after=15s "$secs""$@";thenlocal rc=$?

In bash, $? inside the then branch of if ! cmd is the exit status of the negated condition — always 0 when the branch is entered (verified empirically). So rc captures 0, the rc -eq 124 hang check never fires, and guardreturns 0 on failure — which also means the script's set -e never triggers. Since every assertion runs under guard (including tracebloc cluster info and the --dry-run smoke), the job prints E2E JOURNEY PASS even if every step fails. The watchdog also can't do the one thing it promises: turn hangs into red failures.

Fix:

local rc=0
timeout --kill-after=15s "$secs""$@"|| rc=$?
[[ $rc-eq 124 ]] && error "step '${label}' exceeded ${secs}s — treating the hang as a failure."return$rc

Note bash -n and shellcheck both pass on the buggy version — worth adding a negative-control run (point the journey at a command that exits non-zero, confirm the job fails) as part of validating the fix.

🟠 High: the fresh-shell mechanism in path-persist.sh likely produces no signal (and false negatives)

Two compounding issues:

  1. Non-interactive shells don't read rc files. "A fresh non-login bash reads ~/.bashrc" is only true for interactive non-login shells. bash -c '...' is non-interactive and reads no rc files; zsh -c reads only .zshenv; zsh -lc reads .zprofile/.zlogin but not.zshrc. So the cells don't reproduce "customer opens a new terminal" (an interactive shell), and a correctly fixed installer that persists PATH to ~/.bashrc/~/.zshrc would still fail the non-login cells whenever the binary lands outside the default PATH — a false red. The interactive cells should use bash -ic / zsh -ic (fish is fine as commented).
  2. Everything runs as root, so install.sh installs to /usr/local/bin — already on every default PATH. The rc-file persistence path (the ~/.local/bin fallback where the cli#61 class actually manifests) is never exercised, so all cells pass trivially on both the pre-fix and fixed installer. To get the promised "red on pre-fix / green on fixed" signal, at least part of the matrix should install as a non-root user (or force the installer's prefix to a non-default-PATH directory).

The PR honestly flags that the red/green proof hasn't run — that proof should gate the follow-up, because as written the matrix probably can't distinguish the two installers.

🟡 Medium

  • No timeout-minutes on either new job.static sets one; path-persist (7 containers doing package installs) and e2e-journey have nothing between them and the 6-hour ceiling — doubly so while guard() is broken. install_kubectl / install_k3d / install_helm in Leg 2 also run unguarded.
  • Label gating won't trigger on labeling. The workflow's pull_request trigger uses default types (opened, synchronize, reopened), so adding the e2e label won't start a run until the next push. Add types: [opened, synchronize, reopened, labeled] if label-to-run is the intended flow.

⚪ Low / docs drift

  • The PR body says the default TRACEBLOC_CLI_REF points at cli's fix/install-path-persist branch with a TODO(cli#61); the merged scripts default to releases/latest/download/install.sh (fix shipped in v0.3.1). Body is stale relative to the code.
  • "Nightly schedule" is inaccurate twice over: the workflow's schedule is weekly (Mondays 03:00 UTC), and if: github.event_name != 'pull_request' also runs e2e-journey on every push to main/develop/openshift touching scripts/** — broader than the described nightly + dispatch + label gating.

What's good

  • Clear cost-tiered design; fail-fast: false matrix; isolated CLUSTER_NAME + TRACEBLOC_NO_AUTOSTART + EXIT-trap teardown mirroring e2e-cluster.sh.
  • The stub manifest targets the CLI's real discovery contract (labels + *-jobs-manager name + ingestor SA) rather than the issue's app: manager shorthand, with the discrepancy documented inline.
  • The non-fatal pending probe for unmerged CLI auto-discovery (TB_EXPECT_NS_AUTODISCOVER=1) is a nice pattern — avoids both false reds and silent gaps.
  • No secrets, read-only mounts, permissions: contents: read.

Since this is already on develop: the guard() fix is one line and I'd prioritize it — until it lands, e2e-journey green runs assert nothing.

🤖 Generated with Claude Code

saadqbal added a commit that referenced this pull request Jul 7, 2026
… label trigger (#310) (#311)
The install-journey harness merged in #214 asserted nothing: guard()
captured `local rc=$?` inside the `then` branch of `if ! timeout ...`,
so rc read the NEGATED status (always 0). Every assertion runs under
guard, so set -e never tripped and the job printed E2E JOURNEY PASS even
when steps failed or hung. Reproduced: a step exiting 3 and a 124 hang
both returned 0.
- guard(): run the command, capture status via `|| rc=$?`, then act on
it — a failed step now propagates, a 124 hang is treated as a hard
failure.
- Add a negative-control self-test at startup: guard() must return
non-zero for a command that exits non-zero, or the script refuses to
run. Locks the vacuous-pass class so it can't regress silently
(bash -n and shellcheck both passed on the buggy version).
- installer-tests.yaml: add timeout-minutes to path-persist (20) and
e2e-journey (30) — every other job already had one; and add `labeled`
to the pull_request trigger types so adding the `e2e` label starts a
run immediately instead of on the next push.
Remaining from the #214 review (path-persist has no red/green signal)
is tracked separately on #310 — it's a genuine rewrite that needs CI
validation across the distro matrix.
Refs #310. Follow-up to #214 (review by @aptracebloc).
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
divyasinghds pushed a commit that referenced this pull request Jul 7, 2026
…ix + interactive per-shell (#310) (#313)
As merged in #214 the path-persist guard could not distinguish a fixed
installer from a broken one:
- It ran as root, so cli/install.sh landed the binary in /usr/local/bin
(on every PATH). The installer only PERSISTS a PATH entry for an
off-PATH prefix, so no rc was ever written and every shell resolved the
binary trivially — green on a broken installer too.
- The login × non-login matrix mis-modelled 'open a new terminal': a
non-interactive `bash -c` reads NO rc file, and a *login* bash reads
~/.profile, not the ~/.bashrc the installer writes on Linux.
Rewrite to actually exercise persistence:
- Pin INSTALL_PREFIX=$HOME/.local/bin (a $HOME dir the installer always
persists, off the default root PATH). Precondition-check it's not
already on PATH, else bail rather than assert nothing.
- install.sh persists to ONE rc keyed off $SHELL, so install once per
shell with SHELL=bash/zsh/fish, then assert a fresh INTERACTIVE shell of
that kind (bash -ic / zsh -ic / fish -ic) — the one that reads that rc.
- Per-shell negative control (bash/zsh): a fresh NON-interactive shell
must NOT resolve the binary; if it does, the prefix leaked onto the base
PATH and a pass would be meaningless. (Skipped for fish: fish_add_path
writes a universal var and `fish -c` still sources config.fish.)
Verified locally: with an isolated binary name (the dev host has a real
tracebloc on PATH), a non-interactive shell does NOT resolve it while a
fresh interactive shell does + `version` runs. bash -n, shellcheck
(error + warning), actionlint all clean. The full distro × shell matrix
is CI-only (real cli binary download + per-distro shell installs).
Fixes#310. Follow-up to #214.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
@saadqbal
saadqbal deleted the test/install-journey-737-fresh-shell-e2e branch July 9, 2026 11:41
Sign up for freeto 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.

4 participants

@LukasWodka@aptracebloc@divyasinghds@saadqbal