Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not allow empty topology key for pod affinities. #49976

Merged
merged 1 commit into from
Aug 21, 2017
Merged

Do not allow empty topology key for pod affinities. #49976

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions pkg/api/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2026,9 +2026,7 @@ type PodAffinityTerm struct {
// 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
// selected pods is running.
// For PreferredDuringScheduling pod anti-affinity, empty topologyKey is interpreted as "all topologies"
// ("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.
// Empty topologyKey is not allowed.
// +optional
TopologyKey string
}
Expand Down
31 changes: 12 additions & 19 deletions pkg/api/validation/validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2429,7 +2429,7 @@ func ValidatePreferredSchedulingTerms(terms []api.PreferredSchedulingTerm, fldPa
}

// 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 = append(allErrs, unversionedvalidation.ValidateLabelSelector(podAffinityTerm.LabelSelector, fldPath.Child("matchExpressions"))...)
Expand All @@ -2438,32 +2438,29 @@ func validatePodAffinityTerm(podAffinityTerm api.PodAffinityTerm, allowEmptyTopo
allErrs = append(allErrs, field.Invalid(fldPath.Child("namespace"), name, msg))
}
}
if !allowEmptyTopologyKey && len(podAffinityTerm.TopologyKey) == 0 {
allErrs = append(allErrs, field.Required(fldPath.Child("topologyKey"), "can only be empty for PreferredDuringScheduling pod anti affinity"))
if len(podAffinityTerm.TopologyKey) == 0 {
allErrs = append(allErrs, field.Required(fldPath.Child("topologyKey"), "can not be empty"))
}
if len(podAffinityTerm.TopologyKey) != 0 {
allErrs = append(allErrs, unversionedvalidation.ValidateLabelName(podAffinityTerm.TopologyKey, fldPath.Child("topologyKey"))...)
}
return allErrs
return append(allErrs, unversionedvalidation.ValidateLabelName(podAffinityTerm.TopologyKey, fldPath.Child("topologyKey"))...)
}

// 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{}
for i, podAffinityTerm := range podAffinityTerms {
allErrs = append(allErrs, validatePodAffinityTerm(podAffinityTerm, allowEmptyTopologyKey, fldPath.Index(i))...)
allErrs = append(allErrs, validatePodAffinityTerm(podAffinityTerm, fldPath.Index(i))...)
}
return allErrs
}

// 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{}
for j, weightedTerm := range weightedPodAffinityTerms {
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, validatePodAffinityTerm(weightedTerm.PodAffinityTerm, allowEmptyTopologyKey, fldPath.Index(j).Child("podAffinityTerm"))...)
allErrs = append(allErrs, validatePodAffinityTerm(weightedTerm.PodAffinityTerm, fldPath.Index(j).Child("podAffinityTerm"))...)
}
return allErrs
}
Expand All @@ -2477,13 +2474,11 @@ func validatePodAntiAffinity(podAntiAffinity *api.PodAntiAffinity, fldPath *fiel
// fldPath.Child("requiredDuringSchedulingRequiredDuringExecution"))...)
//}
if podAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution != nil {
// empty topologyKey is not allowed for hard pod anti-affinity
allErrs = append(allErrs, validatePodAffinityTerms(podAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution, false,
allErrs = append(allErrs, validatePodAffinityTerms(podAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution,
fldPath.Child("requiredDuringSchedulingIgnoredDuringExecution"))...)
}
if podAntiAffinity.PreferredDuringSchedulingIgnoredDuringExecution != nil {
// empty topologyKey is allowed for soft pod anti-affinity
allErrs = append(allErrs, validateWeightedPodAffinityTerms(podAntiAffinity.PreferredDuringSchedulingIgnoredDuringExecution, true,
allErrs = append(allErrs, validateWeightedPodAffinityTerms(podAntiAffinity.PreferredDuringSchedulingIgnoredDuringExecution,
fldPath.Child("preferredDuringSchedulingIgnoredDuringExecution"))...)
}
return allErrs
Expand All @@ -2498,13 +2493,11 @@ func validatePodAffinity(podAffinity *api.PodAffinity, fldPath *field.Path) fiel
// fldPath.Child("requiredDuringSchedulingRequiredDuringExecution"))...)
//}
if podAffinity.RequiredDuringSchedulingIgnoredDuringExecution != nil {
// empty topologyKey is not allowed for hard pod affinity
allErrs = append(allErrs, validatePodAffinityTerms(podAffinity.RequiredDuringSchedulingIgnoredDuringExecution, false,
allErrs = append(allErrs, validatePodAffinityTerms(podAffinity.RequiredDuringSchedulingIgnoredDuringExecution,
fldPath.Child("requiredDuringSchedulingIgnoredDuringExecution"))...)
}
if podAffinity.PreferredDuringSchedulingIgnoredDuringExecution != nil {
// empty topologyKey is not allowed for soft pod affinity
allErrs = append(allErrs, validateWeightedPodAffinityTerms(podAffinity.PreferredDuringSchedulingIgnoredDuringExecution, false,
allErrs = append(allErrs, validateWeightedPodAffinityTerms(podAffinity.PreferredDuringSchedulingIgnoredDuringExecution,
fldPath.Child("preferredDuringSchedulingIgnoredDuringExecution"))...)
}
return allErrs
Expand Down
42 changes: 36 additions & 6 deletions pkg/api/validation/validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4655,8 +4655,8 @@ func TestValidatePod(t *testing.T) {
}),
},
},
"invalid pod affinity, empty topologyKey is not allowed for hard pod affinity": {
expectedError: "can only be empty for PreferredDuringScheduling pod anti affinity",
"invalid hard pod affinity, empty topologyKey is not allowed for hard pod affinity": {
expectedError: "can not be empty",
spec: api.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "123",
Expand All @@ -4682,8 +4682,8 @@ func TestValidatePod(t *testing.T) {
}),
},
},
"invalid pod anti-affinity, empty topologyKey is not allowed for hard pod anti-affinity": {
expectedError: "can only be empty for PreferredDuringScheduling pod anti affinity",
"invalid hard pod anti-affinity, empty topologyKey is not allowed for hard pod anti-affinity": {
expectedError: "can not be empty",
spec: api.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "123",
Expand All @@ -4709,8 +4709,8 @@ func TestValidatePod(t *testing.T) {
}),
},
},
"invalid pod anti-affinity, empty topologyKey is not allowed for soft pod affinity": {
expectedError: "can only be empty for PreferredDuringScheduling pod anti affinity",
"invalid soft pod affinity, empty topologyKey is not allowed for soft pod affinity": {
expectedError: "can not be empty",
spec: api.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "123",
Expand Down Expand Up @@ -4739,6 +4739,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": {
expectedError: "spec.tolerations[0].key",
spec: api.Pod{
Expand Down
1 change: 0 additions & 1 deletion plugin/pkg/scheduler/algorithm/priorities/util/topologies.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ type Topologies struct {
}

// 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 {
return NodesHaveSameTopologyKey(nodeA, nodeB, topologyKey)
}