Repository navigation
fix(flux): tune source-watcher concurrency + CPU to unblock platform install under load - #3163
Conversation
source-watcher reconciles one ArtifactGenerator per PackageSource and builds the resulting ExternalArtifacts. On the management cluster that is ~100 artifact builds during install, and the platform HelmRelease blocks until all of them are Ready. It was the only Flux controller left untuned. In the bootstrap all-in-one pod it ran without an explicit --concurrent (binary default 10) and with the shared 100m CPU request; in the production FluxInstance it was missing from the --concurrent / CPU patch that source-, kustomize- and helm-controller already receive. Under load its per-artifact builds finish at staggered times, leaving a random tail of not-yet-Ready artifacts that times out the platform install. Raise source-watcher to --concurrent=20 in both deployments. In the bootstrap all-in-one pod also give it a 500m CPU request; the pod has no CPU limits, so the request is what drives its CPU share under contention. In the production FluxInstance add it to the shared reconciler patch, so it now receives the same --requeue-dependency=5s and 2000m/2048Mi limits the other tuned controllers already have. The flux-aio module exposes a resources override for the watcher (set via CUE) but no args override, so the bootstrap --concurrent bump is a post-render patch in the Makefile. Assisted-By: Claude <[email protected]> Signed-off-by: Aleksei Sviridkin <[email protected]>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR increases the concurrency setting (--concurrent=20) and resource allocations (CPU/memory requests and limits) for the source-watcher container across the generated Flux manifest, the Makefile that regenerates it, the flux-aio CUE configuration, and the values.yaml patch target selector. Changessource-watcher concurrency and resource tuning
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: `size/M` Suggested reviewers: `androndo`, `IvanHunters` 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
size/S
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses performance bottlenecks in the Flux source-watcher controller that were causing platform installation timeouts. By increasing concurrency and CPU resource requests, the controller can now handle the high volume of concurrent artifact builds more efficiently, preventing the staggered completion times that previously triggered installation failures. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request increases the concurrency and CPU resources for the source-watcher component in the flux-aio package. Specifically, it sets --concurrent=20 and increases the CPU request to 500m for the watcher controller, updating the manifests, Makefile, Cue configuration, and Helm values accordingly. The review feedback points out that the commit scope flux used in the PR title and release note is invalid according to the repository guidelines, and suggests using fluxcd or flux-aio instead.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| - target: | ||
| kind: Deployment | ||
| name: "(kustomize-controller|helm-controller|source-controller)" | ||
| name: "(kustomize-controller|helm-controller|source-controller|source-watcher)" |
There was a problem hiding this comment.
The commit scope flux used in the PR title and release note is not a valid scope according to the repository guidelines. Please use fluxcd (under System scopes) or flux-aio (as a package-specific scope) to adhere to the Conventional Commits specification for this repository.
References
- Each commit must follow Conventional Commits format: type(scope): brief description. Valid scopes include System scopes like 'fluxcd' or package-specific scopes matching a directory under packages/. (link)
VerdictLGTM with non-blocking notes The load-bearing risk (the production Claim mismatches[PARTIAL] "It was the only Flux controller left without concurrency/CPU tuning ... now gets the same [PARTIAL] "source-watcher's support for both Caveats
Recommended follow-ups
|
IvanHunters
left a comment
IvanHunters
left a comment
There was a problem hiding this comment.
Verdict
LGTM with non-blocking notes
The load-bearing risk (the production op: replace on resources/limits failing on a missing path) is disproven at source level: flux-operator's embedded source-watcher manifest for 2.8.x ships a resources.limits block, so the patch applies cleanly. Both --concurrent and --requeue-dependency are real source-watcher flags. Remaining items are consistency/framing notes, not blockers.
Claim mismatches
[PARTIAL] "It was the only Flux controller left without concurrency/CPU tuning ... now gets the same --concurrent=20" — accurate for the production FluxInstance path, but loose for the bootstrap AIO path. In internal/fluxinstall/manifests/fluxcd.yaml the three sibling controllers run --concurrent=5 (lines 7938, 8011, 8076) and requests.cpu: 100m, while this PR sets source-watcher to --concurrent=20 and requests.cpu: 500m. So in the AIO pod source-watcher is not brought in line with its siblings — it is set 4x their concurrency and 5x their CPU request. This is defensible (source-watcher is the one on the critical path), but the "same tuning / only one left untuned" framing does not match the AIO numbers.
[PARTIAL] "source-watcher's support for both --concurrent and --requeue-dependency was confirmed against its cmd/main.go" — confirmed correct: --concurrent (default 10) and --requeue-dependency (default 5s) both exist. The PR body's "binary default --concurrent=10" is right.
Caveats
- Path asymmetry between the two install paths: the production
FluxInstancepatch (packages/system/fluxcd/values.yaml:20-34) adds BOTH--concurrent=20AND--requeue-dependency=5sto source-watcher, but the bootstrap AIO (internal/fluxinstall/manifests/fluxcd.yaml:8212) adds only--concurrent=20. The source-watcher default for--requeue-dependencyis already 5s, so this is a no-op difference in practice — worth a one-line note that the AIO omission is intentional/harmless. op: replaceverified safe: the target path/spec/template/spec/containers/0/resources/limitsexists in the source-watcher Deployment (fluxcd/source-watcherconfig/manager/deployment.yamlshipslimits: {cpu: 1000m, memory: 1Gi},requests: {cpu: 50m, memory: 64Mi}; flux-operator's embedded 2.8.x manifests carry the same block).containers/0is themanagercontainer, resulting request (50m) < limit (2000m). The patch does not fail on either fresh install or N-1 upgrade. Verified independently against both the upstream source-watcher repo and the flux-operator 2.8.0/2.8.8 embedded manifests.- Production upgrade behaviour: on an existing management cluster this change re-renders the source-watcher Deployment (new args + limits), triggering a rolling restart of the source-watcher pod. PackageSource→ExternalArtifact reconciliation pauses briefly during the restart — a benign, self-healing event, not an outage. The limits bump to 2000m/2048Mi does not change scheduling (requests unchanged at 50m), it only raises the CPU burst ceiling.
- Generation-drift verified clean for the
--concurrentinjection: the Makefileyqguard (packages/core/flux-aio/Makefile:22) is idempotent — re-running it against the committedinternal/fluxinstall/manifests/fluxcd.yamlproduces zero diff and keeps a single--concurrent=20. The CUE-drivenresourceschange (500m request) in the committed manifest was not reproduced locally (timoni not available); the CUE edit (flux-aio.cue:19-24) is well-formed and itscontrollers: watcher: resourcesmaps to the committed source-watcher block. If CI does not runmake -C packages/core/flux-aio updateand diff, a reviewer should confirm the 500m value is what the module emits. - Fresh-install schedulability: the AIO pod's summed CPU request rises by 400m (source-watcher 100m→500m). The AIO sets no CPU limit, so this only affects proportional share under saturation, not scheduling hard-fail unless the node is already near capacity.
Recommended follow-ups
- Reword the PR body's "only controller left untuned / same tuning" to acknowledge the AIO siblings run
--concurrent=5/ 100m, so the AIO source-watcher is intentionally tuned higher rather than brought to parity. - Latent fragility on the shared
op: replace(not introduced here, but now extended to source-watcher):distribution.versionis the floating range2.8.x, whileop: replacehard-requiresresources/limitsto already exist in whatever patch version flux-operator resolves. If a future 2.8.z ships any of these controllers without alimitsblock, theop: replacefails the entire FluxInstance kustomize build and stalls flux cluster-wide.op: addon the same path (which succeeds whether or not the member pre-exists) or pinning the distribution to an exact version would remove this class of risk for all four controllers at once. - Consider pinning the flux-aio module (
flux-aio.cue:8usesversion: "latest") in a separate change — pre-existing, not introduced here, but it makes the AIO regeneration non-reproducible.
…tus-patch stalls Symptom cozystack-platform HelmRelease times out at 15m on fresh install with a single PackageSource stuck in InProgress. Snapshot from #3178 CI run 28587656430: PackageSource cozystack.vm-disk-application Ready: Unknown, reason: Progressing lastTransitionTime: 2026-07-02T13:39:26Z (snapshot generated 30+ minutes later — no state change) ArtifactGenerator cozystack.vm-disk-application status.inventory: 2 items with sha256 digests status.observedSourcesDigest: populated status.conditions[Ready]: Unknown, reason: Progressing status.conditions[Reconciling]: True, reason: Progressing ExternalArtifacts are produced. Downstream Flux consumes them and its HelmReleases install fine. But PackageSourceReconciler faithfully copies the Unknown Ready condition through, so cozystack-platform (which waits on PackageSource.Ready=True) never converges. Root cause Traced in fluxcd/source-watcher:v2.1.0 log for the affected ArtifactGenerator: 13:39:28.475 ExternalArtifact vm-disk reconciled ✅ 13:39:28.641 ExternalArtifact vm-disk-rd reconciled ✅ 13:39:45.719 ❌ failed to update status: etcdserver: request timed out 13:39:45.720 ❌ Reconciler error (etcd timeout) 13:39:46.485 ✅ No drift detected, 2 artifact(s) up to date ——— nothing else for this ArtifactGenerator ——— Under first-install etcd pressure the patchStatus call times out. The reconciler returns the etcd error; controller-runtime prints the "returned both a result and non-nil error, RequeueAfter ignored" warning and requeues. The next reconcile hits the "No drift detected" short-circuit (artifacts are already up to date) and early-exits before touching status. Result: the Ready condition is stuck Unknown until either the process restarts or something forces a state change on the ArtifactGenerator. Neither happens in a real install run. This is an upstream bug in source-watcher's reconcile loop. It has been papered over once already at the runtime-tuning level in #3163 (source-watcher concurrency + CPU), but a bad etcd second during install still lands us here. Cozystack cannot afford to wait for an upstream release cycle to unblock every install. Fix PackageSourceReconciler.updateStatus now checks two invariants before falling back to copying the (potentially stuck) upstream Ready condition: 1. ArtifactGenerator.status.Inventory is non-empty 2. ArtifactGenerator.status.ObservedSourcesDigest is set Both fields are only written by source-watcher after the artifacts have actually been produced — status.Inventory carries the ExternalArtifact digests downstream Flux is already consuming. If BOTH are present AND the upstream Ready condition is still Unknown (or absent), the PackageSource Ready condition is synthesised as True with a distinctive reason `ArtifactsGeneratedAwaitingUpstreamStatus` and a message that names the upstream bug and the observable state it relied on. If the upstream Ready ever catches up (True or False) the Owns() watch re-fires the reconciler and the real condition overwrites the synthetic one — True stays True, False surfaces the real failure. Failures are NEVER masked: the synthesis path only triggers when Ready is Unknown or missing. Safety * Never synthesises True over a real False → upstream failure is surfaced unchanged. * Requires BOTH Inventory non-empty AND ObservedSourcesDigest set → a fresh ArtifactGenerator that has literally not started work cannot be prematurely marked Ready. * ObservedGeneration is stamped so a spec change re-invalidates the synthesis until the next observable-artifacts pass. * Reason string is greppable so operators can audit which PackageSources came up via the workaround. Tests Added TestArtifactGeneratorObservablyReady with 7 cases covering every branch of the predicate: - fresh generator: false - inventory only: false - digest only: false - both set + Unknown Ready: true (the actual bug) - both set + no Ready cond: true (same window pre-stamp) - both set + Ready=True: false (pass real condition) - both set + Ready=False: false (never mask failure) All pass; existing operator unit tests are unaffected. Signed-off-by: Ivan Okhotnikov <[email protected]>
…tus-patch stalls Symptom cozystack-platform HelmRelease times out at 15m on fresh install with a single PackageSource stuck in InProgress. Snapshot from #3178 CI run 28587656430: PackageSource cozystack.vm-disk-application Ready: Unknown, reason: Progressing lastTransitionTime: 2026-07-02T13:39:26Z (snapshot generated 30+ minutes later — no state change) ArtifactGenerator cozystack.vm-disk-application status.inventory: 2 items with sha256 digests status.observedSourcesDigest: populated status.conditions[Ready]: Unknown, reason: Progressing status.conditions[Reconciling]: True, reason: Progressing ExternalArtifacts are produced. Downstream Flux consumes them and its HelmReleases install fine. But PackageSourceReconciler faithfully copies the Unknown Ready condition through, so cozystack-platform (which waits on PackageSource.Ready=True) never converges. Root cause Traced in fluxcd/source-watcher:v2.1.0 log for the affected ArtifactGenerator: 13:39:28.475 ExternalArtifact vm-disk reconciled ✅ 13:39:28.641 ExternalArtifact vm-disk-rd reconciled ✅ 13:39:45.719 ❌ failed to update status: etcdserver: request timed out 13:39:45.720 ❌ Reconciler error (etcd timeout) 13:39:46.485 ✅ No drift detected, 2 artifact(s) up to date ——— nothing else for this ArtifactGenerator ——— Under first-install etcd pressure the patchStatus call times out. The reconciler returns the etcd error; controller-runtime prints the "returned both a result and non-nil error, RequeueAfter ignored" warning and requeues. The next reconcile hits the "No drift detected" short-circuit (artifacts are already up to date) and early-exits before touching status. Result: the Ready condition is stuck Unknown until either the process restarts or something forces a state change on the ArtifactGenerator. Neither happens in a real install run. This is an upstream bug in source-watcher's reconcile loop. It has been papered over once already at the runtime-tuning level in #3163 (source-watcher concurrency + CPU), but a bad etcd second during install still lands us here. Cozystack cannot afford to wait for an upstream release cycle to unblock every install. Fix PackageSourceReconciler.updateStatus now checks two invariants before falling back to copying the (potentially stuck) upstream Ready condition: 1. ArtifactGenerator.status.Inventory is non-empty 2. ArtifactGenerator.status.ObservedSourcesDigest is set Both fields are only written by source-watcher after the artifacts have actually been produced — status.Inventory carries the ExternalArtifact digests downstream Flux is already consuming. If BOTH are present AND the upstream Ready condition is still Unknown (or absent), the PackageSource Ready condition is synthesised as True with a distinctive reason `ArtifactsGeneratedAwaitingUpstreamStatus` and a message that names the upstream bug and the observable state it relied on. If the upstream Ready ever catches up (True or False) the Owns() watch re-fires the reconciler and the real condition overwrites the synthetic one — True stays True, False surfaces the real failure. Failures are NEVER masked: the synthesis path only triggers when Ready is Unknown or missing. Safety * Never synthesises True over a real False → upstream failure is surfaced unchanged. * Requires BOTH Inventory non-empty AND ObservedSourcesDigest set → a fresh ArtifactGenerator that has literally not started work cannot be prematurely marked Ready. * ObservedGeneration is stamped so a spec change re-invalidates the synthesis until the next observable-artifacts pass. * Reason string is greppable so operators can audit which PackageSources came up via the workaround. Tests Added TestArtifactGeneratorObservablyReady with 7 cases covering every branch of the predicate: - fresh generator: false - inventory only: false - digest only: false - both set + Unknown Ready: true (the actual bug) - both set + no Ready cond: true (same window pre-stamp) - both set + Ready=True: false (pass real condition) - both set + Ready=False: false (never mask failure) All pass; existing operator unit tests are unaffected. Signed-off-by: Ivan Okhotnikov <[email protected]>
…tus-patch stalls Symptom cozystack-platform HelmRelease times out at 15m on fresh install with a single PackageSource stuck in InProgress. Snapshot from #3178 CI run 28587656430: PackageSource cozystack.vm-disk-application Ready: Unknown, reason: Progressing lastTransitionTime: 2026-07-02T13:39:26Z (snapshot generated 30+ minutes later — no state change) ArtifactGenerator cozystack.vm-disk-application status.inventory: 2 items with sha256 digests status.observedSourcesDigest: populated status.conditions[Ready]: Unknown, reason: Progressing status.conditions[Reconciling]: True, reason: Progressing ExternalArtifacts are produced. Downstream Flux consumes them and its HelmReleases install fine. But PackageSourceReconciler faithfully copies the Unknown Ready condition through, so cozystack-platform (which waits on PackageSource.Ready=True) never converges. Root cause Traced in fluxcd/source-watcher:v2.1.0 log for the affected ArtifactGenerator: 13:39:28.475 ExternalArtifact vm-disk reconciled ✅ 13:39:28.641 ExternalArtifact vm-disk-rd reconciled ✅ 13:39:45.719 ❌ failed to update status: etcdserver: request timed out 13:39:45.720 ❌ Reconciler error (etcd timeout) 13:39:46.485 ✅ No drift detected, 2 artifact(s) up to date ——— nothing else for this ArtifactGenerator ——— Under first-install etcd pressure the patchStatus call times out. The reconciler returns the etcd error; controller-runtime prints the "returned both a result and non-nil error, RequeueAfter ignored" warning and requeues. The next reconcile hits the "No drift detected" short-circuit (artifacts are already up to date) and early-exits before touching status. Result: the Ready condition is stuck Unknown until either the process restarts or something forces a state change on the ArtifactGenerator. Neither happens in a real install run. This is an upstream bug in source-watcher's reconcile loop. It has been papered over once already at the runtime-tuning level in #3163 (source-watcher concurrency + CPU), but a bad etcd second during install still lands us here. Cozystack cannot afford to wait for an upstream release cycle to unblock every install. Fix PackageSourceReconciler.updateStatus now checks two invariants before falling back to copying the (potentially stuck) upstream Ready condition: 1. ArtifactGenerator.status.Inventory is non-empty 2. ArtifactGenerator.status.ObservedSourcesDigest is set Both fields are only written by source-watcher after the artifacts have actually been produced — status.Inventory carries the ExternalArtifact digests downstream Flux is already consuming. If BOTH are present AND the upstream Ready condition is still Unknown (or absent), the PackageSource Ready condition is synthesised as True with a distinctive reason `ArtifactsGeneratedAwaitingUpstreamStatus` and a message that names the upstream bug and the observable state it relied on. If the upstream Ready ever catches up (True or False) the Owns() watch re-fires the reconciler and the real condition overwrites the synthetic one — True stays True, False surfaces the real failure. Failures are NEVER masked: the synthesis path only triggers when Ready is Unknown or missing. Safety * Never synthesises True over a real False → upstream failure is surfaced unchanged. * Requires BOTH Inventory non-empty AND ObservedSourcesDigest set → a fresh ArtifactGenerator that has literally not started work cannot be prematurely marked Ready. * ObservedGeneration is stamped so a spec change re-invalidates the synthesis until the next observable-artifacts pass. * Reason string is greppable so operators can audit which PackageSources came up via the workaround. Tests Added TestArtifactGeneratorObservablyReady with 7 cases covering every branch of the predicate: - fresh generator: false - inventory only: false - digest only: false - both set + Unknown Ready: true (the actual bug) - both set + no Ready cond: true (same window pre-stamp) - both set + Ready=True: false (pass real condition) - both set + Ready=False: false (never mask failure) All pass; existing operator unit tests are unaffected. Signed-off-by: Ivan Okhotnikov <[email protected]>
What this PR does
source-watcheris the Flux controller that turns eachPackageSourceinto anExternalArtifact(oneArtifactGeneratorper source, ~100 builds on the management cluster during install). Thecozystack-platformHelmRelease blocks until all of those artifacts are Ready, so source-watcher's throughput sits on the install's critical path.It was the only Flux controller left without concurrency/CPU tuning. In the bootstrap all-in-one pod it ran on the binary default (
--concurrent=10) with the shared 100m CPU request; in the productionFluxInstanceit was absent from the reconciler patch that source-, kustomize- and helm-controller already receive. Under load its per-artifact builds finish at staggered times, leaving a random tail of not-yet-Ready artifacts that times out the platform install.A failed run's diagnostics show the shape of this: the
cozystack-platformHelmRelease timed out withtimeout waiting for: PackageSource/… status: 'InProgress'(a run-to-run varying tail — e.g.bucket-application,linstor-gui,capi-provider-core), while its ~200 ExternalArtifacts reached Ready in a staggered trickle spread over ~60 seconds rather than together. That staggered completion of independent per-artifact builds is the signature of a throughput-limited builder, not a single all-or-nothing pull.This raises source-watcher to
--concurrent=20in both install paths:internal/fluxinstall/manifests/fluxcd.yaml, regenerated fromflux-aio.cue+ theflux-aioMakefile):--concurrent=20plus a 500m CPU request. The AIO pod sets no CPU limits, so the request is what drives source-watcher's proportional CPU share when the node is saturated.FluxInstance(packages/system/fluxcd/values.yaml): source-watcher is added to the shared reconciler patch, so it now gets the same--concurrent=20,--requeue-dependency=5sand 2000m/2048Mi limits as the other tuned controllers.The
flux-aiomodule exposes a per-controller resources override for the watcher (set via CUE) but no args override, so the bootstrap--concurrentbump is a guarded post-render patch in the Makefile; the embedded manifest is the product of that pipeline. source-watcher's support for both--concurrentand--requeue-dependencywas confirmed against itscmd/main.go, and the productionop: replaceonresources/limitsis safe because the distribution ships source-watcher with aresources.limitsblock (same shape as source-controller).Optional follow-up, intentionally left out to keep this focused on the root cause: if the artifact-build tail still brushes the 10-minute platform HelmRelease timeout under load, the installer's HelmRelease install/upgrade timeout values give a safety margin.
Release note
Summary by CodeRabbit
New Features
Bug Fixes