STAC-25565 Port the beest verification trigger to GitHub Actions - #458
STAC-25565 Port the beest verification trigger to GitHub Actions#458LouisParkin wants to merge 5 commits into
Conversation
|
Blocked on an App permission, not on review. This workflow calls Order: grant |
44b78c2 to
346723d
Compare
| # branch was not overridden -- against another branch our SHA means nothing, | ||
| # so leave it unset and let beest fall back. | ||
| agent_hash="${AGENT_HASH_INPUT}" | ||
| if [ -z "${agent_hash}" ] && [ -z "${AGENT_BRANCH_INPUT}" ]; then |
There was a problem hiding this comment.
[P1] Supplying agent_branch_under_test without a hash lands here with agent_hash empty. Current beest does not resolve that branch to an image; an unset hash uses the Helm chart default, so this can pass while testing the wrong agent. Please require agent_hash_under_test for an overridden branch, or resolve and validate the selected branch commit.
There was a problem hiding this comment.
Confirmed and fixed in db081dc. You are right about the mechanism: helpers/resolve-agent-hashes.sh explicitly declines to resolve a branch ("Do not auto-resolve hashes. Leave unset to use Helm chart defaults when not provided"), and agent_branch_under_test only feeds RUN_ID and the Qase labels — agent-extra-values.j2.yml gates nodeAgent, clusterAgent and checksAgent tags on have_agent_tag, so an unset hash deployed chart defaults.
Took the resolve-and-validate option: a new first step resolves hash → overridden branch tip → github.sha to a concrete commit via the commits API, failing if it does not resolve. Every dispatch now carries hashes_under_test, so the unpinned path is gone.
| fi | ||
|
|
||
| echo "dispatching ${wf}" | ||
| gh workflow run "${wf}" "${args[@]}" |
There was a problem hiding this comment.
[P2] The GitLab job was playable only after both agent and cluster-agent manifest jobs succeeded. This standalone dispatch has no equivalent gate, so it can start an expensive beest run before those images exist or after publication failed. Please fail closed until both required image publication paths for the selected SHA have completed.
There was a problem hiding this comment.
Added a fail-closed gate in db081dc. Before dispatching, it requires a successful Publish and sign multi-architecture agent image and Publish and sign multi-architecture cluster-agent image for the resolved commit, walking the workflow runs for that head_sha. Both are needed because beest points clusterAgent at the cluster-agent image and nodeAgent/checksAgent at the agent image, all at the same <sha8>-<arch>.
Verified against the API: the current tip of stackstate-7.78.2 has no successful manifest job, so the gate refuses it — correctly, since the branch has been red since 13 Aug on the Go stdlib CVE gate (#461). Practical consequence: beest verification stays blocked until #461 lands, which is the intended behaviour rather than deploying images that do not exist.
| # gh does not report the queued run, and every beest AWS workflow shares | ||
| # one global lock, so look the run up rather than assuming it started. | ||
| sleep 10 | ||
| url="$(gh run list --repo StackVista/beest --workflow "${wf}" \ |
There was a problem hiding this comment.
[P3] This selects the latest run, not necessarily the run just dispatched. API propagation delay, another dispatch, or a different ref can make the summary link an older or unrelated run. Capture the existing run IDs before dispatch and wait for a new matching workflow and ref run.
There was a problem hiding this comment.
Fixed in db081dc. It now snapshots the run IDs for the workflow/ref before dispatch, then polls up to 150s for a run whose ID was not in that set, rather than taking the newest.
Tested the selection logic against stubbed run lists, including the case you describe where the newest run is pre-existing and the dispatched one appears below it — it now picks the new run. If none appears it emits a warning and links the workflow page instead of a wrong run.
beest_trigger_verification was the last job in the GitLab pipeline with no GitHub Actions equivalent. Every other job is covered by the STAC-25142 / STAC-25457 / STAC-25500 stack. In GitLab the job sits in the postbuild stage, needs both merge_docker_manifest jobs and is `when: manual`, passing AGENT_BRANCH_UNDER_TEST, AGENT_HASH_UNDER_TEST and TRIGGER_AGENT_X86_TESTS into the stackvista/integrations/beest project. beest has since migrated to GitHub, and its agent-x86.yml and arm.yml both expose workflow_dispatch with an agent_branch_under_test input, so the port is a cross-repo workflow dispatch rather than a pipeline trigger. beest resolves the agent image from the branch name, so the commit SHA is no longer part of its input contract; it is recorded in the run summary for traceability instead. Keeping the workflow workflow_dispatch-only preserves the GitLab `when: manual` semantics. These runs provision real EKS infrastructure in the sandbox account and share a single global concurrency lock in beest, so firing them automatically on push would queue runs behind each other and spend hours of cluster time per merge. The suite input defaults to x86, matching TRIGGER_AGENT_X86_TESTS: true; arm and both are available because beest now exposes an arm workflow that the GitLab job never reached. The scenario selector is passed through rather than re-declared, so beest stays the single owner of the valid scenario list. Requires a GitHub App credential in this repo with actions:write on StackVista/beest, provisioned via pulumi-infra: BEEST_DISPATCH_APP_CLIENT_ID (variable) and BEEST_DISPATCH_APP_PRIVATE_KEY (secret). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The first version of this workflow sent only the branch and put the SHA in the run summary, on the reasoning that beest had no hash input. That was the wrong conclusion: beest's own GitLab port dropped the input while keeping the machinery, so the missing input was a regression to fix rather than a constraint to design around. beest#61 restores it. A branch builds many images, so branch-only means always testing whichever build is newest -- there is no way to verify a specific commit or to reproduce a failure against the image that produced it. Defaults to this run's commit, matching the GitLab job's CI_COMMIT_SHA. When agent_branch_under_test points at some other branch our SHA does not exist there, so the pin is left unset and beest falls back instead of dispatching a hash that resolves to nothing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The workflow referenced BEEST_DISPATCH_APP_CLIENT_ID/PRIVATE_KEY, which are provisioned nowhere. The beest App already exists as BEEST_GH_APP_CLIENT_ID / BEEST_GH_APP_PRIVATE_KEY, matching the <PURPOSE>_GH_APP_* convention every other App credential in the estate follows. Those variables are currently bound only to the beest repo, so pulumi-infra must also bind them to stackstate-agent before this workflow can mint a token. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
beest replaces agent_hash_under_test with hashes_under_test in StackVista/beest#63, so the dispatch has to send agent=<sha> through the new field. Must land together with that PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Review feedback on #444's follow-up: three ways the dispatch could report success while doing the wrong thing. Resolve the commit under test up front. beest only pins nodeAgent, clusterAgent and checksAgent to <sha8>-<arch> when a hash reaches it, and its resolve-agent-hashes.sh deliberately refuses to resolve a branch ("Leave unset to use Helm chart defaults"). Passing agent_branch_under_test without a hash therefore deployed the chart default image, so the run could pass while testing an agent nobody selected. An overridden branch now resolves to its tip, and every dispatch carries a concrete commit. Require both multi-architecture manifest jobs to have succeeded for that commit before dispatching. The GitLab job was only playable once agent and cluster-agent had published; the standalone dispatch had no equivalent barrier and could spend a full beest run, holding the global beest AWS lock, on images that were never pushed. Select the dispatched run by diffing against the runs that existed beforehand. Taking the newest run raced with API propagation, concurrent dispatches and the shared lock, so the summary regularly linked an unrelated run. Needs actions:read and contents:read to resolve the commit and read its runs; the App token stays scoped to dispatching beest. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
db081dc to
9e83d82
Compare
LouisLotter
left a comment
There was a problem hiding this comment.
Re-review of the current head: the concrete-SHA resolution and publication gate are addressed. Two issues remain.
| args+=(--field "hashes_under_test=agent=${AGENT_HASH}") | ||
| args+=(--field "no_destroy=${NO_DESTROY}") | ||
| if [ -n "${SCENARIOS}" ]; then | ||
| args+=(--field "scenarios=${SCENARIOS}") |
There was a problem hiding this comment.
[P2] agent-x86.yml and arm.yml have disjoint scenario choices: contd-eks-x86-* versus contd-eks-arm-*. With suite=both, any non-empty architecture-specific value is forwarded to the wrong workflow, so that dispatch is rejected or constructs an invalid scenario. Please validate scenarios per suite or split the input; empty/default and all are the only shared values.
| while read -r id candidate; do | ||
| case ",${before}," in | ||
| *",${id},"*) ;; | ||
| *) url="${candidate}"; break ;; |
There was a problem hiding this comment.
[P3] Snapshotting removes old runs, but the first new run is still not necessarily this dispatch: another source can enqueue the same workflow/ref after the snapshot and be selected here. The target global concurrency creates both run records before execution, so it does not remove this race. Please correlate with a dispatch marker, or collect candidates and link only when the result is unambiguous; otherwise use the workflow page. The baseline-list failure should also fail closed rather than being hidden by || true.
Ports the last unported GitLab job,
beest_trigger_verification. Every other GitLab job now has a GitHub equivalent.Manual (
workflow_dispatch) only, matching the GitLab job, which waswhen: manual. Mints a short-lived App token scoped toactions: writeonbeestalone, then dispatchesagent-x86.yml/arm.yml.Pins the exact agent commit. A branch builds many images with different hashes, so sending only the branch would force every run onto whichever build is newest, with no way to verify a specific commit or reproduce a failure against the image that produced it. Defaults to this run's SHA, matching the GitLab job's
CI_COMMIT_SHA. Ifagent_branch_under_testnames a different branch, our SHA does not exist there, so the pin is left unset and beest falls back rather than dispatching a hash that resolves to nothing.Admin ask (blocking a live run): needs
BEEST_DISPATCH_APP_CLIENT_ID(var) andBEEST_DISPATCH_APP_PRIVATE_KEY(secret) on this repo, for an App withactions: writeonbeest. beest's own credential is repo-level there and not reachable from here.Base branch:
stackstate-7.78.2rather than the #444 stack. It shares no files or jobs with the build lanes and is manual-only, so it cannot affect any push/PR pipeline and need not queue behind those reviews. Trivial to retarget if preferred.Dispatch logic tested against a mock
ghacross the pin default, branch-override, explicit-hash, both-suites and invalid-suite paths. actionlint and zizmor clean.