feat(matrix-stack): add affinity support to component workloads#1442
feat(matrix-stack): add affinity support to component workloads#1442somaz94 wants to merge 1 commit into
Conversation
| "properties": { | ||
| "nodeAffinity": { | ||
| "type": "object", | ||
| "additionalProperties": true |
There was a problem hiding this comment.
I'm fairly strongly against additionalProperties: true for these 3 objects. Unlike e.g. extraVolumes the potential options are fairly constrained. Having a strict schema that validates the structure of these is desirable IMO
| {%- endif %} | ||
| ## Supports nodeAffinity, podAffinity and podAntiAffinity. | ||
| ## More info: https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#affinity-and-anti-affinity | ||
| {{ key }}: {} |
There was a problem hiding this comment.
| {{ key }}: {} | |
| {{ key }}: | |
| nodeAffinity: {} | |
| podAffinity: {} | |
| podAntiAffinity: {} |
With commented examples above each sub-key.
Potentially the default of {} needs to differ between globally and per-component so that you've a way of unsetting/blanking out a global e.g. nodeAffinity
| {{- with (coalesce .priorityClassName $root.Values.priorityClassName) }} | ||
| priorityClassName: {{ . }} | ||
| {{- end }} | ||
| {{- with (coalesce .affinity $root.Values.affinity) }} |
There was a problem hiding this comment.
I think the 3 affinity sub-keys need to be rendered individually. At present setting $.Values.affinity.nodeAffinity.... and $.Values.synapse.affinity.podAntiAffinity... means that Synapse loses the nodeAffinity. I don't think that's correct.
There should be a way to apply a global nodeAffinity and blank it it out, be it with null or empty dict and the tests should cover it.
Ideally podAntiAffinity should have labelSelector and matchLabelKeys set by default like how we do with topologySpreadConstraints but that's a nice to have and can be added later
There was a problem hiding this comment.
These tests:
- Need to be done independently for each of the 3 affinities
- Should show that setting one of the affinities at a component level doesn't impact others at the global level
- Show that a global setting of one of the affinities can be blanked out on a per-component basis
What this does
Adds support for configuring
affinity(nodeAffinity, podAffinity and podAntiAffinity) on component workloads, plus a globalaffinityapplied to all components. A per-componentaffinityoverrides the global one.This follows the exact pattern of the already-merged
priorityClassNamesupport (#1438): a singleaffinityobject passthrough, wired through every component's source fragment (.json+.yaml.j2), the global block insub_schema_values.yaml.j2, and the_pods.tplrender ({{- with (coalesce .affinity $root.Values.affinity) }}).Why
Operators need to control pod scheduling (node/pod affinity and anti-affinity) for HA spreading, dedicated node pools, and co-location/anti-co-location — currently only
nodeSelector,tolerations,topologySpreadConstraintsandpriorityClassNameare exposed.related: #368
Notes
source/common/affinity.jsonusesadditionalProperties: falseat the top level (only the three affinity sub-keys allowed) butadditionalProperties: trueinside each sub-object, so the full Kubernetes affinity content passes through while typos in the three top-level keys are still caught.values.yaml/values.schema.jsonregenerated viascripts/assemble_helm_charts_from_fragments.sh(idempotent — regeneration produces no drift).Testing
tests/manifests/test_pod_affinity.py(default-absent / per-component / global / component-overrides-global).tests/manifestssuite green locally (2485 passed, 26 skipped).reuse lint,ruff check,towncrier checkall pass.