feat: add affinity and nodeSelector to common daemonset config by amarkdotdev · Pull Request #2645 · NVIDIA/gpu-operator

amarkdotdev · 2026-07-16T16:40:47Z

ClusterPolicy already exposes common tolerations for operand daemonsets but there was no matching knob for affinity or nodeSelector. This adds both under spec.daemonsets and surfaces them in Helm as daemonsets.affinity and daemonsets.nodeSelector.

User nodeSelector labels are merged into the operand nodeSelector, so the existing nvidia.com/gpu.deploy.* labels stay put.

The affinity CRD field is schemaless so we accept a normal Kubernetes affinity object without inlining the full OpenAPI schema into the ClusterPolicy CRD.

Testing

go test ./controllers/... -run TestApplyCommonDaemonSetConfig

copy-pr-bot · 2026-07-16T16:40:51Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

amarkdotdev · 2026-07-28T03:23:24Z

Hey, this is waiting on CI workflow approval / first-time contributor vetting. Could a maintainer kick that off when convenient? Thanks!

rajathagasthya · 2026-07-28T13:49:25Z

/ok-to-test be752b6

amarkdotdev · 2026-07-28T19:36:58Z

@rajathagasthya Fixed the generated GPUCluster CRDs and rebased the branch onto upstream/main. make check’s generated-assets validation is now clean.

rajathagasthya · 2026-07-28T22:46:26Z

Thanks @amarkdotdev! We are in a bit of a code freeze right now for an upcoming release, so we're only taking release-critical PRs. You can expect this PR to get attention once the release is done.

amarkdotdev · 2026-07-29T03:11:33Z

@rajathagasthya ok no problem:)

Expose spec.daemonsets.affinity and spec.daemonsets.nodeSelector on ClusterPolicy and wire them through Helm. User nodeSelector labels are merged with existing operand nodeSelector entries. Use a schemaless CRD field for affinity so we do not embed the full corev1.Affinity OpenAPI schema in the ClusterPolicy CRD. Signed-off-by: amarkdotdev <amarkdotdev@users.noreply.github.com> # Conflicts: # controllers/transforms_test.go Signed-off-by: Aaron <amark@g.jct.ac.il> Signed-off-by: Aaron <amark@g.jct.ac.il>

Regenerate published GPUCluster CRDs for the common daemonset affinity and nodeSelector API fields. Signed-off-by: amarkdotdev <amarkdotdev@users.noreply.github.com> Signed-off-by: Aaron <amark@g.jct.ac.il>

coderabbitai · 2026-08-29T18:36:26Z

📝 Walkthrough

Walkthrough

ClusterPolicy.spec.daemonsets now supports optional nodeSelector and affinity settings. Helm values provide empty defaults and render configured fields. Common DaemonSet configuration merges node selectors with existing pod selectors and applies configured affinity. Tests cover selector preservation, selector merging, and affinity application.

Merge Risk: 🟡 Moderate · up to aebd5

User-provided nodeSelector values can currently overwrite protected GPU Operator selectors, potentially placing daemonset operands on unsuitable nodes or preventing expected GPU/NFD targeting. Merge should wait until those protected selectors remain authoritative.

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 1

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: 29c358f5-21e9-441c-8a35-005c6985e51e

📥 Commits

Reviewing files that changed from the base of the PR and between fa49ea6 and aebd56c.

⛔ Files ignored due to path filters (7)

api/nvidia/v1/zz_generated.deepcopy.go is excluded by !**/zz_generated.*.go
bundle/manifests/nvidia.com_clusterpolicies.yaml is excluded by !bundle/manifests/nvidia.com_*.yaml
bundle/manifests/nvidia.com_gpuclusters.yaml is excluded by !bundle/manifests/nvidia.com_*.yaml
config/crd/bases/nvidia.com_clusterpolicies.yaml is excluded by !config/crd/bases/**
config/crd/bases/nvidia.com_gpuclusters.yaml is excluded by !config/crd/bases/**
deployments/gpu-operator/crds/nvidia.com_clusterpolicies.yaml is excluded by !deployments/gpu-operator/crds/**
deployments/gpu-operator/crds/nvidia.com_gpuclusters.yaml is excluded by !deployments/gpu-operator/crds/**

📒 Files selected for processing (5)

api/nvidia/v1/clusterpolicy_types.go
controllers/object_controls.go
controllers/transforms_test.go
deployments/gpu-operator/templates/clusterpolicy.yaml
deployments/gpu-operator/values.yaml

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.

amarkdotdev · 2026-08-29T18:38:34Z

rebased

Signed-off-by: Aaron <amark@g.jct.ac.il>

kvalliyurnatt

@amarkdotdev Thanks for your contribution. Left a few comments

kvalliyurnatt · 2026-08-31T19:44:25Z

+						},
+					},
+				},
+			})


I think there is a comma missing here

Suggested change

})

}),

added the missing comma

kvalliyurnatt · 2026-08-31T19:50:38Z

+			obj.Spec.Template.Spec.NodeSelector = make(map[string]string)
+		}
+		for key, value := range config.Daemonsets.NodeSelector {
+			if strings.HasPrefix(key, "nvidia.com/gpu.deploy.") {


I think we still have a few more selectors we want to protect. For example nvidia.com/mps.capable: "true" if overriden can end up placing MPS control daemon on unsuitable nodes. I think we should make sure we protect all operand selectors here. I think we don't want to overwrite any existing NodeSelectors probably right ? maybe we need to keep the user sent node selectors to strictly be additive

nodeSelector is additive now, existing operand keys stay put

kvalliyurnatt · 2026-08-31T19:53:07Z

+	}
+
+	if config.Daemonsets.Affinity != nil {
+		obj.Spec.Template.Spec.Affinity = config.Daemonsets.Affinity


This also removes any existing affinity, I think affinity should be merged with existing operand defined affinity here.

affinity merges with the operand now instead of replacing it

kvalliyurnatt · 2026-08-31T20:10:36Z

+                  affinity:
+                    description: 'Optional: Set affinity for all Daemonsets'
+                    type: object
+                    x-kubernetes-preserve-unknown-fields: true


I understand we want affinity to be schema less to prevent pulling in the entire affinity definition, but the way we have it now makes validation difficult, would it be so bad to inline the openAPI spec ?

kept it schemaless for now to avoid dragging the full affinity openapi into the crd bundle

Only add nodeSelector keys that are not already set on the operand and merge cluster policy affinity with existing daemonset affinity. Signed-off-by: Aaron <amark@g.jct.ac.il>

kvalliyurnatt · 2026-09-01T14:46:37Z

+		if merged.RequiredDuringSchedulingIgnoredDuringExecution == nil {
+			merged.RequiredDuringSchedulingIgnoredDuringExecution = src.RequiredDuringSchedulingIgnoredDuringExecution.DeepCopy()
+		} else {
+			merged.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms = append(


NodeSelectorTerms are ORed by Kubernetes, so appending the source terms changes the result to existing OR user, which broadens scheduling, I think we want existing AND user to be additive in this case. We would need to do a cartesian product I believe of the existing terms with the new terms to make it additive.

cartesian product on the selector terms now

NodeSelectorTerms are ORed by Kubernetes. Merge existing operand terms with user terms via cartesian product so constraints are additive instead of broadening scheduling. Signed-off-by: Aaron <amark@g.jct.ac.il>

amarkdotdev requested review from cdesiniotis, karthikvetrivel, rahulait, rajathagasthya, shivamerla and tariq1890 as code owners July 16, 2026 16:40

amarkdotdev mentioned this pull request Jul 16, 2026

[Feature]: Support affinity / nodeSelector for operand DaemonSets in ClusterPolicy spec.daemonsets #2626

Open

amarkdotdev force-pushed the feat/daemonset-affinity-nodeselector-2626 branch 3 times, most recently from f0aa669 to 530835d Compare July 21, 2026 19:23

amarkdotdev force-pushed the feat/daemonset-affinity-nodeselector-2626 branch from 530835d to be752b6 Compare July 27, 2026 20:29

amarkdotdev force-pushed the feat/daemonset-affinity-nodeselector-2626 branch from be752b6 to 9efb8b7 Compare July 28, 2026 18:25

myeolenv assigned rajathagasthya Aug 24, 2026

amarkdotdev added 2 commits August 29, 2026 21:34

chore: regenerate GPUCluster CRDs

aebd56c

Regenerate published GPUCluster CRDs for the common daemonset affinity and nodeSelector API fields. Signed-off-by: amarkdotdev <amarkdotdev@users.noreply.github.com> Signed-off-by: Aaron <amark@g.jct.ac.il>

amarkdotdev force-pushed the feat/daemonset-affinity-nodeselector-2626 branch from 9efb8b7 to aebd56c Compare August 29, 2026 18:34

coderabbitai Bot reviewed Aug 29, 2026

View reviewed changes

Comment thread controllers/object_controls.go

fix: do not override gpu.deploy node selectors

76b9385

Signed-off-by: Aaron <amark@g.jct.ac.il>

myeolenv assigned kvalliyurnatt and unassigned rajathagasthya Aug 31, 2026

kvalliyurnatt requested changes Aug 31, 2026

View reviewed changes

feat: make daemonset nodeSelector and affinity additive

56390d1

Only add nodeSelector keys that are not already set on the operand and merge cluster policy affinity with existing daemonset affinity. Signed-off-by: Aaron <amark@g.jct.ac.il>

kvalliyurnatt reviewed Sep 1, 2026

View reviewed changes

fix: cartesian product on node affinity merge

c00d9f0

NodeSelectorTerms are ORed by Kubernetes. Merge existing operand terms with user terms via cartesian product so constraints are additive instead of broadening scheduling. Signed-off-by: Aaron <amark@g.jct.ac.il>

Uh oh!

Conversation

amarkdotdev commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing

Uh oh!

copy-pr-bot Bot commented Jul 16, 2026

Uh oh!

amarkdotdev commented Jul 28, 2026

Uh oh!

rajathagasthya commented Jul 28, 2026

Uh oh!

amarkdotdev commented Jul 28, 2026

Uh oh!

rajathagasthya commented Jul 28, 2026

Uh oh!

amarkdotdev commented Jul 29, 2026

Uh oh!

coderabbitai Bot commented Aug 29, 2026

Walkthrough

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

amarkdotdev commented Aug 29, 2026

Uh oh!

kvalliyurnatt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

amarkdotdev commented Jul 16, 2026 •

edited

Loading