fix(affinity): apply component-specific pod anti-affinity to aggregator and server deployments#223
fix(affinity): apply component-specific pod anti-affinity to aggregator and server deployments#223pinkneyj wants to merge 3 commits into
Conversation
Greptile SummaryThis PR fixes component-specific pod anti-affinity rules for the aggregator and server deployments, bringing them in line with the existing pattern used in
Confidence Score: 2/5The affinity template changes are correct and safe, but the new values file appears to contain a real CloudZero API key committed to the repository — do not merge until the key is confirmed fake or revoked and replaced. The two template files are a clean, correct fix. The blocking concern is charts/cloudzero-agent/values-staging-nonprod.yaml — verify whether the API key is real and whether this file belongs in version control at all.
|
| Filename | Overview |
|---|---|
| charts/cloudzero-agent/templates/agent-deploy.yaml | Adds component-specific podAntiAffinity for the server deployment; matchLabels correctly target app.kubernetes.io/name: server, matching actual pod labels. |
| charts/cloudzero-agent/templates/aggregator-deploy.yaml | Adds component-specific podAntiAffinity for the aggregator; matchLabels correctly target app.kubernetes.io/name: aggregator, matching actual pod labels. |
| charts/cloudzero-agent/values-staging-nonprod.yaml | New staging values file committed with a non-placeholder apiKey value despite an in-file warning not to do so; also exposes a cloud account ID and cluster name in version control. |
Reviews (2): Last reviewed commit: "fix trailing whitespace" | Re-trigger Greptile
|
|
||
| # WARNING: do not commit this file with a real API key | ||
| apiKey: "gn451xCSZHUCLzS2jS5gaz6d6T-_G4vj2s8cLA" | ||
|
|
There was a problem hiding this comment.
Possible real API key committed to git
The apiKey field contains a non-placeholder value (gn451xCSZHUCLzS2jS5gaz6d6T-_G4vj2s8cLA), and the inline comment on line 6 explicitly warns "do not commit this file with a real API key." If this is a genuine CloudZero API key, it should be revoked immediately and replaced with a placeholder (e.g. "REPLACE_ME") before merging. The file also embeds a cloud account ID and cluster details that should not live in version control. Consider adding values-staging-nonprod.yaml to .gitignore.
|
Rotated api key after mess up here... |
|
Hi @pinkneyj, thanks for the contribution! It's deeply appreciated. This is definitely an issue we want to get resolved before the next release. This content is just mirrored from the helm/ directory in the Cloudzero/cloudzero-agent repository, so we can't accept patches to this content here; they would just get overwritten the next time anything got merged into the cloudzero-agent repo. I've opened Cloudzero/cloudzero-agent#881 for the aggregator (using This also uncovered Cloudzero/cloudzero-agent#882 |
|
Hi @evan-cz I've still opened a PR to the other repository: Cloudzero/cloudzero-agent#898 Would it still not be better to have the affinity set automatically, as is done in webhook-server? |
By submitting a PR to this repository, you agree to the terms within the CloudZero Code of Conduct. Please see the contributing guidelines for how to create and submit a high-quality PR for this repo.
Please note that changes to the
cloudzero-agentHelm chart should be made instead in thehelm directoryin the cloudzero-agent repository, and will automatically be mirrored to this repository as soon as they are merged.Description
The aggregator-deploy.yaml template passed only defaults.affinity to the generateAffinity helper, with no component-specific anti-affinity logic. This caused any label set in defaults.affinity to be applied to all components indiscriminately — for example, setting an aggregator-targeted matchLabels in defaults would incorrectly propagate that label selector to the server deployment, making the server avoid nodes running aggregator pods rather than other server pods.
This change brings aggregator-deploy.yaml and agent-deploy.yaml (server) in line with the existing pattern used in webhook-deploy.yaml: each template now constructs a $podAntiAffinity variable with its own component-specific app.kubernetes.io/name label, merges it with any user-supplied affinity from values, and passes the result to generateAffinity. This ensures each component's pod anti-affinity rule targets the correct peer pods, and that defaults.affinity is no longer required to carry component-specific label selectors.
No breaking changes. The aggregator.affinity and server.affinity values fields continue to work as user overrides.
References
Pattern sourced from existing webhook-deploy.yaml implementation (lines 106–108)
Testing
Tested by deploying the Helm chart to a Kubernetes cluster and verifying:
Aggregator pods have matchLabels: app.kubernetes.io/name: aggregator in their anti-affinity spec
Server pods have matchLabels: app.kubernetes.io/name: server in their anti-affinity spec
Neither component inherits the other's label selector
defaults.affinity can be set to {} without affecting component-specific anti-affinity behaviour
labels can be manually set for one component without others being changed eg app.kubernetes.io/part-of: cloudzero-agent set for aggregator while others remain unchanged
This change adds test coverage for new/changed/fixed functionality
Checklist
main