feature(pkg/api): warning for Pod with null labelSelector in PodAffinity and TopologySpread #117025

sanposhiho · 2023-03-31T02:17:41Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

warning for Pod with null labelSelector in PodAffinity and TopologySpread.
The null lebalSelector means no-match. So, it doesn't make sense to use null labelSelector in PodAffinity or TopologySpread.

Which issue(s) this PR fixes:

Fixes #115873

Special notes for your reviewer:

Does this PR introduce a user-facing change?

The Kubernetes apiserver now emits a warning message for Pods with a null labelSelector in podAffinity or topologySpreadConstraints. The null labelSelector means "match none". Using it in podAffinity or topologySpreadConstraint could lead to unintended behavior.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

…ity and TopologySpread

k8s-ci-robot · 2023-03-31T02:17:45Z

Please note that we're already in Test Freeze for the release-1.27 branch. This means every merged PR will be automatically fast-forwarded via the periodic ci-fast-forward job to the release branch of the upcoming v1.27.0 release.

Fast forwards are scheduled to happen every 6 hours, whereas the most recent run was: Thu Mar 30 22:31:05 UTC 2023.

sanposhiho · 2023-03-31T02:17:48Z

/sig scheduling

k8s-ci-robot · 2023-03-31T02:17:50Z

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

sanposhiho · 2023-03-31T02:18:14Z

/cc @alculquicondor @ahg-g

alculquicondor · 2023-03-31T12:59:57Z

/release-note-edit

The Kubernetes apiserver now emits a warning message for Pods with a null labelSelector in podAffinity or topologySpreadConstraints. The null labelSelector means "match none". Using it in podAffinity or topologySpreadConstraint leads to unschedulable pods.

alculquicondor · 2023-03-31T13:02:50Z

/release-note-edit

The Kubernetes apiserver now emits a warning message for Pods with a null labelSelector in podAffinity or topologySpreadConstraints. The null labelSelector means "match none". Using it in podAffinity or topologySpreadConstraint could lead to unintended behavior.

alculquicondor · 2023-03-31T13:01:37Z

pkg/api/pod/warnings.go

+
+		// warn if labelSelector is empty which is no-match.
+		if t.LabelSelector == nil {
+			warnings = append(warnings, fmt.Sprintf("%s: LabelSelector mustn't be empty; it will result in matching with no object", fieldPath.Child("spec", "topologySpreadConstraints").Index(i).Child("labelSelector")))


Suggested change

warnings = append(warnings, fmt.Sprintf("%s: LabelSelector mustn't be empty; it will result in matching with no object", fieldPath.Child("spec", "topologySpreadConstraints").Index(i).Child("labelSelector")))

warnings = append(warnings, fmt.Sprintf("%s: a null labelSelector results in matching no pod", fieldPath.Child("spec", "topologySpreadConstraints").Index(i).Child("labelSelector")))

alculquicondor · 2023-03-31T13:04:22Z

pkg/api/pod/warnings.go

+	var warnings []string
+	for i, t := range terms {
+		if t.LabelSelector == nil {
+			warnings = append(warnings, fmt.Sprintf("%s: LabelSelector mustn't be empty; it will result in matching with no object", fieldPath.Index(i).Child("labelSelector")))


Suggested change

warnings = append(warnings, fmt.Sprintf("%s: LabelSelector mustn't be empty; it will result in matching with no object", fieldPath.Index(i).Child("labelSelector")))

warnings = append(warnings, fmt.Sprintf("%s: a null labelSelector results in matching no pod", fieldPath.Index(i).Child("labelSelector")))

alculquicondor · 2023-03-31T13:06:24Z

pkg/api/pod/warnings.go

+	for i, t := range terms {
+		// warn if labelSelector is empty which is no-match.
+		if t.PodAffinityTerm.LabelSelector == nil {
+			warnings = append(warnings, fmt.Sprintf("%s: LabelSelector mustn't be empty; it will result in matching with no object", fieldPath.Index(i).Child("podAffinityTerm", "labelSelector")))


same recommendation

alculquicondor · 2023-03-31T13:06:59Z

pkg/api/pod/warnings_test.go

@@ -502,6 +520,85 @@ func TestWarnings(t *testing.T) {
 				`spec.terminationGracePeriodSeconds: must be >= 0; negative values are invalid and will be treated as 1`,
 			},
 		},
+		{
+			name: "empty LabelSelector in topologySpreadConstraints",


Suggested change

name: "empty LabelSelector in topologySpreadConstraints",

name: "null LabelSelector in topologySpreadConstraints",

alculquicondor · 2023-03-31T13:07:25Z

/approve

sanposhiho · 2023-04-01T10:35:06Z

@alculquicondor Thanks. I've fixed it as you suggested.

alculquicondor · 2023-04-03T12:30:55Z

/lgtm
/approve
/label api-review

k8s-ci-robot · 2023-04-03T12:31:03Z

LGTM label has been added.

Git tree hash: 3dea8fd9b790026f9a052c4b429c897d83428ba5

msau42 · 2023-04-13T21:58:35Z

/lgtm
/approve

API changes lgtm

k8s-ci-robot · 2023-04-13T21:59:01Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alculquicondor, msau42, sanposhiho

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:

~~pkg/api/OWNERS~~ [msau42]

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

feature(pkg/api): warning for Pod with null labelSelector in PodAffin…

ffcf3ee

…ity and TopologySpread

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 31, 2023

k8s-ci-robot requested a review from smarterclayton March 31, 2023 02:17

k8s-ci-robot requested a review from thockin March 31, 2023 02:17

k8s-ci-robot added do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Mar 31, 2023

k8s-ci-robot requested review from ahg-g and alculquicondor March 31, 2023 02:18

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Mar 31, 2023

alculquicondor reviewed Mar 31, 2023

View reviewed changes

fix as suggested

e389d14

k8s-ci-robot added the api-review Categorizes an issue or PR as actively needing an API review. label Apr 3, 2023

k8s-ci-robot assigned alculquicondor Apr 3, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 3, 2023

liggitt assigned msau42 Apr 12, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 13, 2023

k8s-ci-robot merged commit c9180e4 into kubernetes:master Apr 13, 2023

k8s-ci-robot added this to the v1.28 milestone Apr 13, 2023

jaskaransarkaria mentioned this pull request Jun 5, 2024

Planning upgrade to EKS 1.28 ministryofjustice/cloud-platform#5570

Closed

sanposhiho mentioned this pull request Oct 23, 2024

Add validation for NodeSelectorRequirement's values #128212

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature(pkg/api): warning for Pod with null labelSelector in PodAffinity and TopologySpread #117025

feature(pkg/api): warning for Pod with null labelSelector in PodAffinity and TopologySpread #117025

sanposhiho commented Mar 31, 2023 •

edited by k8s-ci-robot

Loading

k8s-ci-robot commented Mar 31, 2023

sanposhiho commented Mar 31, 2023

k8s-ci-robot commented Mar 31, 2023

sanposhiho commented Mar 31, 2023

alculquicondor commented Mar 31, 2023

alculquicondor commented Mar 31, 2023

alculquicondor Mar 31, 2023

alculquicondor Mar 31, 2023

alculquicondor Mar 31, 2023

alculquicondor Mar 31, 2023

alculquicondor commented Mar 31, 2023

sanposhiho commented Apr 1, 2023

alculquicondor commented Apr 3, 2023

k8s-ci-robot commented Apr 3, 2023

msau42 commented Apr 13, 2023

k8s-ci-robot commented Apr 13, 2023

	warnings = append(warnings, fmt.Sprintf("%s: LabelSelector mustn't be empty; it will result in matching with no object", fieldPath.Child("spec", "topologySpreadConstraints").Index(i).Child("labelSelector")))
	warnings = append(warnings, fmt.Sprintf("%s: a null labelSelector results in matching no pod", fieldPath.Child("spec", "topologySpreadConstraints").Index(i).Child("labelSelector")))

	name: "empty LabelSelector in topologySpreadConstraints",
	name: "null LabelSelector in topologySpreadConstraints",

feature(pkg/api): warning for Pod with null labelSelector in PodAffinity and TopologySpread #117025

feature(pkg/api): warning for Pod with null labelSelector in PodAffinity and TopologySpread #117025

Conversation

sanposhiho commented Mar 31, 2023 • edited by k8s-ci-robot Loading

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot commented Mar 31, 2023

sanposhiho commented Mar 31, 2023

k8s-ci-robot commented Mar 31, 2023

sanposhiho commented Mar 31, 2023

alculquicondor commented Mar 31, 2023

alculquicondor commented Mar 31, 2023

alculquicondor Mar 31, 2023

Choose a reason for hiding this comment

alculquicondor Mar 31, 2023

Choose a reason for hiding this comment

alculquicondor Mar 31, 2023

Choose a reason for hiding this comment

alculquicondor Mar 31, 2023

Choose a reason for hiding this comment

alculquicondor commented Mar 31, 2023

sanposhiho commented Apr 1, 2023

alculquicondor commented Apr 3, 2023

k8s-ci-robot commented Apr 3, 2023

msau42 commented Apr 13, 2023

k8s-ci-robot commented Apr 13, 2023

sanposhiho commented Mar 31, 2023 •

edited by k8s-ci-robot

Loading