From bdf11c447be2f529d6879d0a8ff4ace57997a5ac Mon Sep 17 00:00:00 2001 From: Poor12 Date: Sat, 20 Aug 2022 15:22:56 +0800 Subject: [PATCH] add cluster resource model check Signed-off-by: Poor12 --- pkg/apis/cluster/mutation/mutation.go | 25 ++- pkg/apis/cluster/mutation/mutation_test.go | 125 +++++++++++ pkg/apis/cluster/validation/validation.go | 50 ++++- .../cluster/validation/validation_test.go | 207 ++++++++++++++++++ pkg/registry/cluster/strategy.go | 12 +- 5 files changed, 414 insertions(+), 5 deletions(-) create mode 100644 pkg/apis/cluster/mutation/mutation_test.go diff --git a/pkg/apis/cluster/mutation/mutation.go b/pkg/apis/cluster/mutation/mutation.go index 4a4ab2215..3d18fc0f3 100644 --- a/pkg/apis/cluster/mutation/mutation.go +++ b/pkg/apis/cluster/mutation/mutation.go @@ -2,6 +2,7 @@ package mutation import ( "math" + "sort" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -30,11 +31,29 @@ func MutateClusterTaints(taints []corev1.Taint) { } } -// SetDefaultClusterResourceModels set default cluster resource models for cluster. -func SetDefaultClusterResourceModels(cluster *clusterapis.Cluster) { - if cluster.Spec.ResourceModels != nil { +// StandardizeClusterResourceModels set cluster resource models for given order and boundary value. +func StandardizeClusterResourceModels(models []clusterapis.ResourceModel) { + if len(models) == 0 { return } + + sort.Slice(models, func(i, j int) bool { + return models[i].Grade < models[j].Grade + }) + + // The Min value of the first grade(usually 0) always acts as zero. + for index := range models[0].Ranges { + models[0].Ranges[index].Min.Set(0) + } + + // The Max value of the last grade always acts as MaxInt64. + for index := range models[len(models)-1].Ranges { + models[len(models)-1].Ranges[index].Max.Set(math.MaxInt64) + } +} + +// SetDefaultClusterResourceModels set default cluster resource models for cluster. +func SetDefaultClusterResourceModels(cluster *clusterapis.Cluster) { cluster.Spec.ResourceModels = []clusterapis.ResourceModel{ { Grade: 0, diff --git a/pkg/apis/cluster/mutation/mutation_test.go b/pkg/apis/cluster/mutation/mutation_test.go new file mode 100644 index 000000000..22f3ddb73 --- /dev/null +++ b/pkg/apis/cluster/mutation/mutation_test.go @@ -0,0 +1,125 @@ +package mutation + +import ( + "math" + "reflect" + "testing" + + "k8s.io/apimachinery/pkg/api/resource" + + clusterapis "github.com/karmada-io/karmada/pkg/apis/cluster" +) + +func TestStandardizeClusterResourceModels(t *testing.T) { + testCases := map[string]struct { + models []clusterapis.ResourceModel + expectedModels []clusterapis.ResourceModel + }{ + "sort models": { + models: []clusterapis.ResourceModel{ + { + Grade: 2, + Ranges: []clusterapis.ResourceModelRange{ + { + Name: clusterapis.ResourceCPU, + Min: *resource.NewQuantity(2, resource.DecimalSI), + Max: *resource.NewQuantity(math.MaxInt64, resource.DecimalSI), + }, + }, + }, + { + Grade: 1, + Ranges: []clusterapis.ResourceModelRange{ + { + Name: clusterapis.ResourceCPU, + Min: *resource.NewQuantity(0, resource.DecimalSI), + Max: *resource.NewQuantity(2, resource.DecimalSI), + }, + }, + }, + }, + expectedModels: []clusterapis.ResourceModel{ + { + Grade: 1, + Ranges: []clusterapis.ResourceModelRange{ + { + Name: clusterapis.ResourceCPU, + Min: *resource.NewQuantity(0, resource.DecimalSI), + Max: *resource.NewQuantity(2, resource.DecimalSI), + }, + }, + }, + { + Grade: 2, + Ranges: []clusterapis.ResourceModelRange{ + { + Name: clusterapis.ResourceCPU, + Min: *resource.NewQuantity(2, resource.DecimalSI), + Max: *resource.NewQuantity(math.MaxInt64, resource.DecimalSI), + }, + }, + }, + }, + }, + "start with 0": { + models: []clusterapis.ResourceModel{ + { + Grade: 1, + Ranges: []clusterapis.ResourceModelRange{ + { + Name: clusterapis.ResourceCPU, + Min: *resource.NewQuantity(1, resource.DecimalSI), + Max: *resource.NewQuantity(math.MaxInt64, resource.DecimalSI), + }, + }, + }, + }, + expectedModels: []clusterapis.ResourceModel{ + { + Grade: 1, + Ranges: []clusterapis.ResourceModelRange{ + { + Name: clusterapis.ResourceCPU, + Min: *resource.NewQuantity(0, resource.DecimalSI), + Max: *resource.NewQuantity(math.MaxInt64, resource.DecimalSI), + }, + }, + }, + }, + }, + "end with MaxInt64": { + models: []clusterapis.ResourceModel{ + { + Grade: 1, + Ranges: []clusterapis.ResourceModelRange{ + { + Name: clusterapis.ResourceCPU, + Min: *resource.NewQuantity(0, resource.DecimalSI), + Max: *resource.NewQuantity(2, resource.DecimalSI), + }, + }, + }, + }, + expectedModels: []clusterapis.ResourceModel{ + { + Grade: 1, + Ranges: []clusterapis.ResourceModelRange{ + { + Name: clusterapis.ResourceCPU, + Min: *resource.NewQuantity(0, resource.DecimalSI), + Max: *resource.NewQuantity(math.MaxInt64, resource.DecimalSI), + }, + }, + }, + }, + }, + } + + for name, testCase := range testCases { + StandardizeClusterResourceModels(testCase.models) + if !reflect.DeepEqual(testCase.models, testCase.expectedModels) { + t.Errorf("expected sorted resource models for %q, but it did not work", name) + return + } + } +} diff --git a/pkg/apis/cluster/validation/validation.go b/pkg/apis/cluster/validation/validation.go index ecc400596..50c063ecc 100644 --- a/pkg/apis/cluster/validation/validation.go +++ b/pkg/apis/cluster/validation/validation.go @@ -2,6 +2,7 @@ package validation import ( "fmt" + "math" "net/url" apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation" @@ -34,7 +35,10 @@ func ValidateClusterName(name string) []string { return kubevalidation.IsDNS1123Label(name) } -var supportedSyncModes = sets.NewString(string(api.Pull), string(api.Push)) +var ( + supportedSyncModes = sets.NewString(string(api.Pull), string(api.Push)) + supportedResourceNames = sets.NewString(string(api.ResourceCPU), string(api.ResourceMemory), string(api.ResourceStorage), string(api.ResourceEphemeralStorage)) +) // ValidateCluster tests if required fields in the Cluster are set. func ValidateCluster(cluster *api.Cluster) field.ErrorList { @@ -90,6 +94,12 @@ func ValidateClusterSpec(spec *api.ClusterSpec, fldPath *field.Path) field.Error allErrs = append(allErrs, lifted.ValidateClusterTaints(spec.Taints, fldPath.Child("taints"))...) } + if len(spec.ResourceModels) > 0 { + if err := ValidateClusterResourceModels(fldPath.Child("resourceModels"), spec.ResourceModels); err != nil { + allErrs = append(allErrs, err) + } + } + return allErrs } @@ -133,3 +143,41 @@ func ValidateClusterProxyURL(fldPath *field.Path, proxyURL string) field.ErrorLi } return allErrs } + +// ValidateClusterResourceModels validates cluster's resource models. +func ValidateClusterResourceModels(fldPath *field.Path, models []api.ResourceModel) *field.Error { + for i, resourceModel := range models { + if i != 0 && resourceModel.Grade == models[i-1].Grade { + return field.Invalid(fldPath, models, "The grade of each models should not be the same") + } + + if i != 0 && len(models[i-1].Ranges) != len(resourceModel.Ranges) { + return field.Invalid(fldPath, models, "The number of resource types should be the same") + } + + for j, resourceModelRange := range resourceModel.Ranges { + if !supportedResourceNames.Has(string(resourceModelRange.Name)) { + return field.NotSupported(fldPath.Child("ranges").Child("name"), resourceModelRange.Name, supportedResourceNames.List()) + } + if resourceModelRange.Max.Cmp(resourceModelRange.Min) <= 0 { + return field.Invalid(fldPath, models, "The max value of each resource must be greater than the min value") + } + if i == 0 { + if !resourceModelRange.Min.IsZero() { + return field.Invalid(fldPath, models, "The min value of each resource in the first model should be 0") + } + } else if models[i-1].Ranges[j].Name != resourceModelRange.Name { + return field.Invalid(fldPath, models, "The resource types of each models should be the same") + } else if models[i-1].Ranges[j].Max != resourceModelRange.Min { + return field.Invalid(fldPath, models, "Model intervals for resources must be contiguous and non-overlapping") + } + + if i == len(models)-1 { + if resourceModelRange.Max.Value() != math.MaxInt64 { + return field.Invalid(fldPath, models, "The max value of each resource in the last model should be MaxInt64") + } + } + } + } + return nil +} diff --git a/pkg/apis/cluster/validation/validation_test.go b/pkg/apis/cluster/validation/validation_test.go index ef85c40ca..3299b4714 100644 --- a/pkg/apis/cluster/validation/validation_test.go +++ b/pkg/apis/cluster/validation/validation_test.go @@ -1,10 +1,12 @@ package validation import ( + "math" "strings" "testing" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" api "github.com/karmada-io/karmada/pkg/apis/cluster" @@ -67,6 +69,211 @@ func TestValidateCluster(t *testing.T) { }, expectError: true, }, + "invalid cluster resource models with the same grade": { + cluster: api.Cluster{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: api.ClusterSpec{ + ResourceModels: []api.ResourceModel{ + { + Grade: 1, + Ranges: []api.ResourceModelRange{ + { + Name: api.ResourceCPU, + Min: *resource.NewQuantity(0, resource.DecimalSI), + Max: *resource.NewQuantity(2, resource.DecimalSI), + }, + }, + }, + { + Grade: 2, + Ranges: []api.ResourceModelRange{ + { + Name: api.ResourceCPU, + Min: *resource.NewQuantity(2, resource.DecimalSI), + Max: *resource.NewQuantity(math.MaxInt64, resource.DecimalSI), + }, + }, + }, + }, + }, + }, + expectError: true, + }, + "invalid cluster resource models with different numbers of resource types": { + cluster: api.Cluster{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: api.ClusterSpec{ + ResourceModels: []api.ResourceModel{ + { + Grade: 1, + Ranges: []api.ResourceModelRange{ + { + Name: api.ResourceCPU, + Min: *resource.NewQuantity(0, resource.DecimalSI), + Max: *resource.NewQuantity(2, resource.DecimalSI), + }, + }, + }, + { + Grade: 2, + Ranges: []api.ResourceModelRange{ + { + Name: api.ResourceCPU, + Min: *resource.NewQuantity(2, resource.DecimalSI), + Max: *resource.NewQuantity(math.MaxInt64, resource.DecimalSI), + }, + { + Name: api.ResourceMemory, + Min: *resource.NewQuantity(2, resource.DecimalSI), + Max: *resource.NewQuantity(math.MaxInt64, resource.DecimalSI), + }, + }, + }, + }, + }, + }, + expectError: true, + }, + "invalid cluster resource models with unreasonable ranges": { + cluster: api.Cluster{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: api.ClusterSpec{ + ResourceModels: []api.ResourceModel{ + { + Grade: 1, + Ranges: []api.ResourceModelRange{ + { + Name: api.ResourceCPU, + Min: *resource.NewQuantity(2, resource.DecimalSI), + Max: *resource.NewQuantity(0, resource.DecimalSI), + }, + }, + }, + }, + }, + }, + expectError: true, + }, + "invalid cluster resource models which the min value of each resource in the first model is not 0": { + cluster: api.Cluster{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: api.ClusterSpec{ + ResourceModels: []api.ResourceModel{ + { + Grade: 1, + Ranges: []api.ResourceModelRange{ + { + Name: api.ResourceCPU, + Min: *resource.NewQuantity(1, resource.DecimalSI), + Max: *resource.NewQuantity(math.MaxInt64, resource.DecimalSI), + }, + }, + }, + }, + }, + }, + expectError: true, + }, + "invalid cluster resource models which the max value of each resource in the last model is not MaxInt64": { + cluster: api.Cluster{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: api.ClusterSpec{ + ResourceModels: []api.ResourceModel{ + { + Grade: 1, + Ranges: []api.ResourceModelRange{ + { + Name: api.ResourceCPU, + Min: *resource.NewQuantity(0, resource.DecimalSI), + Max: *resource.NewQuantity(2, resource.DecimalSI), + }, + }, + }, + }, + }, + }, + expectError: true, + }, + "invalid cluster resource models which the resource types of each models are different": { + cluster: api.Cluster{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: api.ClusterSpec{ + ResourceModels: []api.ResourceModel{ + { + Grade: 1, + Ranges: []api.ResourceModelRange{ + { + Name: api.ResourceCPU, + Min: *resource.NewQuantity(0, resource.DecimalSI), + Max: *resource.NewQuantity(2, resource.DecimalSI), + }, + }, + }, + { + Grade: 2, + Ranges: []api.ResourceModelRange{ + { + Name: api.ResourceMemory, + Min: *resource.NewQuantity(2, resource.DecimalSI), + Max: *resource.NewQuantity(math.MaxInt64, resource.DecimalSI), + }, + }, + }, + }, + }, + }, + expectError: true, + }, + "invalid cluster resource models with contiguous and non-overlapping": { + cluster: api.Cluster{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: api.ClusterSpec{ + ResourceModels: []api.ResourceModel{ + { + Grade: 1, + Ranges: []api.ResourceModelRange{ + { + Name: api.ResourceCPU, + Min: *resource.NewQuantity(0, resource.DecimalSI), + Max: *resource.NewQuantity(2, resource.DecimalSI), + }, + }, + }, + { + Grade: 2, + Ranges: []api.ResourceModelRange{ + { + Name: api.ResourceCPU, + Min: *resource.NewQuantity(1, resource.DecimalSI), + Max: *resource.NewQuantity(math.MaxInt64, resource.DecimalSI), + }, + }, + }, + }, + }, + }, + expectError: true, + }, + "invalid cluster resource models with invalid resource name": { + cluster: api.Cluster{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: api.ClusterSpec{ + ResourceModels: []api.ResourceModel{ + { + Grade: 1, + Ranges: []api.ResourceModelRange{ + { + Name: "test", + Min: *resource.NewQuantity(0, resource.DecimalSI), + Max: *resource.NewQuantity(math.MaxInt64, resource.DecimalSI), + }, + }, + }, + }, + }, + }, + expectError: true, + }, } for name, testCase := range testCases { diff --git a/pkg/registry/cluster/strategy.go b/pkg/registry/cluster/strategy.go index d42435f3e..afc1a4ba1 100644 --- a/pkg/registry/cluster/strategy.go +++ b/pkg/registry/cluster/strategy.go @@ -74,12 +74,22 @@ func (Strategy) GetResetFields() map[fieldpath.APIVersion]*fieldpath.Set { func (Strategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { cluster := obj.(*clusterapis.Cluster) if utilfeature.DefaultMutableFeatureGate.Enabled(features.CustomizedClusterResourceModeling) { - mutation.SetDefaultClusterResourceModels(cluster) + if len(cluster.Spec.ResourceModels) == 0 { + mutation.SetDefaultClusterResourceModels(cluster) + } else { + mutation.StandardizeClusterResourceModels(cluster.Spec.ResourceModels) + } } } // PrepareForUpdate is invoked on update before validation to normalize the object. func (Strategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) { + cluster := obj.(*clusterapis.Cluster) + if utilfeature.DefaultMutableFeatureGate.Enabled(features.CustomizedClusterResourceModeling) { + if len(cluster.Spec.ResourceModels) != 0 { + mutation.StandardizeClusterResourceModels(cluster.Spec.ResourceModels) + } + } } // Validate returns an ErrorList with validation errors or nil.