Reevaluate the PodSpec equality factors

[tenzen-y]

What would you like to be added:
We want to reevaluate the PodSpec equality factors and use only the necessary factors (fields) to identify the PodSpec.

kueue/pkg/util/equality/podset.go

Lines 29 to 37 in b9e1730

	func comparePodTemplate(a, b *corev1.PodSpec, ignoreTolerations bool) bool {
	if !ignoreTolerations && !equality.Semantic.DeepEqual(a.Tolerations, b.Tolerations) {
	return false
	}
	if !equality.Semantic.DeepEqual(a.InitContainers, b.InitContainers) {
	return false
	}
	return equality.Semantic.DeepEqual(a.Containers, b.Containers)
	}

Why is this needed:
The current approach sometimes causes unexpected update detection as we discussed in #3090.
So, it would be better to adopt the selective equality evaluation as opposed to the current nonselective equality.

Before we modify the actual equality function, we need to summarize the necessary fields for developers and users.

Completion requirements:

This enhancement requires the following artifacts:

• Design doc
• API change
• Docs update

The artifacts should be linked in subsequent comments.

[tenzen-y]

cc: @alculquicondor @dgrove-oss

[alculquicondor]

Interestingly, I think we are missing some important fields, such as pod overhead.

But from within the containers, we just care about requests.

@mimowo anything else in the context of TAS?

[alculquicondor]

FYI: we need to be careful about pod group support, as we use the fields to build hashes. That list of fields will be harder to change. If we change the list and a pod group is running while there is a kueue upgrade, the new pods might be considered invalid.

[mimowo]

From the perspective of TAS I don't see a need to change this yet, but let me check my understanding.

There are 2 levels here: PodSpec and Container, and the current approach for comparing is mixed (based on the quoted comparePodTemplate function):

• selective at the level of PodSpec (only check Containers, InitContainers, and Tolerations), see the full list of fields here
• non-selective at the lower level of containers (the full list of fields here)

So, the comment in code suggests to be less selective at the PodSpec level, while the issue here suggests to be more selective at the container level?

[dgrove-oss]

/assign dgrove-oss
I'll post a comment in the issue with the proposed fields to compare within the next week or so. Assuming we converge on that, I can implement in time for 0.11.

[tenzen-y]

From the perspective of TAS I don't see a need to change this yet, but let me check my understanding.

There are 2 levels here: PodSpec and Container, and the current approach for comparing is mixed (based on the quoted comparePodTemplate function):

• selective at the level of PodSpec (only check Containers, InitContainers, and Tolerations), see the full list of fields here
• non-selective at the lower level of containers (the full list of fields here)

So, the comment in code suggests to be less selective at the PodSpec level, while the issue here suggests to be more selective at the container level?

Actually, we aim to rework the comparative in both levels.
Anyway, I believe that @dgrove-oss can represent the direction later in this PR.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[dgrove-oss]

/remove-lifecycle stale