Merge pull request #49976 from aveshagarwal/master-pod-affinities-topology-key

Automatic merge from submit-queue (batch tested with PRs 50531, 50853, 49976, 50939, 50607)

Do not allow empty topology key for pod affinities.

**What this PR does / why we need it**:
This PR do not allow empty topology key for all 4 pod affinities.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #

**Special notes for your reviewer**:

**Release note**:

```release-note
Affinity in annotations alpha feature is no longer supported in 1.8. Anyone upgrading from 1.7 with AffinityInAnnotation feature enabled must ensure pods (specifically with pod anti-affinity PreferredDuringSchedulingIgnoredDuringExecution) with empty TopologyKey fields must be removed before upgrading to 1.8.
```
@kubernetes/sig-scheduling-bugs  @bsalamat @davidopp
This commit is contained in:
Kubernetes Submit Queue 2017-08-21 15:46:20 -07:00 committed by GitHub
commit 0f8eaa45dd
4 changed files with 49 additions and 29 deletions

View File

@ -2034,9 +2034,7 @@ type PodAffinityTerm struct {
// the labelSelector in the specified namespaces, where co-located is defined as running on a node // the labelSelector in the specified namespaces, where co-located is defined as running on a node
// whose value of the label with key topologyKey matches that of any node on which any of the // whose value of the label with key topologyKey matches that of any node on which any of the
// selected pods is running. // selected pods is running.
// For PreferredDuringScheduling pod anti-affinity, empty topologyKey is interpreted as "all topologies" // Empty topologyKey is not allowed.
// ("all topologies" here means all the topologyKeys indicated by scheduler command-line argument --failure-domains);
// for affinity and for RequiredDuringScheduling pod anti-affinity, empty topologyKey is not allowed.
// +optional // +optional
TopologyKey string TopologyKey string
} }

View File

@ -2435,7 +2435,7 @@ func ValidatePreferredSchedulingTerms(terms []api.PreferredSchedulingTerm, fldPa
} }
// validatePodAffinityTerm tests that the specified podAffinityTerm fields have valid data // validatePodAffinityTerm tests that the specified podAffinityTerm fields have valid data
func validatePodAffinityTerm(podAffinityTerm api.PodAffinityTerm, allowEmptyTopologyKey bool, fldPath *field.Path) field.ErrorList { func validatePodAffinityTerm(podAffinityTerm api.PodAffinityTerm, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{} allErrs := field.ErrorList{}
allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(podAffinityTerm.LabelSelector, fldPath.Child("matchExpressions"))...) allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(podAffinityTerm.LabelSelector, fldPath.Child("matchExpressions"))...)
@ -2444,32 +2444,29 @@ func validatePodAffinityTerm(podAffinityTerm api.PodAffinityTerm, allowEmptyTopo
allErrs = append(allErrs, field.Invalid(fldPath.Child("namespace"), name, msg)) allErrs = append(allErrs, field.Invalid(fldPath.Child("namespace"), name, msg))
} }
} }
if !allowEmptyTopologyKey && len(podAffinityTerm.TopologyKey) == 0 { if len(podAffinityTerm.TopologyKey) == 0 {
allErrs = append(allErrs, field.Required(fldPath.Child("topologyKey"), "can only be empty for PreferredDuringScheduling pod anti affinity")) allErrs = append(allErrs, field.Required(fldPath.Child("topologyKey"), "can not be empty"))
} }
if len(podAffinityTerm.TopologyKey) != 0 { return append(allErrs, unversionedvalidation.ValidateLabelName(podAffinityTerm.TopologyKey, fldPath.Child("topologyKey"))...)
allErrs = append(allErrs, unversionedvalidation.ValidateLabelName(podAffinityTerm.TopologyKey, fldPath.Child("topologyKey"))...)
}
return allErrs
} }
// validatePodAffinityTerms tests that the specified podAffinityTerms fields have valid data // validatePodAffinityTerms tests that the specified podAffinityTerms fields have valid data
func validatePodAffinityTerms(podAffinityTerms []api.PodAffinityTerm, allowEmptyTopologyKey bool, fldPath *field.Path) field.ErrorList { func validatePodAffinityTerms(podAffinityTerms []api.PodAffinityTerm, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{} allErrs := field.ErrorList{}
for i, podAffinityTerm := range podAffinityTerms { for i, podAffinityTerm := range podAffinityTerms {
allErrs = append(allErrs, validatePodAffinityTerm(podAffinityTerm, allowEmptyTopologyKey, fldPath.Index(i))...) allErrs = append(allErrs, validatePodAffinityTerm(podAffinityTerm, fldPath.Index(i))...)
} }
return allErrs return allErrs
} }
// validateWeightedPodAffinityTerms tests that the specified weightedPodAffinityTerms fields have valid data // validateWeightedPodAffinityTerms tests that the specified weightedPodAffinityTerms fields have valid data
func validateWeightedPodAffinityTerms(weightedPodAffinityTerms []api.WeightedPodAffinityTerm, allowEmptyTopologyKey bool, fldPath *field.Path) field.ErrorList { func validateWeightedPodAffinityTerms(weightedPodAffinityTerms []api.WeightedPodAffinityTerm, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{} allErrs := field.ErrorList{}
for j, weightedTerm := range weightedPodAffinityTerms { for j, weightedTerm := range weightedPodAffinityTerms {
if weightedTerm.Weight <= 0 || weightedTerm.Weight > 100 { if weightedTerm.Weight <= 0 || weightedTerm.Weight > 100 {
allErrs = append(allErrs, field.Invalid(fldPath.Index(j).Child("weight"), weightedTerm.Weight, "must be in the range 1-100")) allErrs = append(allErrs, field.Invalid(fldPath.Index(j).Child("weight"), weightedTerm.Weight, "must be in the range 1-100"))
} }
allErrs = append(allErrs, validatePodAffinityTerm(weightedTerm.PodAffinityTerm, allowEmptyTopologyKey, fldPath.Index(j).Child("podAffinityTerm"))...) allErrs = append(allErrs, validatePodAffinityTerm(weightedTerm.PodAffinityTerm, fldPath.Index(j).Child("podAffinityTerm"))...)
} }
return allErrs return allErrs
} }
@ -2483,13 +2480,11 @@ func validatePodAntiAffinity(podAntiAffinity *api.PodAntiAffinity, fldPath *fiel
// fldPath.Child("requiredDuringSchedulingRequiredDuringExecution"))...) // fldPath.Child("requiredDuringSchedulingRequiredDuringExecution"))...)
//} //}
if podAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution != nil { if podAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution != nil {
// empty topologyKey is not allowed for hard pod anti-affinity allErrs = append(allErrs, validatePodAffinityTerms(podAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution,
allErrs = append(allErrs, validatePodAffinityTerms(podAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution, false,
fldPath.Child("requiredDuringSchedulingIgnoredDuringExecution"))...) fldPath.Child("requiredDuringSchedulingIgnoredDuringExecution"))...)
} }
if podAntiAffinity.PreferredDuringSchedulingIgnoredDuringExecution != nil { if podAntiAffinity.PreferredDuringSchedulingIgnoredDuringExecution != nil {
// empty topologyKey is allowed for soft pod anti-affinity allErrs = append(allErrs, validateWeightedPodAffinityTerms(podAntiAffinity.PreferredDuringSchedulingIgnoredDuringExecution,
allErrs = append(allErrs, validateWeightedPodAffinityTerms(podAntiAffinity.PreferredDuringSchedulingIgnoredDuringExecution, true,
fldPath.Child("preferredDuringSchedulingIgnoredDuringExecution"))...) fldPath.Child("preferredDuringSchedulingIgnoredDuringExecution"))...)
} }
return allErrs return allErrs
@ -2504,13 +2499,11 @@ func validatePodAffinity(podAffinity *api.PodAffinity, fldPath *field.Path) fiel
// fldPath.Child("requiredDuringSchedulingRequiredDuringExecution"))...) // fldPath.Child("requiredDuringSchedulingRequiredDuringExecution"))...)
//} //}
if podAffinity.RequiredDuringSchedulingIgnoredDuringExecution != nil { if podAffinity.RequiredDuringSchedulingIgnoredDuringExecution != nil {
// empty topologyKey is not allowed for hard pod affinity allErrs = append(allErrs, validatePodAffinityTerms(podAffinity.RequiredDuringSchedulingIgnoredDuringExecution,
allErrs = append(allErrs, validatePodAffinityTerms(podAffinity.RequiredDuringSchedulingIgnoredDuringExecution, false,
fldPath.Child("requiredDuringSchedulingIgnoredDuringExecution"))...) fldPath.Child("requiredDuringSchedulingIgnoredDuringExecution"))...)
} }
if podAffinity.PreferredDuringSchedulingIgnoredDuringExecution != nil { if podAffinity.PreferredDuringSchedulingIgnoredDuringExecution != nil {
// empty topologyKey is not allowed for soft pod affinity allErrs = append(allErrs, validateWeightedPodAffinityTerms(podAffinity.PreferredDuringSchedulingIgnoredDuringExecution,
allErrs = append(allErrs, validateWeightedPodAffinityTerms(podAffinity.PreferredDuringSchedulingIgnoredDuringExecution, false,
fldPath.Child("preferredDuringSchedulingIgnoredDuringExecution"))...) fldPath.Child("preferredDuringSchedulingIgnoredDuringExecution"))...)
} }
return allErrs return allErrs

View File

@ -4747,8 +4747,8 @@ func TestValidatePod(t *testing.T) {
}), }),
}, },
}, },
"invalid pod affinity, empty topologyKey is not allowed for hard pod affinity": { "invalid hard pod affinity, empty topologyKey is not allowed for hard pod affinity": {
expectedError: "can only be empty for PreferredDuringScheduling pod anti affinity", expectedError: "can not be empty",
spec: api.Pod{ spec: api.Pod{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Name: "123", Name: "123",
@ -4774,8 +4774,8 @@ func TestValidatePod(t *testing.T) {
}), }),
}, },
}, },
"invalid pod anti-affinity, empty topologyKey is not allowed for hard pod anti-affinity": { "invalid hard pod anti-affinity, empty topologyKey is not allowed for hard pod anti-affinity": {
expectedError: "can only be empty for PreferredDuringScheduling pod anti affinity", expectedError: "can not be empty",
spec: api.Pod{ spec: api.Pod{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Name: "123", Name: "123",
@ -4801,8 +4801,8 @@ func TestValidatePod(t *testing.T) {
}), }),
}, },
}, },
"invalid pod anti-affinity, empty topologyKey is not allowed for soft pod affinity": { "invalid soft pod affinity, empty topologyKey is not allowed for soft pod affinity": {
expectedError: "can only be empty for PreferredDuringScheduling pod anti affinity", expectedError: "can not be empty",
spec: api.Pod{ spec: api.Pod{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Name: "123", Name: "123",
@ -4831,6 +4831,36 @@ func TestValidatePod(t *testing.T) {
}), }),
}, },
}, },
"invalid soft pod anti-affinity, empty topologyKey is not allowed for soft pod anti-affinity": {
expectedError: "can not be empty",
spec: api.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "123",
Namespace: "ns",
},
Spec: validPodSpec(&api.Affinity{
PodAntiAffinity: &api.PodAntiAffinity{
PreferredDuringSchedulingIgnoredDuringExecution: []api.WeightedPodAffinityTerm{
{
Weight: 10,
PodAffinityTerm: api.PodAffinityTerm{
LabelSelector: &metav1.LabelSelector{
MatchExpressions: []metav1.LabelSelectorRequirement{
{
Key: "key2",
Operator: metav1.LabelSelectorOpNotIn,
Values: []string{"value1", "value2"},
},
},
},
Namespaces: []string{"ns"},
},
},
},
},
}),
},
},
"invalid toleration key": { "invalid toleration key": {
expectedError: "spec.tolerations[0].key", expectedError: "spec.tolerations[0].key",
spec: api.Pod{ spec: api.Pod{

View File

@ -75,7 +75,6 @@ type Topologies struct {
} }
// NodesHaveSameTopologyKey checks if nodeA and nodeB have same label value with given topologyKey as label key. // NodesHaveSameTopologyKey checks if nodeA and nodeB have same label value with given topologyKey as label key.
// If the topologyKey is empty, check if the two nodes have any of the default topologyKeys, and have same corresponding label value.
func (tps *Topologies) NodesHaveSameTopologyKey(nodeA, nodeB *v1.Node, topologyKey string) bool { func (tps *Topologies) NodesHaveSameTopologyKey(nodeA, nodeB *v1.Node, topologyKey string) bool {
return NodesHaveSameTopologyKey(nodeA, nodeB, topologyKey) return NodesHaveSameTopologyKey(nodeA, nodeB, topologyKey)
} }