Skip to content

Fix nodePlugin affinity not propagating from OperatorConfig to DaemonSet#388

Closed
ksc98 wants to merge 1 commit intoceph:mainfrom
ksc98:fix-nodeplugin-affinity-merge
Closed

Fix nodePlugin affinity not propagating from OperatorConfig to DaemonSet#388
ksc98 wants to merge 1 commit intoceph:mainfrom
ksc98:fix-nodeplugin-affinity-merge

Conversation

@ksc98
Copy link

@ksc98 ksc98 commented Jan 25, 2026

Describe what this PR does

When using the ceph-csi-operator with Rook, node affinity configured in the OperatorConfig's driverSpecDefaults.nodePlugin.affinity is ignored.

The CSI nodeplugin DaemonSet runs on all nodes regardless of the configured affinity rules.

Root cause: Rook's operator creates Driver CRs with a non-nil but empty Affinity struct:

Affinity: &corev1.Affinity{
    NodeAffinity: getNodeAffinity(pluginNodeAffinityEnv, &corev1.NodeAffinity{}),
}

The mergeDriverSpecs() function only checks if dest.Affinity == nil before merging defaults from OperatorConfig. Since the Affinity pointer is an empty struct (never nil!), the merge never happens.

Example Scenario

In my cluster, there are some nodes that won't ever run pods that need Ceph PVCs. If we don't run a DaemonSet on these nodes, we can free up some requests, which is especially beneficial for smaller nodes.

Is there anything that requires special attention

Do you have any questions?

Is the change backward compatible?

Are there concerns around backward compatibility?

Provide any external context for the change, if any.

For example:

  • Kubernetes links that explain why the change is required
  • Ceph-CSI spec related changes/catch-up that necessitates this patch
  • golang related practices that necessitates this change

Related issues

Mention any github issues relevant to this PR. Adding below line
will help to auto close the issue once the PR is merged.

Fixes: #issue_number

Future concerns

List items that are not part of the PR and do not impact it's
functionality, but are work items that can be taken up subsequently.

Checklist:

  • Commit Message Formatting: Commit titles and messages follow
    guidelines in the developer
    guide
    .
  • Reviewed the developer guide on Submitting a Pull
    Request
  • Pending release
    notes

    updated with breaking and/or notable changes for the next major release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.

@ksc98 ksc98 force-pushed the fix-nodeplugin-affinity-merge branch 5 times, most recently from 545388d to c5cc325 Compare January 25, 2026 06:52
Rook creates Driver CRs with a non-nil but empty Affinity struct.
The merge logic only checked if Affinity == nil, so the affinity
from OperatorConfig.driverSpecDefaults was never applied.

Add isAffinityEmpty() helper to check for nil or effectively empty
affinity structs, allowing the merge to properly fill in defaults.

Signed-off-by: ksc98 <kylechang96@gmail.com>
@ksc98 ksc98 force-pushed the fix-nodeplugin-affinity-merge branch from c5cc325 to 611400f Compare January 25, 2026 06:58
@ksc98 ksc98 marked this pull request as ready for review January 25, 2026 06:58
@Madhu-1
Copy link
Collaborator

Madhu-1 commented Jan 27, 2026

@ksc98 can we please fix this in Rook to keep it nil if it does not need to be set?

@ksc98
Copy link
Author

ksc98 commented Jan 27, 2026

@ksc98 can we please fix this in Rook to keep it nil if it does not need to be set?

I can definitely raise a PR there instead. Thanks for taking a look!

@ksc98
Copy link
Author

ksc98 commented Jan 27, 2026

Hi @Madhu-1, just wanted to let you know that the issue I'm describing was actually fixed in this PR (#346), which unfortunately landed just one week after the v0.4.1 (current) release!

#346

the node plugin and other Daemon sets should use the affinity specified in the driver. Example: role=storage-node where non storage-node(s) don't require the node plugin

Weirdly, the rook operator has the code to correctly create the OperatorConfig. it was added in 9d298fe

So all set here, nothing needed besides my waiting for the next release :P

@ksc98 ksc98 deleted the fix-nodeplugin-affinity-merge branch January 27, 2026 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants