ci(kwok): add argocd-git lane with in-cluster gitea#1390
Conversation
Signed-off-by: Christopher Haar <[email protected]>
📝 WalkthroughWalkthroughThis PR introduces the Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
kwok/scripts/validate-scheduling.sh (1)
448-450:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
gitto preflight checks forargocd-git.Line 449 only requires
gitforflux-git, butargocd-gitalso callspush_bundle_to_gitea()and executesgitcommands. This currently fails later at runtime instead of failing fast in dependency preflight.Suggested fix
- [[ "$DEPLOYER" == "flux-git" ]] && required+=(git) + [[ "$DEPLOYER" == "flux-git" || "$DEPLOYER" == "argocd-git" ]] && required+=(git)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@kwok/scripts/validate-scheduling.sh` around lines 448 - 450, The preflight check currently only adds `git` to the required dependencies array when the DEPLOYER is "flux-git", but the `argocd-git` deployer also requires `git` since it calls `push_bundle_to_gitea()` which executes git commands. Extend the condition on line 449 to also require `git` when the DEPLOYER equals "argocd-git" so that missing git is caught during preflight validation instead of failing at runtime.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@kwok/scripts/validate-scheduling.sh`:
- Around line 1942-1946: The usage() function output is missing argocd-git from
the list of valid deployer options, while the validation logic at line 1942
accepts it as a valid value. Update the usage() function to include argocd-git
in both the deployer list description and any example output to ensure the CLI
help text is consistent with the actual accepted values.
---
Outside diff comments:
In `@kwok/scripts/validate-scheduling.sh`:
- Around line 448-450: The preflight check currently only adds `git` to the
required dependencies array when the DEPLOYER is "flux-git", but the
`argocd-git` deployer also requires `git` since it calls
`push_bundle_to_gitea()` which executes git commands. Extend the condition on
line 449 to also require `git` when the DEPLOYER equals "argocd-git" so that
missing git is caught during preflight validation instead of failing at runtime.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 8bdf2e0a-f35c-4f59-bc55-82ebdfcf9a3f
📒 Files selected for processing (11)
.github/actions/kwok-test/action.yml.github/workflows/kwok-recipes.yamlMakefiledocs/contributor/tests.mddocs/design/010-kwok-git-source-lanes.mdkwok/README.mdkwok/scripts/install-infra.shkwok/scripts/run-all-recipes.shkwok/scripts/validate-scheduling.shtests/chainsaw/kwok/argocd-git-sync/chainsaw-test.yamltests/chainsaw/kwok/argocd-sync/chainsaw-test.yaml
mchmarny
left a comment
mchmarny
left a comment
There was a problem hiding this comment.
Clean, well-scoped addition of the argocd-git KWOK lane — completes the #963 Git-source round-trip matrix for both GitOps controllers. No Go changes; the bundler's argocd Git output shape already existed and this wires the CI lane that protects it.
The git-mode gate is genuinely fail-closed via three independent guards, which is the thing that matters most here:
- bundle-time
repoURLgrep onapp-of-apps.yaml(rejects a silent OCI fallback before the push), - the new
assert-root-app-git-sourcechainsaw step doing an exactrepoURL == $repoURLmatch with no default binding (an unpassed value fails loud rather than vacuously passing), - the driver's empty-
GIT_IN_CLUSTER_URLguard before the sync gate.
Consumer/matrix parity is complete — every site that enumerates the deployer set is updated (Tier 1 matrix + Tier 3 list, both shell allowlists, action.yml, Makefile, install-infra.sh, run-all-recipes.sh, ADR-010, tests.md, kwok/README.md). The compute_gitea_urls() / push_bundle_to_gitea() extraction with flux-git refactored onto it is the right dedup, and the SYNC NOTE headers on both chainsaw files flag the byte-identical-steps invariant. The known assert-all-applications-pass exists-semantics weakness is correctly scoped out and tracked (#1288).
One non-blocking nit inline on the bundle-time grep (unescaped regex interpolation; the chainsaw assert is the real authority).
Not blocking, but worth confirming before merge: the 146 KWOK checks are still queued — for a CI-lane PR the lanes actually going green is the real proof, so I'd wait for Tier 1 … (argocd-git) to pass rather than merge on the wiring review alone.
2 participants
Summary
Adds the
argocd-gitKWOK deployer lane: the filesystem bundle isgit pushed tothe in-cluster Gitea and Argo CD's repo-server clones + reconciles the app-of-apps
from the Git
repoURLthe Argo CD counterpart toflux-git, completing theGit-source round-trip matrix.
Motivation / Context
The OCI lanes (
argocd-oci,argocd-helm-oci,flux-oci) andflux-git(#1290)left one documented product surface uncovered: the Argo CD filesystem/Git output
shape. With
--output ./dir --repo <git-url>, the argocd deployer bakes the GitURL into the app-of-apps and every per-component
application.yaml(targetRevision: main, path-based) — entirely different rendering from the OCI path, and previouslyexercised by no lane. This lane reuses the in-cluster Gitea infra
flux-gitalreadyestablished (same host-port 3300, same anonymous-public-repo contract).
Fixes: #963
Related: #1290 (flux-git), #843 (KWOK deployer matrix), #1288 (argocd sync-gate
exists-semantics, tracked separately)
Type of Change
Component(s) Affected
cmd/aicr,pkg/cli)cmd/aicrd,pkg/server)pkg/recipe)pkg/bundler,pkg/component/*)pkg/collector,pkg/snapshotter)pkg/validator)pkg/errors,pkg/k8s)docs/,examples/)kwok/,tests/chainsaw/kwok), CI workflow (.github/)Implementation Notes
(
resolveRepoSettings()defaultingtargetRevisiontomain,app-of-apps.yaml.tmpl, per-componentapplication.yamlrepoURL); this PR addsthe CI lane that protects it.
compute_gitea_urls()/push_bundle_to_gitea()invalidate-scheduling.shand refactored the existingflux-gitbranch onto them, so the runner→Service-DNS dual-view URL rule and thegit init/commit/push --force mainblock live in one place.install-infra.shargocd-gitcase installs Argo CD + repo secret andGitea (a superset of the
argocd-ociinstall, mirroring howflux-gitcomposesinstall_flux+install_gitea).argocd-gitreuses the existing source-agnosticargocd-syncchainsaw gate (root app
nvidia-stack), via a dedicatedargocd-git-syncsiblingthat adds one leading step asserting the root Application's
spec.source.repoURLis the Git URL — the "Git mode engaged" proof (analogue of
flux-git-sync'sGitRepository-Ready step). The shared steps stay byte-identical between the two
files (SYNC NOTE headers enforce this by convention).
credentials), exactly what the bundler's argocd output emits.
argocd-sync(assert-all-applications-pass)is out of scope here and tracked in a separate issue ([Bug]: ci(kwok): chainsaw sync gates pass when ANY resource converges, not ALL #1288 argocd half); this
lane behaves identically to the existing
argocd-oci/argocd-helm-ocilanes.Testing
# Commands run (prefer `make qualify` for non-trivial changes) make qualifyRisk Assessment
Rollout notes:
CI matrix grows by one cell per generic overlay (Tier 1) and one
deployer (Tier 3), staying under GitHub's 256-config cap. No new cluster topology —
the Gitea Service + host-port 3300 mapping shipped with #1290; clusters created
after that need no recreation. Existing argocd-oci/argocd-helm-oci/flux-* lanes
are unchanged.
Checklist
make testwith-race)make lint)git commit -S) — GPG signing info