Propagate podsetupdates to jobs

[mimowo]

What type of PR is this?

/kind feature

What this PR does / why we need it:

Which issue(s) this PR fixes:

Part of #1145

Special notes for your reviewer:

For all fields we want to fail on conflict according to the KEP: https://github.com/kubernetes-sigs/kueue/tree/main/keps/1145-additional-labels.

Adjusted unit tests which tested update of nodeSelectors, because this scenario could not happen e2e.

Does this PR introduce a user-facing change?

A mechanism for AdmissionChecks to provide labels, annotations, tolerations and node selectors to the pod templates when starting a job

[k8s-ci-robot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[netlify]

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name	Link
🔨 Latest commit	85bc270
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/65293246bf1a9200086a5634

[mimowo]

Also, it looks like the PR description is outdated?

Which part of the description? I looks all accurate to me.

[mimowo]

Also, it looks like the PR description is outdated?

Which part of the description? I looks all accurate to me.

Adjusted the description due to the drop of the nodeSelectorOverwrite

[mimowo]

I think all comments are addressed.

[tenzen-y]

Overall LGTM
I left a comment for a nit.

[tenzen-y]

I'm ok with either way. If @alculquicondor would like to take another way let us any comment here.

[tenzen-y]

There shouldn't be any conflicts, because the node selectors coming from the job are used to filter out flavors. https://kueue.sigs.k8s.io/docs/concepts/resource_flavor/#resourceflavor-labels

In almost situations, resourceFlavors are created by batch admins and jobs are created by batch users. So I guess that conflicts could happen. Certainly, when users set the proper parameters for flvs and jobs, conflicts won't happen.

Actually, current implementation, each integration controller overwrites podSpec's nodeSelector with podSetInfo one (flvs):

kueue/pkg/controller/jobs/job/job_controller.go

Lines 227 to 228 in c3cfc18

	info := podSetsInfo[0]
	j.Spec.Template.Spec.NodeSelector = utilmaps.MergeKeepFirst(info.NodeSelector, j.Spec.Template.Spec.NodeSelector)

However, I think the previous implementation (8555fd3) looks a bit hacky.

...
	newNodeSelector := make(map[string]string)
	for k, v := range o.NodeSelector {
		if _, exists := podSetInfo.NodeSelectorOverwrite[k]; !exists {
			newNodeSelector[k] = v
		}
	}
	if err := utilmaps.HaveConflict(podSetInfo.NodeSelector, newNodeSelector); err != nil {
...

In this implementation, maybe utilmaps.HaveConflict(podSetInfo.NodeSelector, newNodeSelector) always returns nil, right?

[tenzen-y]

This is nice work! Thank you!
/lgtm
/approve

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 975543b57801ad9a50214e3490fafb8b5b8ef51a

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mimowo, tenzen-y

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [tenzen-y]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[alculquicondor]

/release-note-edit

A mechanism for AdmissionChecks to provide labels, annotations, tolerations and node selectors to the pod templates when starting a job