Skip to content

Commit

Permalink
Merge pull request #1 from kubernetes/master
Browse files Browse the repository at this point in the history 
docs: clarify eviction-max-pod-grace-period behavior with negative values
  • Loading branch information
@FabianIMV
FabianIMV authored Nov 25, 2024
2 parents 35d098a + e4c1f98 commit b5649df
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 12 deletions.
28 changes: 18 additions & 10 deletions pkg/apis/core/validation/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4477,7 +4477,7 @@ func validateWindows(spec *core.PodSpec, fldPath *field.Path) field.ErrorList {
}

// ValidateNodeSelectorRequirement tests that the specified NodeSelectorRequirement fields has valid data
func ValidateNodeSelectorRequirement(rq core.NodeSelectorRequirement, fldPath *field.Path) field.ErrorList {
func ValidateNodeSelectorRequirement(rq core.NodeSelectorRequirement, allowInvalidLabelValueInRequiredNodeAffinity bool, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
switch rq.Operator {
case core.NodeSelectorOpIn, core.NodeSelectorOpNotIn:
Expand All @@ -4496,9 +4496,15 @@ func ValidateNodeSelectorRequirement(rq core.NodeSelectorRequirement, fldPath *f
default:
allErrs = append(allErrs, field.Invalid(fldPath.Child("operator"), rq.Operator, "not a valid selector operator"))
}

allErrs = append(allErrs, unversionedvalidation.ValidateLabelName(rq.Key, fldPath.Child("key"))...)

if !allowInvalidLabelValueInRequiredNodeAffinity {
path := fldPath.Child("values")
for valueIndex, value := range rq.Values {
for _, msg := range validation.IsValidLabelValue(value) {
allErrs = append(allErrs, field.Invalid(path.Index(valueIndex), value, msg))
}
}
}
return allErrs
}

Expand Down Expand Up @@ -4534,11 +4540,11 @@ func ValidateNodeFieldSelectorRequirement(req core.NodeSelectorRequirement, fldP
}

// ValidateNodeSelectorTerm tests that the specified node selector term has valid data
func ValidateNodeSelectorTerm(term core.NodeSelectorTerm, fldPath *field.Path) field.ErrorList {
func ValidateNodeSelectorTerm(term core.NodeSelectorTerm, allowInvalidLabelValueInRequiredNodeAffinity bool, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

for j, req := range term.MatchExpressions {
allErrs = append(allErrs, ValidateNodeSelectorRequirement(req, fldPath.Child("matchExpressions").Index(j))...)
allErrs = append(allErrs, ValidateNodeSelectorRequirement(req, allowInvalidLabelValueInRequiredNodeAffinity, fldPath.Child("matchExpressions").Index(j))...)
}

for j, req := range term.MatchFields {
Expand All @@ -4549,7 +4555,7 @@ func ValidateNodeSelectorTerm(term core.NodeSelectorTerm, fldPath *field.Path) f
}

// ValidateNodeSelector tests that the specified nodeSelector fields has valid data
func ValidateNodeSelector(nodeSelector *core.NodeSelector, fldPath *field.Path) field.ErrorList {
func ValidateNodeSelector(nodeSelector *core.NodeSelector, allowInvalidLabelValueInRequiredNodeAffinity bool, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

termFldPath := fldPath.Child("nodeSelectorTerms")
Expand All @@ -4558,7 +4564,7 @@ func ValidateNodeSelector(nodeSelector *core.NodeSelector, fldPath *field.Path)
}

for i, term := range nodeSelector.NodeSelectorTerms {
allErrs = append(allErrs, ValidateNodeSelectorTerm(term, termFldPath.Index(i))...)
allErrs = append(allErrs, ValidateNodeSelectorTerm(term, allowInvalidLabelValueInRequiredNodeAffinity, termFldPath.Index(i))...)
}

return allErrs
Expand Down Expand Up @@ -4659,7 +4665,9 @@ func ValidatePreferredSchedulingTerms(terms []core.PreferredSchedulingTerm, fldP
allErrs = append(allErrs, field.Invalid(fldPath.Index(i).Child("weight"), term.Weight, "must be in the range 1-100"))
}

allErrs = append(allErrs, ValidateNodeSelectorTerm(term.Preference, fldPath.Index(i).Child("preference"))...)
// we always allow invalid label-value for preferred affinity
// as they can success when cluster has only one node
allErrs = append(allErrs, ValidateNodeSelectorTerm(term.Preference, true, fldPath.Index(i).Child("preference"))...)
}
return allErrs
}
Expand Down Expand Up @@ -4729,7 +4737,7 @@ func validateNodeAffinity(na *core.NodeAffinity, fldPath *field.Path) field.Erro
// allErrs = append(allErrs, ValidateNodeSelector(na.RequiredDuringSchedulingRequiredDuringExecution, fldPath.Child("requiredDuringSchedulingRequiredDuringExecution"))...)
// }
if na.RequiredDuringSchedulingIgnoredDuringExecution != nil {
allErrs = append(allErrs, ValidateNodeSelector(na.RequiredDuringSchedulingIgnoredDuringExecution, fldPath.Child("requiredDuringSchedulingIgnoredDuringExecution"))...)
allErrs = append(allErrs, ValidateNodeSelector(na.RequiredDuringSchedulingIgnoredDuringExecution, true /* TODO: opts.AllowInvalidLabelValueInRequiredNodeAffinity */, fldPath.Child("requiredDuringSchedulingIgnoredDuringExecution"))...)
}
if len(na.PreferredDuringSchedulingIgnoredDuringExecution) > 0 {
allErrs = append(allErrs, ValidatePreferredSchedulingTerms(na.PreferredDuringSchedulingIgnoredDuringExecution, fldPath.Child("preferredDuringSchedulingIgnoredDuringExecution"))...)
Expand Down Expand Up @@ -7783,7 +7791,7 @@ func validateVolumeNodeAffinity(nodeAffinity *core.VolumeNodeAffinity, fldPath *
}

if nodeAffinity.Required != nil {
allErrs = append(allErrs, ValidateNodeSelector(nodeAffinity.Required, fldPath.Child("required"))...)
allErrs = append(allErrs, ValidateNodeSelector(nodeAffinity.Required, true /* TODO: opts.AllowInvalidLabelValueInRequiredNodeAffinity */, fldPath.Child("required"))...)
} else {
allErrs = append(allErrs, field.Required(fldPath.Child("required"), "must specify required node constraints"))
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/resource/validation/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ func validateAllocationResult(allocation *resource.AllocationResult, fldPath *fi
var allErrs field.ErrorList
allErrs = append(allErrs, validateDeviceAllocationResult(allocation.Devices, fldPath.Child("devices"), requestNames, stored)...)
if allocation.NodeSelector != nil {
allErrs = append(allErrs, corevalidation.ValidateNodeSelector(allocation.NodeSelector, fldPath.Child("nodeSelector"))...)
allErrs = append(allErrs, corevalidation.ValidateNodeSelector(allocation.NodeSelector, false, fldPath.Child("nodeSelector"))...)
}
return allErrs
}
Expand Down Expand Up @@ -490,7 +490,7 @@ func validateResourceSliceSpec(spec, oldSpec *resource.ResourceSliceSpec, fldPat
}
if spec.NodeSelector != nil {
numNodeSelectionFields++
allErrs = append(allErrs, corevalidation.ValidateNodeSelector(spec.NodeSelector, fldPath.Child("nodeSelector"))...)
allErrs = append(allErrs, corevalidation.ValidateNodeSelector(spec.NodeSelector, false, fldPath.Child("nodeSelector"))...)
if len(spec.NodeSelector.NodeSelectorTerms) != 1 {
// This additional constraint simplifies merging of different selectors
// when devices are allocated from different slices.
Expand Down
47 changes: 47 additions & 0 deletions pkg/apis/resource/validation/validation_resourceclaim_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1282,6 +1282,53 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
},
deviceStatusFeatureGate: false,
},
"invalid-update-invalid-label-value": {
wantFailures: field.ErrorList{
field.Invalid(field.NewPath("status", "allocation", "nodeSelector", "nodeSelectorTerms").Index(0).Child("matchExpressions").Index(0).Child("values").Index(0), "-1", "a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue', or 'my_value', or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')"),
},
oldClaim: validClaim,
update: func(claim *resource.ResourceClaim) *resource.ResourceClaim {
claim = claim.DeepCopy()
claim.Status.Allocation = validAllocatedClaim.Status.Allocation.DeepCopy()
claim.Status.Allocation.NodeSelector = &core.NodeSelector{
NodeSelectorTerms: []core.NodeSelectorTerm{{
MatchExpressions: []core.NodeSelectorRequirement{{
Key: "foo",
Operator: core.NodeSelectorOpIn,
Values: []string{"-1"},
}},
}},
}
return claim
},
},
"valid-update-with-invalid-label-value": {
oldClaim: func() *resource.ResourceClaim {
claim := validAllocatedClaim.DeepCopy()
claim.Status.Allocation = validAllocatedClaim.Status.Allocation.DeepCopy()
claim.Status.Allocation.NodeSelector = &core.NodeSelector{
NodeSelectorTerms: []core.NodeSelectorTerm{{
MatchExpressions: []core.NodeSelectorRequirement{{
Key: "foo",
Operator: core.NodeSelectorOpIn,
Values: []string{"-1"},
}},
}},
}
return claim
}(),
update: func(claim *resource.ResourceClaim) *resource.ResourceClaim {
for i := 0; i < resource.ResourceClaimReservedForMaxSize; i++ {
claim.Status.ReservedFor = append(claim.Status.ReservedFor,
resource.ResourceClaimConsumerReference{
Resource: "pods",
Name: fmt.Sprintf("foo-%d", i),
UID: types.UID(fmt.Sprintf("%d", i)),
})
}
return claim
},
},
}

for name, scenario := range scenarios {
Expand Down
46 changes: 46 additions & 0 deletions pkg/apis/resource/validation/validation_resourceslice_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,23 @@ func TestValidateResourceSlice(t *testing.T) {
return slice
}(),
},
"invalid-node-selecor-label-value": {
wantFailures: field.ErrorList{field.Invalid(field.NewPath("spec", "nodeSelector", "nodeSelectorTerms").Index(0).Child("matchExpressions").Index(0).Child("values").Index(0), "-1", "a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue', or 'my_value', or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')")},
slice: func() *resourceapi.ResourceSlice {
slice := testResourceSlice(goodName, goodName, goodName, 3)
slice.Spec.NodeName = ""
slice.Spec.NodeSelector = &core.NodeSelector{
NodeSelectorTerms: []core.NodeSelectorTerm{{
MatchExpressions: []core.NodeSelectorRequirement{{
Key: "foo",
Operator: core.NodeSelectorOpIn,
Values: []string{"-1"},
}},
}},
}
return slice
}(),
},
}

for name, scenario := range scenarios {
Expand Down Expand Up @@ -485,6 +502,35 @@ func TestValidateResourceSliceUpdate(t *testing.T) {
return slice
},
},
"invalid-update-to-invalid-nodeselector-label-value": {
wantFailures: field.ErrorList{field.Invalid(field.NewPath("spec", "nodeSelector", "nodeSelectorTerms").Index(0).Child("matchExpressions").Index(0).Child("values").Index(0), "-1", "a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue', or 'my_value', or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')")},
oldResourceSlice: func() *resourceapi.ResourceSlice {
slice := validResourceSlice.DeepCopy()
slice.Spec.NodeName = ""
slice.Spec.NodeSelector = &core.NodeSelector{
NodeSelectorTerms: []core.NodeSelectorTerm{{
MatchExpressions: []core.NodeSelectorRequirement{{
Key: "foo",
Operator: core.NodeSelectorOpIn,
Values: []string{"bar"},
}},
}},
}
return slice
}(),
update: func(slice *resourceapi.ResourceSlice) *resourceapi.ResourceSlice {
slice.Spec.NodeSelector = &core.NodeSelector{
NodeSelectorTerms: []core.NodeSelectorTerm{{
MatchExpressions: []core.NodeSelectorRequirement{{
Key: "foo",
Operator: core.NodeSelectorOpIn,
Values: []string{"-1"},
}},
}},
}
return slice
},
},
}

for name, scenario := range scenarios {
Expand Down

0 comments on commit b5649df

Please sign in to comment.