test: integrate controller-manager image publishing + e2e GINKGO_NODES parallelism - #19
Merged
Conversation
The hardcoded ByohAgentDebPackageURL constant required a manual hand-edit every time the agent bundle changed. Replace it with a ByohAgentBundleURL() function that composes the OCI repo path with byohctl's own baked-in version.GetVersion(), so byohctl and its expected agent bundle are always tied to the same git-describe version with zero manual edits. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Version now comes from `git describe --dirty --tags --match='v*'` (overridable via `BYOH_DEB_VERSION` env var) instead of an externally- supplied `BUILD_NUMBER`. This ensures the published bundle tag matches what byohctl bakes in at build time with no manual coordination. Also, update the publish target to match the new quay.io/platform9/cluster-api-provider-bringyourownhost/agent path that byohctl now expects. This ensures the CI pipeline pushes to the same OCI registry location that the agent installer references.
Closes the loop: every push to main that passes CI now automatically builds and publishes the agent deb bundle tagged with that commit's git-describe version. This replaces the previous manual rebuild-and- hand-edit-a-constant process. Triggers via `workflow_run` off the existing CI workflow's success (avoiding a duplicate test run) plus a manual `workflow_dispatch` for dry runs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add a new `tag` target that prints GIT_VERSION, derived from git-describe-based versioning. This becomes the one place every agent-side artifact (byohctl binary, agent bundle) gets its version from, instead of each consumer independently recomputing the same git-describe expression. The tag target is distinct from the existing TAG variable, which is the separate controller-manager image tag (still static "dev" by default). Unifying controller-manager versioning is follow-up work. Matches kaapi's philosophy in ~/pf9/kaapi/Makefile. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Previously, cmd/byohctl/Makefile, .ci/build-push-agent-deb.sh, and .github/workflows/build-push-agent-bundle.yml each computed their own copy of the git-describe expression independently: git describe --dirty --tags --match='v*' 2>/dev/null || echo "v0.0.0-..." Now all three defer to the root Makefile's `make tag` target instead. This ensures exactly one place owns the version computation and every agent-side artifact gets the same version without duplication or drift. byohctl's VERSION override escape hatch (`make build VERSION=...`) still works since make tag is only consulted via ?=. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The git describe call backing make tag now passes --abbrev=8 to standardize on 8-character short SHAs instead of git's default 7 characters. The fallback git rev-parse --short=8 is updated to match, ensuring both code paths produce the same short SHA length. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The Print tag step previously computed the tag into a shell variable and exported it to $GITHUB_ENV for the build step to reuse. This is now redundant since .ci/build-push-agent-deb.sh already calls make tag itself as its own fallback, so the step now simply runs make tag and prints the value directly without the extra plumbing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This extends the same git-describe-based versioning philosophy already applied to the agent bundle to the controller-manager Docker image. .ci/build-push-controller.sh now computes IMAGE_TAG from make tag instead of a manually-set \$BYOHCM_VERSION.\$BUILD_NUMBER, and publishes to quay.io/platform9/cluster-api-provider-bringyourownhost/controller-manager (nested under the same path as the agent bundle, just a different image name) instead of the separately-named quay.io/platform9/byoh-controller-manager. The configure_docker_registry function's registry-match guard was updated to match the new default registry path; it gates whether docker login runs and would have silently skipped login otherwise. The new workflow mirrors the agent bundle workflow's trigger design (workflow_run off CI success on main, plus workflow_dispatch for manual dry runs) but requires a real Set up Go step (unlike the agent bundle workflow) since the script's go mod vendor call runs on the host. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ci.yml: Add draft filter to jobs and extend trigger types.
- Added `if: ${{ !github.event.pull_request.draft }}` to both
workflow jobs to skip execution on draft PRs.
- Added `ready_for_review` to trigger types list (default types are
opened/synchronize/reopened, which don't include the draft-to-ready
transition). This re-runs CI when a draft PR is marked ready without
requiring a new push.
build-push-agent-bundle.yml and build-push-controller-manager.yml:
Add pull_request trigger for PR builds and publishes.
- Previously these workflows only ran via workflow_run (on CI success on
main) or workflow_dispatch. Now they also trigger on non-draft PR
push (same draft-skip via `if: ${{ !github.event.pull_request.draft }}`
and same ready_for_review addition).
- This enables building and pushing real tagged artifacts on every
non-draft PR push, per explicit request. (Fork PRs will fail at quay
login due to GitHub Actions not exposing repo secrets to fork-sourced
pull_request events — a known, accepted limitation.)
Checkout ref fix for pull_request events:
- The checkout `ref` now falls back to `github.event.pull_request.head.sha`
before `github.sha`. For pull_request events, `github.sha` points at a
synthetic merge commit, not the actual PR branch's head. Using the
synthetic commit would cause `git describe` (and thus `make tag`) to
compute a different version than the same commit checked out any other
way, leading to inconsistent versioning. The explicit PR head SHA
ensures consistent version computation across all checkout contexts.
GNU Make automatically enables --print-directory whenever -C is used, printing
'make: Entering directory' and 'Leaving directory' messages. This repo's dev
Mac ships an ancient GNU Make 3.81 that does not do this, but the GitHub
Actions Ubuntu runner's newer Make (4.x) does.
In .ci/build-push-controller.sh, IMAGE_TAG=$(make -C "${project_root}" tag)
captured that chatter into IMAGE_TAG via command substitution, corrupting the
docker build -t argument and failing the controller-manager publish workflow.
The same latent bug existed in cmd/byohctl/Makefile's VERSION assignment, just
not yet triggered since byohctl had only been built locally with the old Make.
Fixed both by passing --no-print-directory to make -C. Also added a fail-loud
whitespace guard on IMAGE_TAG in the controller script so this class of bug
cannot silently corrupt a docker tag again.
Verified: reproduced the failure locally with GNU Make 4.4.1, confirmed
--no-print-directory fixes it, and re-verified go build, go test, and shell
linting all pass.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The e2e workflow previously ran all 7 Ginkgo specs sequentially in a single job, taking ~59 minutes with resource contention (shared kind cluster/docker daemon) causing 600s control-plane timeouts on 4/7 specs. Each spec is independently scoped (creates/tears down its own namespace, hosts, cluster) with no technical coupling. This branch only exercised 2 of 7 specs; the catch-all step that ran everything had been removed. Restructure the workflow to give each spec a unique Ginkgo focus label and run them as separate parallel jobs via GINKGO_FOCUS matrix (fail-fast: false). Each spec now runs in isolation without resource contention, significantly reducing total wall-clock time and preventing timeouts. Go changes add unique focus labels to specs that had none or shared a non-exclusive tag, plus a Platform9 copyright header addition (no logic changes). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
When a job uses a matrix strategy without an explicit name, GitHub Actions
auto-appends every matrix key to the job's display name, creating noise with
escaped regex characters. Setting name: e2e (${{ matrix.name }}) provides a
cleaner, consistent UI by showing just the GINKGO_FOCUS variant name.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Prior commit (633390b) split the e2e workflow into 7 GitHub Actions jobs, one per Ginkgo spec, which fixed coverage but redundantly repeated setup (docker builds, kind cluster, CAPI provider install, host-agent compile) across each job. This switch consolidates to a single job with Ginkgo's built-in GINKGO_NODES=4 process parallelism, leveraging the existing SynchronizedBeforeSuite/SynchronizedAfterSuite scaffolding already in test/e2e/e2e_suite_test.go (designed exactly for this) to share resources once. This eliminates per-job redundant setup entirely and drops the dead "Install ginkgo" step from the workflow. Workflow also bumps actions/setup-go to v7 with go-version 1.26.2 (matching go.mod directive) and enables action cache. Making specs run concurrently in one process required fixing verified concurrency bugs: hardcoded Docker container names ("byohost1"/"byohost2") and /tmp log file paths (including collisions among the templated versions) duplicated across multiple spec files, a shared /tmp kubeconfig-staging path used during container setup made race-prone by concurrent access (now unique per call), and two /tmp debug-script paths made per-process-unique via PID suffix on test failure. Trade-off accepted: resource contention moves from sequential (original 59-minute single-process run with 4/7 specs timing out) to concurrent (all specs at once on one runner, one shared docker daemon and kind API server). This is a different, unproven failure mode that requires validation with a real run. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A separate sibling workflow (build-push-controller-manager.yml) now publishes the controller-manager image to quay.io/platform9/cluster-api-provider-bringyourownhost/controller-manager instead of the old gcr.io/k8s-staging-cluster-api/cluster-api-byoh-controller. Before this commit, the e2e test config (test/e2e/config/provider.yaml) still hardcoded the old gcr.io reference with loadBehavior: mustLoad, which would prevent the locally-built image (via `make docker-build` using the Makefile's STAGING_REGISTRY/IMAGE_NAME defaults) from matching what e2e's bootstrap cluster setup expects to load, and the rendered manager Deployment manifest would reference the old name. This commit updates Makefile's STAGING_REGISTRY and IMAGE_NAME defaults, the kustomize image transformer's newName in config/manager/kustomization.yaml, and both references in test/e2e/config/provider.yaml (the mustLoad image entry and the tag-rewrite replacements rule) so they all consistently point at the new quay.io path. Intent: the image built by `make docker-build` with no overrides should be usable directly by e2e tests, with no separate image reference juggling needed. config/manager/manager.yaml's base image field was deliberately left unchanged because the kustomize image transformer always overrides it at build/deploy time regardless, verified not to affect `make deploy`/`make install`/`make publish-infra-yaml`. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Integration/testing branch combining two in-progress efforts so they can be validated together before landing on the real repo's main:
dg/predictable-agent-versioning: git-describe-based versioning for agent/byohctl artifacts, plus newbuild-push-agent-bundle.yml/build-push-controller-manager.ymlworkflows that publish images toquay.io/platform9/cluster-api-provider-bringyourownhost/*.dg/fix-all-e2e: e2e CI switched from a 7-way GINKGO_FOCUS matrix to a single job using Ginkgo's ownGINKGO_NODESparallelism (with the concurrency-safety fixes that requires), plus aligning the default controller-manager image reference (Makefile, kustomization, e2e provider config) with the new quay.io registry somake docker-build's default output is directly usable by e2e with no custom image reference juggling.Both branches were merged in cleanly (one fast-forward, one auto-merge, no conflicts).
Test plan
build-push-controller-manager.ymlruns on this PR and publishes successfullye2e-suiteruns as a single job withGINKGO_NODES=4and check whether the shared kind control plane holds up under concurrent specs (this is the main open question — resource contention moved from sequential to concurrent, unproven either way)