Skip to content

Commit

Permalink
Merge pull request #3829 from whitewindmills/preemption-validation
Browse files Browse the repository at this point in the history
feat: validate resourceSelectors if Preemption is enabled
  • Loading branch information
karmada-bot authored Jul 24, 2023
2 parents 6ef427a + 268498a commit 0d50f59
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 0 deletions.
16 changes: 16 additions & 0 deletions pkg/util/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,22 @@ func ValidatePropagationSpec(spec policyv1alpha1.PropagationSpec) field.ErrorLis
allErrs = append(allErrs, field.Invalid(field.NewPath("spec").Child("propagateDeps"), spec.PropagateDeps, "application failover is set, propagateDeps must be true"))
}
allErrs = append(allErrs, ValidateFailover(spec.Failover, field.NewPath("spec").Child("failover"))...)
allErrs = append(allErrs, validateResourceSelectorsIfPreemptionEnabled(spec, field.NewPath("spec").Child("resourceSelectors"))...)
return allErrs
}

// validateResourceSelectorsIfPreemptionEnabled validates ResourceSelectors if Preemption is Always.
func validateResourceSelectorsIfPreemptionEnabled(spec policyv1alpha1.PropagationSpec, fldPath *field.Path) field.ErrorList {
if spec.Preemption != policyv1alpha1.PreemptAlways {
return nil
}

var allErrs field.ErrorList
for index, resourceSelector := range spec.ResourceSelectors {
if len(resourceSelector.Name) == 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Index(index).Child("name"), resourceSelector.Name, "name can not be empty if preemption is Always, the empty name may cause unexpected resources preemption"))
}
}
return allErrs
}

Expand Down
14 changes: 14 additions & 0 deletions pkg/util/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,20 @@ func TestValidatePropagationSpec(t *testing.T) {
}},
expectedErr: "the cluster spread constraint must be enabled in one of the constraints in case of SpreadByField is enabled",
},
{
name: "resourceSelector name is empty when preemption is enabled",
spec: policyv1alpha1.PropagationSpec{
ResourceSelectors: []policyv1alpha1.ResourceSelector{
{
APIVersion: "v1",
Kind: "Pod",
Namespace: "default",
},
},
Preemption: policyv1alpha1.PreemptAlways,
},
expectedErr: "name can not be empty if preemption is Always, the empty name may cause unexpected resources preemption",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down

0 comments on commit 0d50f59

Please sign in to comment.