Skip to content

Make ToSystemAffinity deterministic by sorting MatchLabels keys#9898

Closed
Copilot wants to merge 3 commits into
mainfrom
copilot/fix-nondeterministic-affinity-ordering
Closed

Make ToSystemAffinity deterministic by sorting MatchLabels keys#9898
Copilot wants to merge 3 commits into
mainfrom
copilot/fix-nondeterministic-affinity-ordering

Conversation

Copilot AI commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Thank you for contributing to Velero!

Please add a summary of your change

ToSystemAffinity() iterated LoadAffinity.NodeSelector.MatchLabels directly. Since it's a Go map, iteration order is nondeterministic, so the generated NodeSelectorRequirement slice could come out in different orders across calls. When the resulting affinity is embedded into continuously reconciled objects (e.g. node-agent DaemonSet pod templates), an order-only diff is treated as a spec change and triggers spurious rollouts/restarts.

Changes:

  • Deterministic ordering — collect MatchLabels keys, sort.Strings them, then build requirements in sorted order. MatchExpressions ordering is already caller-defined.
  • Rationale comment — documents that the output may feed objects reconciled in a loop where order-only churn causes restarts, so callers no longer need to sort the result themselves.
  • Test — added a TestToSystemAffinity case with multiple match labels asserting sorted-by-key output.
  • Callers verifiedpkg/exposer/{generic_restore,csi_snapshot,pod_volume}.go and pkg/repository/maintenance/maintenance.go only assign the result into a Pod/Job spec; none depend on ordering.
keys := make([]string, 0, len(loadAffinity.NodeSelector.MatchLabels))
for k := range loadAffinity.NodeSelector.MatchLabels {
	keys = append(keys, k)
}
sort.Strings(keys)

for _, k := range keys {
	v := loadAffinity.NodeSelector.MatchLabels[k]
	requirements = append(requirements, corev1api.NodeSelectorRequirement{
		Key:      k,
		Values:   []string{v},
		Operator: corev1api.NodeSelectorOpIn,
	})
}

Does your change fix a particular issue?

Please indicate you've done the following:

@netlify

netlify Bot commented Jun 9, 2026

Copy link
Copy Markdown

Deploy Preview for velero canceled.

Name Link
🔨 Latest commit 323900a
🔍 Latest deploy log https://app.netlify.com/projects/velero/deploys/6a288fcd295ae60008fd0e00

Copilot AI changed the title [WIP] Fix nondeterministic affinity ordering in ToSystemAffinity Make ToSystemAffinity deterministic by sorting MatchLabels keys Jun 9, 2026
Copilot AI requested a review from kaovilai June 9, 2026 22:12
@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Comment thread pkg/util/kube/pod.go
func ToSystemAffinity(loadAffinity *LoadAffinity, volumeTopology *corev1api.NodeSelector) *corev1api.Affinity {
requirements := []corev1api.NodeSelectorRequirement{}
if loadAffinity != nil {
for k, v := range loadAffinity.NodeSelector.MatchLabels {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you could instead sort the final requirements, which looks more clear in the code

@kaovilai kaovilai closed this Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nondeterministic affinity ordering in ToSystemAffinity causes spurious spec diffs and restarts

3 participants