Merge pull request #117025 from sanposhiho/warning-in-empty-selector

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

pkg/api/pod/warnings.go

@@ -112,6 +112,11 @@ func warningsForPodSpecAndMeta(fieldPath *field.Path, podSpec *api.PodSpec, meta
 				msg,
 			))
 		}
+
+		// warn if labelSelector is empty which is no-match.
+		if t.LabelSelector == nil {
+			warnings = append(warnings, fmt.Sprintf("%s: a null labelSelector results in matching no pod", fieldPath.Child("spec", "topologySpreadConstraints").Index(i).Child("labelSelector")))
+		}
 	}
 
 	// use of deprecated annotations
@@ -254,5 +259,38 @@ func warningsForPodSpecAndMeta(fieldPath *field.Path, podSpec *api.PodSpec, meta
 	if podSpec.TerminationGracePeriodSeconds != nil && *podSpec.TerminationGracePeriodSeconds < 0 {
 		warnings = append(warnings, fmt.Sprintf("%s: must be >= 0; negative values are invalid and will be treated as 1", fieldPath.Child("spec", "terminationGracePeriodSeconds")))
 	}
+
+	if podSpec.Affinity != nil {
+		if affinity := podSpec.Affinity.PodAffinity; affinity != nil {
+			warnings = append(warnings, warningsForPodAffinityTerms(affinity.RequiredDuringSchedulingIgnoredDuringExecution, fieldPath.Child("spec", "affinity", "podAffinity", "requiredDuringSchedulingIgnoredDuringExecution"))...)
+			warnings = append(warnings, warningsForWeightedPodAffinityTerms(affinity.PreferredDuringSchedulingIgnoredDuringExecution, fieldPath.Child("spec", "affinity", "podAffinity", "preferredDuringSchedulingIgnoredDuringExecution"))...)
+		}
+		if affinity := podSpec.Affinity.PodAntiAffinity; affinity != nil {
+			warnings = append(warnings, warningsForPodAffinityTerms(affinity.RequiredDuringSchedulingIgnoredDuringExecution, fieldPath.Child("spec", "affinity", "podAntiAffinity", "requiredDuringSchedulingIgnoredDuringExecution"))...)
+			warnings = append(warnings, warningsForWeightedPodAffinityTerms(affinity.PreferredDuringSchedulingIgnoredDuringExecution, fieldPath.Child("spec", "affinity", "podAntiAffinity", "preferredDuringSchedulingIgnoredDuringExecution"))...)
+		}
+	}
+
+	return warnings
+}
+
+func warningsForPodAffinityTerms(terms []api.PodAffinityTerm, fieldPath *field.Path) []string {
+	var warnings []string
+	for i, t := range terms {
+		if t.LabelSelector == nil {
+			warnings = append(warnings, fmt.Sprintf("%s: a null labelSelector results in matching no pod", fieldPath.Index(i).Child("labelSelector")))
+		}
+	}
+	return warnings
+}
+
+func warningsForWeightedPodAffinityTerms(terms []api.WeightedPodAffinityTerm, fieldPath *field.Path) []string {
+	var warnings []string
+	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: a null labelSelector results in matching no pod", fieldPath.Index(i).Child("podAffinityTerm", "labelSelector")))
+		}
+	}
 	return warnings
 }

pkg/api/pod/warnings_test.go

@@ -401,12 +401,30 @@ func TestWarnings(t *testing.T) {
 			template: &api.PodTemplateSpec{
 				Spec: api.PodSpec{
 					TopologySpreadConstraints: []api.TopologySpreadConstraint{
-						{TopologyKey: `foo`},
+						{
-						{TopologyKey: `beta.kubernetes.io/arch`},
+							TopologyKey:   `foo`,
-						{TopologyKey: `beta.kubernetes.io/os`},
+							LabelSelector: &metav1.LabelSelector{},
-						{TopologyKey: `failure-domain.beta.kubernetes.io/region`},
+						},
-						{TopologyKey: `failure-domain.beta.kubernetes.io/zone`},
+						{
-						{TopologyKey: `beta.kubernetes.io/instance-type`},
+							TopologyKey:   `beta.kubernetes.io/arch`,
+							LabelSelector: &metav1.LabelSelector{},
+						},
+						{
+							TopologyKey:   `beta.kubernetes.io/os`,
+							LabelSelector: &metav1.LabelSelector{},
+						},
+						{
+							TopologyKey:   `failure-domain.beta.kubernetes.io/region`,
+							LabelSelector: &metav1.LabelSelector{},
+						},
+						{
+							TopologyKey:   `failure-domain.beta.kubernetes.io/zone`,
+							LabelSelector: &metav1.LabelSelector{},
+						},
+						{
+							TopologyKey:   `beta.kubernetes.io/instance-type`,
+							LabelSelector: &metav1.LabelSelector{},
+						},
 					},
 				},
 			},
@@ -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: "null LabelSelector in topologySpreadConstraints",
+			template: &api.PodTemplateSpec{
+				ObjectMeta: metav1.ObjectMeta{},
+				Spec: api.PodSpec{
+					TopologySpreadConstraints: []api.TopologySpreadConstraint{
+						{
+							LabelSelector: &metav1.LabelSelector{},
+						},
+						{
+							LabelSelector: nil,
+						},
+					},
+				},
+			},
+			expected: []string{
+				`spec.topologySpreadConstraints[1].labelSelector: a null labelSelector results in matching no pod`,
+			},
+		},
+		{
+			name: "null LabelSelector in PodAffinity",
+			template: &api.PodTemplateSpec{
+				ObjectMeta: metav1.ObjectMeta{},
+				Spec: api.PodSpec{
+					Affinity: &api.Affinity{
+						PodAffinity: &api.PodAffinity{
+							RequiredDuringSchedulingIgnoredDuringExecution: []api.PodAffinityTerm{
+								{
+									LabelSelector: &metav1.LabelSelector{},
+								},
+								{
+									LabelSelector: nil,
+								},
+							},
+							PreferredDuringSchedulingIgnoredDuringExecution: []api.WeightedPodAffinityTerm{
+								{
+									PodAffinityTerm: api.PodAffinityTerm{
+										LabelSelector: &metav1.LabelSelector{},
+									},
+								},
+								{
+									PodAffinityTerm: api.PodAffinityTerm{
+										LabelSelector: nil,
+									},
+								},
+							},
+						},
+						PodAntiAffinity: &api.PodAntiAffinity{
+							RequiredDuringSchedulingIgnoredDuringExecution: []api.PodAffinityTerm{
+								{
+									LabelSelector: &metav1.LabelSelector{},
+								},
+								{
+									LabelSelector: nil,
+								},
+							},
+							PreferredDuringSchedulingIgnoredDuringExecution: []api.WeightedPodAffinityTerm{
+								{
+									PodAffinityTerm: api.PodAffinityTerm{
+										LabelSelector: &metav1.LabelSelector{},
+									},
+								},
+								{
+									PodAffinityTerm: api.PodAffinityTerm{
+										LabelSelector: nil,
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			expected: []string{
+				`spec.affinity.podAffinity.requiredDuringSchedulingIgnoredDuringExecution[1].labelSelector: a null labelSelector results in matching no pod`,
+				`spec.affinity.podAffinity.preferredDuringSchedulingIgnoredDuringExecution[1].podAffinityTerm.labelSelector: a null labelSelector results in matching no pod`,
+				`spec.affinity.podAntiAffinity.requiredDuringSchedulingIgnoredDuringExecution[1].labelSelector: a null labelSelector results in matching no pod`,
+				`spec.affinity.podAntiAffinity.preferredDuringSchedulingIgnoredDuringExecution[1].podAffinityTerm.labelSelector: a null labelSelector results in matching no pod`,
+			},
+		},
 	}
 
 	for _, tc := range testcases {