From c62d869cc2e8086d5b2803e586d43b3b46dbe1e6 Mon Sep 17 00:00:00 2001 From: Fansong Zeng Date: Tue, 9 Jan 2024 19:48:35 +0800 Subject: [PATCH] webhook: add node webhook for resource amplification (#1785) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: 逐灵 --- apis/extension/cpu_normalization.go | 7 + apis/extension/cpu_normalization_test.go | 32 ++ apis/extension/node_resource_amplification.go | 29 +- .../node_resource_amplification_test.go | 120 ------- config/webhook/manifests.yaml | 21 ++ .../validation/validation_loadaware_test.go | 203 +++++++++++ .../validation/validation_pluginargs_test.go | 171 +++++++++ pkg/features/features.go | 6 +- .../plugins/batchresource/util_test.go | 4 +- .../plugins/cpunormalization/plugin.go | 8 +- pkg/util/resource.go | 30 +- pkg/util/resource_test.go | 4 +- pkg/webhook/add_node.go | 5 + pkg/webhook/node/mutating/mutating_handler.go | 151 ++++++++ .../node/mutating/mutating_handler_test.go | 114 ++++++ pkg/webhook/node/mutating/webhooks.go | 30 ++ .../resource_amplification.go | 164 +++++++++ .../resource_amplification_test.go | 339 ++++++++++++++++++ 18 files changed, 1270 insertions(+), 168 deletions(-) create mode 100644 pkg/descheduler/apis/config/validation/validation_loadaware_test.go create mode 100644 pkg/webhook/node/mutating/mutating_handler.go create mode 100644 pkg/webhook/node/mutating/mutating_handler_test.go create mode 100644 pkg/webhook/node/mutating/webhooks.go create mode 100644 pkg/webhook/node/plugins/resourceamplification/resource_amplification.go create mode 100644 pkg/webhook/node/plugins/resourceamplification/resource_amplification_test.go diff --git a/apis/extension/cpu_normalization.go b/apis/extension/cpu_normalization.go index 983dbc7ee..c7b00e967 100644 --- a/apis/extension/cpu_normalization.go +++ b/apis/extension/cpu_normalization.go @@ -35,6 +35,9 @@ const ( // AnnotationCPUBasicInfo denotes the basic CPU info of the node. AnnotationCPUBasicInfo = NodeDomainPrefix + "/cpu-basic-info" + + // NormalizationRatioDiffEpsilon is the min difference between two cpu normalization ratios. + NormalizationRatioDiffEpsilon = 0.01 ) // GetCPUNormalizationRatio gets the cpu normalization ratio from the node. @@ -90,6 +93,10 @@ func GetCPUNormalizationEnabled(node *corev1.Node) (*bool, error) { return pointer.Bool(v), nil } +func IsCPUNormalizationRatioDifferent(old, new float64) bool { + return old > new+NormalizationRatioDiffEpsilon || old < new-NormalizationRatioDiffEpsilon +} + // CPUBasicInfo describes the cpu basic features and status. type CPUBasicInfo struct { CPUModel string `json:"cpuModel,omitempty"` diff --git a/apis/extension/cpu_normalization_test.go b/apis/extension/cpu_normalization_test.go index 8287a20fd..55a9b2601 100644 --- a/apis/extension/cpu_normalization_test.go +++ b/apis/extension/cpu_normalization_test.go @@ -388,3 +388,35 @@ func TestSetCPUBasicInfo(t *testing.T) { }) } } + +func TestCPUNormalizationRatioDifferent(t *testing.T) { + testCases := []struct { + old float64 + new float64 + expectedDiff bool + }{ + { + old: 1.2, + new: 1.2, + expectedDiff: false, + }, + { + old: 1.2, + new: 1.3, + expectedDiff: true, + }, + { + old: 1.2, + new: 1.205, + expectedDiff: false, + }, + { + old: 1.2, + new: 1.195, + expectedDiff: false, + }, + } + for _, tc := range testCases { + assert.Equal(t, tc.expectedDiff, IsCPUNormalizationRatioDifferent(tc.old, tc.new)) + } +} diff --git a/apis/extension/node_resource_amplification.go b/apis/extension/node_resource_amplification.go index 5e4f05566..d7ed1690f 100644 --- a/apis/extension/node_resource_amplification.go +++ b/apis/extension/node_resource_amplification.go @@ -30,9 +30,6 @@ const ( // AnnotationNodeResourceAmplificationRatio denotes the resource amplification ratio of the node. AnnotationNodeResourceAmplificationRatio = NodeDomainPrefix + "/resource-amplification-ratio" - // AnnotationNodeRawCapacity denotes the un-amplified raw capacity of the node. - AnnotationNodeRawCapacity = NodeDomainPrefix + "/raw-capacity" - // AnnotationNodeRawAllocatable denotes the un-amplified raw allocatable of the node. AnnotationNodeRawAllocatable = NodeDomainPrefix + "/raw-allocatable" ) @@ -106,28 +103,10 @@ func SetNodeResourceAmplificationRatio(node *corev1.Node, resource corev1.Resour return true, nil } -// GetNodeRawCapacity gets the raw capacity of node from annotations. -func GetNodeRawCapacity(annotations map[string]string) (corev1.ResourceList, error) { - s, ok := annotations[AnnotationNodeRawCapacity] - if !ok { - return nil, nil - } - - var capacity corev1.ResourceList - if err := json.Unmarshal([]byte(s), &capacity); err != nil { - return nil, fmt.Errorf("failed to unmarshal node raw capacity: %w", err) - } - - return capacity, nil -} - -// SetNodeRawCapacity sets the node annotation according to the raw capacity. -func SetNodeRawCapacity(node *corev1.Node, capacity corev1.ResourceList) { - s, _ := json.Marshal(capacity) - if node.Annotations == nil { - node.Annotations = map[string]string{} - } - node.Annotations[AnnotationNodeRawCapacity] = string(s) +// HasNodeRawAllocatable checks if the node has raw allocatable annotation. +func HasNodeRawAllocatable(annotations map[string]string) bool { + _, ok := annotations[AnnotationNodeRawAllocatable] + return ok } // GetNodeRawAllocatable gets the raw allocatable of node from annotations. diff --git a/apis/extension/node_resource_amplification_test.go b/apis/extension/node_resource_amplification_test.go index 7d3d0f71b..a50bda2f2 100644 --- a/apis/extension/node_resource_amplification_test.go +++ b/apis/extension/node_resource_amplification_test.go @@ -369,126 +369,6 @@ func TestSetNodeResourceAmplificationRatio(t *testing.T) { } } -func TestGetNodeRawCapacity(t *testing.T) { - tests := []struct { - name string - arg *corev1.Node - want corev1.ResourceList - wantErr bool - }{ - { - name: "node has no annotation", - arg: &corev1.Node{}, - want: nil, - wantErr: false, - }, - { - name: "node has no raw capacity annotation", - arg: &corev1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - "xxx": "yyy", - }, - }, - }, - want: nil, - wantErr: false, - }, - { - name: "node has valid raw capacity annotation", - arg: &corev1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - "xxx": "yyy", - AnnotationNodeRawCapacity: `{"cpu":"1"}`, - }, - }, - }, - want: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("1"), - }, - wantErr: false, - }, - { - name: "node has invalid raw capacity annotation", - arg: &corev1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - "xxx": "yyy", - AnnotationNodeRawCapacity: "invalid", - }, - }, - }, - want: nil, - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, gotErr := GetNodeRawCapacity(tt.arg.Annotations) - assert.Equal(t, tt.want, got) - assert.Equal(t, tt.wantErr, gotErr != nil) - }) - } -} - -func TestSetNodeRawCapacity(t *testing.T) { - type args struct { - node *corev1.Node - capacity corev1.ResourceList - } - tests := []struct { - name string - args args - wantField *corev1.Node - }{ - { - name: "set for node with no annotation", - args: args{ - node: &corev1.Node{}, - capacity: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("1"), - }, - }, - wantField: &corev1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - AnnotationNodeRawCapacity: `{"cpu":"1"}`, - }, - }, - }, - }, - { - name: "set for node with old annotation", - args: args{ - node: &corev1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - AnnotationNodeRawCapacity: `{"cpu":"2"}`, - }, - }, - }, - capacity: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("1"), - }, - }, - wantField: &corev1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - AnnotationNodeRawCapacity: `{"cpu":"1"}`, - }, - }, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - SetNodeRawCapacity(tt.args.node, tt.args.capacity) - assert.Equal(t, tt.wantField, tt.args.node) - }) - } -} - func TestGetNodeRawAllocatable(t *testing.T) { tests := []struct { name string diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index f7cbf6da9..d67194488 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -25,6 +25,27 @@ webhooks: resources: - elasticquotas sideEffects: None +- admissionReviewVersions: + - v1 + - v1beta1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /mutate-node-status + failurePolicy: Ignore + name: mnode-status.koordinator.sh + rules: + - apiGroups: + - "" + apiVersions: + - v1 + operations: + - CREATE + - UPDATE + resources: + - nodes/status + sideEffects: None - admissionReviewVersions: - v1 - v1beta1 diff --git a/pkg/descheduler/apis/config/validation/validation_loadaware_test.go b/pkg/descheduler/apis/config/validation/validation_loadaware_test.go new file mode 100644 index 000000000..30bfbf591 --- /dev/null +++ b/pkg/descheduler/apis/config/validation/validation_loadaware_test.go @@ -0,0 +1,203 @@ +/* +Copyright 2023 The Koordinator Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation + +import ( + "testing" + + deschedulerconfig "github.com/koordinator-sh/koordinator/pkg/descheduler/apis/config" + "github.com/stretchr/testify/assert" +) + +func TestValidateLowLoadUtilizationArgs_NumerOfNodes(t *testing.T) { + testCases := []struct { + numOfNodes int + expectedError bool + }{ + { + numOfNodes: 10, + expectedError: false, + }, + { + numOfNodes: 5, + expectedError: false, + }, + { + numOfNodes: 0, + expectedError: false, + }, + { + numOfNodes: -1, + expectedError: true, + }, + { + numOfNodes: -5, + expectedError: true, + }, + } + + for _, tc := range testCases { + args := &deschedulerconfig.LowNodeLoadArgs{ + NumberOfNodes: int32(tc.numOfNodes), + } + err := ValidateLowLoadUtilizationArgs(nil, args) + if tc.expectedError { + assert.Error(t, err, "Expected an error for invalid NumberOfNodes") + assert.Contains(t, err.Error(), "must be greater than or equal to 0", "Expected specific error message") + } else { + assert.Nil(t, err, "Expected no error for valid configuration") + } + + } +} + +func TestValidateLowLoadUtilizationArgs_EvictableNamespaces(t *testing.T) { + testCases := []struct { + include []string + exclude []string + expectedError bool + }{ + { + include: []string{"namespace1"}, + expectedError: false, + }, + { + exclude: []string{"namespace1"}, + expectedError: false, + }, + { + include: []string{"namespace1", "namespace2"}, + expectedError: false, + }, + { + include: []string{"namespace1"}, + exclude: []string{"namespace2"}, + expectedError: true, + }, + { + include: []string{"namespace1", "namespace11"}, + exclude: []string{"namespace2"}, + expectedError: true, + }, + { + include: []string{"namespace1"}, + exclude: []string{"namespace2", "namespace22"}, + expectedError: true, + }, + } + + for _, tc := range testCases { + args := &deschedulerconfig.LowNodeLoadArgs{ + EvictableNamespaces: &deschedulerconfig.Namespaces{ + Include: tc.include, + Exclude: tc.exclude, + }, + } + err := ValidateLowLoadUtilizationArgs(nil, args) + if tc.expectedError { + assert.Error(t, err, "Expected an error for invalid EvictableNamespaces", tc.include, tc.exclude) + assert.Contains(t, err.Error(), "only one of Include/Exclude namespaces can be set", "Expected specific error message") + } else { + assert.Nil(t, err, "Expected no error for valid configuration") + } + } +} + +func TestValidateLowLoadUtilizationArgs_NodePoolThresholds(t *testing.T) { + testCases := []struct { + highThresholds int + lowThresholds int + anomalyCondition *deschedulerconfig.LoadAnomalyCondition + expectedError bool + }{ + { + highThresholds: 100, + lowThresholds: 90, + expectedError: false, + }, + { + highThresholds: 0, + lowThresholds: 0, + expectedError: false, + }, + { + highThresholds: 0, + lowThresholds: -1, + expectedError: true, + }, + { + highThresholds: 100, + lowThresholds: -10, + expectedError: true, + }, + { + highThresholds: -10, + lowThresholds: -100, + expectedError: true, + }, + { + highThresholds: 100, + lowThresholds: 50, + anomalyCondition: &deschedulerconfig.LoadAnomalyCondition{ + ConsecutiveAbnormalities: 5, + }, + expectedError: false, + }, + { + highThresholds: 100, + lowThresholds: 50, + anomalyCondition: &deschedulerconfig.LoadAnomalyCondition{ + ConsecutiveAbnormalities: 0, + }, + expectedError: true, + }, + { + highThresholds: 120, // we do not check threshold larger than 100 + lowThresholds: 50, + expectedError: false, + }, + { + highThresholds: 120, // we do not check threshold larger than 100 + lowThresholds: 120, + expectedError: false, + }, + } + + for _, tc := range testCases { + anomalyCondition := &deschedulerconfig.LoadAnomalyCondition{} + if tc.anomalyCondition != nil { + anomalyCondition.ConsecutiveAbnormalities = tc.anomalyCondition.ConsecutiveAbnormalities + } else { + anomalyCondition.ConsecutiveAbnormalities = 5 + } + args := &deschedulerconfig.LowNodeLoadArgs{ + NodePools: []deschedulerconfig.LowNodeLoadNodePool{ + { + HighThresholds: deschedulerconfig.ResourceThresholds{"cpu": deschedulerconfig.Percentage(tc.highThresholds)}, + LowThresholds: deschedulerconfig.ResourceThresholds{"cpu": deschedulerconfig.Percentage(tc.lowThresholds)}, + AnomalyCondition: anomalyCondition, + }, + }, + } + err := ValidateLowLoadUtilizationArgs(nil, args) + if tc.expectedError { + assert.Error(t, err, "Expected an error for invalid NodePool thresholds") + } else { + assert.Nil(t, err, "Expected no error for valid configuration") + } + } +} diff --git a/pkg/descheduler/apis/config/validation/validation_pluginargs_test.go b/pkg/descheduler/apis/config/validation/validation_pluginargs_test.go index b0c442db4..b7d4a4c66 100644 --- a/pkg/descheduler/apis/config/validation/validation_pluginargs_test.go +++ b/pkg/descheduler/apis/config/validation/validation_pluginargs_test.go @@ -22,6 +22,7 @@ import ( "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/utils/pointer" deschedulerconfig "github.com/koordinator-sh/koordinator/pkg/descheduler/apis/config" @@ -104,3 +105,173 @@ func TestValidateMigrationControllerArgs(t *testing.T) { }) } } + +func TestValidateMigrationControllerArgs_MaxMigratingPerNamespace(t *testing.T) { + testCases := []struct { + maxMigratingPerNamespace *int32 + wantErr bool + }{ + { + maxMigratingPerNamespace: int32Ptr(10), + wantErr: false, + }, + { + maxMigratingPerNamespace: int32Ptr(100), + wantErr: false, + }, + { + maxMigratingPerNamespace: int32Ptr(0), + wantErr: false, + }, + { + maxMigratingPerNamespace: int32Ptr(-1), + wantErr: true, + }, + } + + for _, tc := range testCases { + argsDefault := &v1alpha2.MigrationControllerArgs{} + v1alpha2.SetDefaults_MigrationControllerArgs(argsDefault) + args := &deschedulerconfig.MigrationControllerArgs{} + assert.NoError(t, v1alpha2.Convert_v1alpha2_MigrationControllerArgs_To_config_MigrationControllerArgs(argsDefault, args, nil)) + args.MaxMigratingPerNamespace = tc.maxMigratingPerNamespace + + err := ValidateMigrationControllerArgs(nil, args) + if tc.wantErr { + assert.Error(t, err, "Expected an error for invalid MaxMigratingPerNamespace") + assert.Contains(t, err.Error(), "maxMigratingPerNamespace should be greater or equal 0", "Expected specific error message") + } else { + assert.Nil(t, err, "Expected no error for valid configuration") + } + } +} + +func TestValidateMigrationControllerArgs_MaxMigratingPerNode(t *testing.T) { + testCases := []struct { + maxMigratingPerNode *int32 + wantErr bool + }{ + { + maxMigratingPerNode: int32Ptr(10), + wantErr: false, + }, + { + maxMigratingPerNode: int32Ptr(100), + wantErr: false, + }, + { + maxMigratingPerNode: int32Ptr(0), + wantErr: false, + }, + { + maxMigratingPerNode: int32Ptr(-1), + wantErr: true, + }, + } + + for _, tc := range testCases { + argsDefault := &v1alpha2.MigrationControllerArgs{} + v1alpha2.SetDefaults_MigrationControllerArgs(argsDefault) + args := &deschedulerconfig.MigrationControllerArgs{} + assert.NoError(t, v1alpha2.Convert_v1alpha2_MigrationControllerArgs_To_config_MigrationControllerArgs(argsDefault, args, nil)) + args.MaxMigratingPerNode = tc.maxMigratingPerNode + + err := ValidateMigrationControllerArgs(nil, args) + if tc.wantErr { + assert.Error(t, err, "Expected an error for invalid MaxMigratingPerNode") + assert.Contains(t, err.Error(), "maxMigratingPerNode should be greater or equal 0", "Expected specific error message") + } else { + assert.Nil(t, err, "Expected no error for valid configuration") + } + } +} + +func TestValidateMigrationControllerArgs_MaxMigratingPerWorkload(t *testing.T) { + testCases := []struct { + maxMigratingPerWorkload *intstr.IntOrString + wantErr bool + }{ + { + maxMigratingPerWorkload: intstrPtr(10), + wantErr: false, + }, + { + maxMigratingPerWorkload: intstrPtr(100), + wantErr: false, + }, + { + maxMigratingPerWorkload: intstrPtr(0), + wantErr: false, + }, + { + maxMigratingPerWorkload: intstrPtr(-1), // we do not check the valid value + wantErr: false, + }, + } + + for _, tc := range testCases { + argsDefault := &v1alpha2.MigrationControllerArgs{} + v1alpha2.SetDefaults_MigrationControllerArgs(argsDefault) + args := &deschedulerconfig.MigrationControllerArgs{} + assert.NoError(t, v1alpha2.Convert_v1alpha2_MigrationControllerArgs_To_config_MigrationControllerArgs(argsDefault, args, nil)) + args.MaxMigratingPerWorkload = tc.maxMigratingPerWorkload + + err := ValidateMigrationControllerArgs(nil, args) + if tc.wantErr { + assert.Error(t, err, "Expected an error for invalid MaxMigratingPerWorkload") + assert.Contains(t, err.Error(), "maxMigratingPerWorkload should be greater or equal 0", "Expected specific error message") + } else { + assert.Nil(t, err, "Expected no error for valid configuration") + } + } +} + +func TestValidateMigrationControllerArgs_MaxUnavailablePerWorkload(t *testing.T) { + testCases := []struct { + maxUnavailablePerWorkload *intstr.IntOrString + wantErr bool + }{ + { + maxUnavailablePerWorkload: intstrPtr(10), + wantErr: false, + }, + { + maxUnavailablePerWorkload: intstrPtr(100), + wantErr: false, + }, + { + maxUnavailablePerWorkload: intstrPtr(0), + wantErr: false, + }, + { + maxUnavailablePerWorkload: intstrPtr(-1), // we do not check the valid value + wantErr: false, + }, + } + + for _, tc := range testCases { + argsDefault := &v1alpha2.MigrationControllerArgs{} + v1alpha2.SetDefaults_MigrationControllerArgs(argsDefault) + args := &deschedulerconfig.MigrationControllerArgs{} + assert.NoError(t, v1alpha2.Convert_v1alpha2_MigrationControllerArgs_To_config_MigrationControllerArgs(argsDefault, args, nil)) + args.MaxUnavailablePerWorkload = tc.maxUnavailablePerWorkload + + err := ValidateMigrationControllerArgs(nil, args) + if tc.wantErr { + assert.Error(t, err, "Expected an error for invalid MaxUnavailablePerWorkload") + assert.Contains(t, err.Error(), "maxUnavailablePerWorkload should be greater or equal 0", "Expected specific error message") + } else { + assert.Nil(t, err, "Expected no error for valid configuration") + } + } +} + +// Helper functions for pointer creation +func int32Ptr(value int32) *int32 { + return &value +} + +func intstrPtr(val int) *intstr.IntOrString { + value := intstr.FromInt(val) + return &value +} diff --git a/pkg/features/features.go b/pkg/features/features.go index 9fa7b2380..948a39e66 100644 --- a/pkg/features/features.go +++ b/pkg/features/features.go @@ -36,6 +36,9 @@ const ( // ElasticQuotaValidatingWebhook enables validating webhook for ElasticQuotas creations or updates ElasticQuotaValidatingWebhook featuregate.Feature = "ElasticValidatingWebhook" + // NodeValidatingWebhook enables mutating webhook for Node Creation or updates + NodeMutatingWebhook featuregate.Feature = "NodeMutatingWebhook" + // NodeValidatingWebhook enables validating webhook for Node Creation or updates NodeValidatingWebhook featuregate.Feature = "NodeValidatingWebhook" @@ -45,7 +48,7 @@ const ( // ColocationProfileSkipMutatingResources config whether to update resourceName according to priority by default ColocationProfileSkipMutatingResources featuregate.Feature = "ColocationProfileSkipMutatingResources" - // WebhookFramework enables webhook framework + // WebhookFramework enables webhook framework, global feature-gate for webhook WebhookFramework featuregate.Feature = "WebhookFramework" // MultiQuotaTree enables multi quota tree. @@ -68,6 +71,7 @@ var defaultFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ PodValidatingWebhook: {Default: true, PreRelease: featuregate.Beta}, ElasticQuotaMutatingWebhook: {Default: true, PreRelease: featuregate.Beta}, ElasticQuotaValidatingWebhook: {Default: true, PreRelease: featuregate.Beta}, + NodeMutatingWebhook: {Default: false, PreRelease: featuregate.Alpha}, NodeValidatingWebhook: {Default: false, PreRelease: featuregate.Alpha}, ConfigMapValidatingWebhook: {Default: false, PreRelease: featuregate.Alpha}, WebhookFramework: {Default: true, PreRelease: featuregate.Beta}, diff --git a/pkg/slo-controller/noderesource/plugins/batchresource/util_test.go b/pkg/slo-controller/noderesource/plugins/batchresource/util_test.go index 4aad4f2c4..9e0a86356 100644 --- a/pkg/slo-controller/noderesource/plugins/batchresource/util_test.go +++ b/pkg/slo-controller/noderesource/plugins/batchresource/util_test.go @@ -644,7 +644,7 @@ func Test_divideResourceList(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got := divideResourceList(tt.arg, tt.arg1) - assert.True(t, util.IsResourceListEqualValue(tt.want, got)) + assert.True(t, util.IsResourceListEqual(tt.want, got)) }) } } @@ -652,7 +652,7 @@ func Test_divideResourceList(t *testing.T) { func assertEqualNUMAResourceList(t *testing.T, want, got []corev1.ResourceList) { assert.Equal(t, len(want), len(got)) for i := range want { - assert.True(t, util.IsResourceListEqualValue(want[i], got[i])) + assert.True(t, util.IsResourceListEqual(want[i], got[i])) } } diff --git a/pkg/slo-controller/noderesource/plugins/cpunormalization/plugin.go b/pkg/slo-controller/noderesource/plugins/cpunormalization/plugin.go index 8773a22da..87e13d0db 100644 --- a/pkg/slo-controller/noderesource/plugins/cpunormalization/plugin.go +++ b/pkg/slo-controller/noderesource/plugins/cpunormalization/plugin.go @@ -19,7 +19,6 @@ package cpunormalization import ( "context" "fmt" - "math" "strconv" topologyv1alpha1 "github.com/k8stopologyawareschedwg/noderesourcetopology-api/pkg/apis/topology/v1alpha1" @@ -41,9 +40,8 @@ const ( defaultRatioStr = "1.00" // in case of unexpected resource amplification // NOTE: Currently we do not support the scaling factor below 1.0. - defaultMinRatio = 1.0 - defaultMaxRatio = 5.0 - ratioDiffEpsilon = 0.01 + defaultMinRatio = 1.0 + defaultMaxRatio = 5.0 ) var ( @@ -99,7 +97,7 @@ func (p *Plugin) NeedSyncMeta(_ *configuration.ColocationStrategy, oldNode, newN if ratioNew == -1 { // annotation to remove return true, "new ratio is nil" } - if math.Abs(ratioNew-ratioOld) < ratioDiffEpsilon { + if !extension.IsCPUNormalizationRatioDifferent(ratioOld, ratioNew) { return false, "ratios are close" } diff --git a/pkg/util/resource.go b/pkg/util/resource.go index f58e38175..c3c2f4ac2 100644 --- a/pkg/util/resource.go +++ b/pkg/util/resource.go @@ -77,27 +77,31 @@ func MinResourceList(a corev1.ResourceList, b corev1.ResourceList) corev1.Resour return result } -// IsResourceListEqualValue checks if the two resource lists are numerically equivalent. -// NOTE: Resource name with a zero value will be ignored in comparison. -// e.g. a = {"cpu": "10", "memory": "0"}, b = {"cpu": "10"} => true -func IsResourceListEqualValue(a, b corev1.ResourceList) bool { - a = quotav1.RemoveZeros(a) - b = quotav1.RemoveZeros(b) - if len(a) != len(b) { // different number of no-zero resources +// IsResourceListEqual checks if the two resource lists are numerically equivalent. +func IsResourceListEqual(a corev1.ResourceList, b corev1.ResourceList) bool { + if len(a) != len(b) { return false } - for key, value := range a { - other, found := b[key] - if !found { // different resource names since all zero values have been dropped - return false - } - if value.Cmp(other) != 0 { // different values + for key, val := range a { + valInB, ok := b[key] + if !ok || val.Cmp(valInB) != 0 { return false } } + return true } +// IsResourceListEqualIgnoreZeroValues checks if the two resource lists are numerically equivalent. +// NOTE: Resource name with a zero value will be ignored in comparison. +// e.g. a = {"cpu": "10", "memory": "0"}, b = {"cpu": "10"} => true +func IsResourceListEqualIgnoreZeroValues(a, b corev1.ResourceList) bool { + a = quotav1.RemoveZeros(a) + b = quotav1.RemoveZeros(b) + + return IsResourceListEqual(a, b) +} + // IsResourceDiff returns whether the new resource has big enough difference with the old one or not func IsResourceDiff(old, new corev1.ResourceList, resourceName corev1.ResourceName, diffThreshold float64) bool { oldResource, oldExist := old[resourceName] diff --git a/pkg/util/resource_test.go b/pkg/util/resource_test.go index 6ceba4c01..5c358c02b 100644 --- a/pkg/util/resource_test.go +++ b/pkg/util/resource_test.go @@ -136,7 +136,7 @@ func TestMinResourceList(t *testing.T) { // compatibility check want1 := quotav1.Subtract(quotav1.Add(tt.args.a, tt.args.b), quotav1.Max(tt.args.a, tt.args.b)) - assert.True(t, IsResourceListEqualValue(want1, got), fmt.Sprintf("want: %+v, got: %+v", want1, got)) + assert.True(t, IsResourceListEqualIgnoreZeroValues(want1, got), fmt.Sprintf("want: %+v, got: %+v", want1, got)) }) } } @@ -248,7 +248,7 @@ func TestIsResourceListEqualValue(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := IsResourceListEqualValue(tt.args.a, tt.args.b) + got := IsResourceListEqualIgnoreZeroValues(tt.args.a, tt.args.b) assert.Equal(t, tt.want, got) }) } diff --git a/pkg/webhook/add_node.go b/pkg/webhook/add_node.go index 4d7fab9f0..f995cdf76 100644 --- a/pkg/webhook/add_node.go +++ b/pkg/webhook/add_node.go @@ -19,6 +19,7 @@ package webhook import ( "github.com/koordinator-sh/koordinator/pkg/features" utilfeature "github.com/koordinator-sh/koordinator/pkg/util/feature" + "github.com/koordinator-sh/koordinator/pkg/webhook/node/mutating" "github.com/koordinator-sh/koordinator/pkg/webhook/node/validating" ) @@ -27,4 +28,8 @@ func init() { addHandlersWithGate(validating.HandlerMap, func() (enabled bool) { return utilfeature.DefaultFeatureGate.Enabled(features.NodeValidatingWebhook) }) + + addHandlersWithGate(mutating.HandlerMap, func() (enabled bool) { + return utilfeature.DefaultFeatureGate.Enabled(features.NodeMutatingWebhook) + }) } diff --git a/pkg/webhook/node/mutating/mutating_handler.go b/pkg/webhook/node/mutating/mutating_handler.go new file mode 100644 index 000000000..42e7f81b6 --- /dev/null +++ b/pkg/webhook/node/mutating/mutating_handler.go @@ -0,0 +1,151 @@ +/* +Copyright 2022 The Koordinator Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mutating + +import ( + "context" + "encoding/json" + "net/http" + "reflect" + + admissionv1 "k8s.io/api/admission/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/klog/v2" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/runtime/inject" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/koordinator-sh/koordinator/pkg/webhook/node/plugins" + "github.com/koordinator-sh/koordinator/pkg/webhook/node/plugins/resourceamplification" +) + +var ( + nodeMutatingPlugins = []plugins.NodePlugin{ + resourceamplification.NewPlugin(), + } +) + +type IgnoreFilter func(req admission.Request) bool + +// NodeMutatingHandler handles Node +type NodeMutatingHandler struct { + Client client.Client + + // Decoder decodes objects + Decoder *admission.Decoder + + ignoreFilter IgnoreFilter +} + +/* Uncomment the following lines if you want to enable mutating for node +// NewNodeMutatingHandler creates a new handler for node/status. +func NewNodeMutatingHandler() *NodeMutatingHandler { + handler := &NodeMutatingHandler{ + ignoreFilter: shouldIgnoreIfNotNode, + } + return handler +} + +func shouldIgnoreIfNotNode(req admission.Request) bool { + // Ignore all calls to nodes status or resources other than node. + if len(req.AdmissionRequest.SubResource) != 0 || req.AdmissionRequest.Resource.Resource != "nodes"{ + return true + } + return false +} +*/ + +// NewNodeMutatingHandler creates a new handler for node/status. +func NewNodeStatusMutatingHandler() *NodeMutatingHandler { + handler := &NodeMutatingHandler{ + ignoreFilter: shouldIgnoreIfNotNodeStatus, + } + return handler +} + +var _ admission.Handler = &NodeMutatingHandler{} + +func shouldIgnoreIfNotNodeStatus(req admission.Request) bool { + // Ignore all calls to nodes or resources other than node status. + return req.AdmissionRequest.Resource.Resource != "nodes" || req.AdmissionRequest.SubResource != "status" +} + +// Handle handles admission requests. +func (h *NodeMutatingHandler) Handle(ctx context.Context, req admission.Request) (resp admission.Response) { + if h.ignoreFilter(req) { + return admission.Allowed("") + } + + obj := &corev1.Node{} + var oldObj *corev1.Node + + var err error + if req.Operation != admissionv1.Delete { + err = h.Decoder.Decode(req, obj) + if err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + } else { + if len(req.OldObject.Raw) != 0 { + if err = h.Decoder.DecodeRaw(req.OldObject, obj); err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + } + } + + if req.Operation == admissionv1.Update { + oldObj = &corev1.Node{} + err = h.Decoder.DecodeRaw(req.OldObject, oldObj) + if err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + } + + clone := obj.DeepCopy() + + for _, plugin := range nodeMutatingPlugins { + if err := plugin.Admit(ctx, req, obj, oldObj); err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + } + + if reflect.DeepEqual(obj, clone) { + return admission.Allowed("") + } + marshaled, err := json.Marshal(obj) + if err != nil { + klog.Errorf("Failed to marshal mutated Node %s, err: %v", obj.Name, err) + return admission.Errored(http.StatusInternalServerError, err) + } + return admission.PatchResponseFromRaw(req.AdmissionRequest.Object.Raw, marshaled) +} + +var _ inject.Client = &NodeMutatingHandler{} + +// InjectClient injects the client into the PodMutatingHandler +func (n *NodeMutatingHandler) InjectClient(c client.Client) error { + n.Client = c + return nil +} + +var _ admission.DecoderInjector = &NodeMutatingHandler{} + +// InjectDecoder injects the decoder into the PodMutatingHandler +func (n *NodeMutatingHandler) InjectDecoder(d *admission.Decoder) error { + n.Decoder = d + return nil +} diff --git a/pkg/webhook/node/mutating/mutating_handler_test.go b/pkg/webhook/node/mutating/mutating_handler_test.go new file mode 100644 index 000000000..30774a6a6 --- /dev/null +++ b/pkg/webhook/node/mutating/mutating_handler_test.go @@ -0,0 +1,114 @@ +/* +Copyright 2022 The Koordinator Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mutating + +import ( + "context" + "encoding/json" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + admissionv1 "k8s.io/api/admission/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/koordinator-sh/koordinator/pkg/scheduler/apis/config/scheme" + "github.com/koordinator-sh/koordinator/pkg/webhook/node/plugins" +) + +type mockNodePlugin struct { +} + +func (n *mockNodePlugin) Name() string { + return "mockNodeResourceAmplificationPlugin" +} + +func (n *mockNodePlugin) Validate(ctx context.Context, req admission.Request, node, oldNode *corev1.Node) error { + return fmt.Errorf("not implemented") +} + +// Admit makes an admission decision based on the request attributes +func (n *mockNodePlugin) Admit(ctx context.Context, req admission.Request, node, oldNode *corev1.Node) error { + if req.Operation == admissionv1.Delete { + return nil + } + + if node.Annotations == nil { + node.Annotations = make(map[string]string) + } + node.Annotations["fake-key"] = "fake-value" + return nil +} + +func TestNodeMutatingHandler_Handle(t *testing.T) { + decoder, _ := admission.NewDecoder(scheme.Scheme) + handler := NewNodeStatusMutatingHandler() + handler.InjectDecoder(decoder) + handler.InjectClient(fake.NewClientBuilder().Build()) + + mockRequest := admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Resource: metav1.GroupVersionResource{ + Resource: "nodes", + }, + SubResource: "status", + Operation: admissionv1.Update, + }, + } + + t.Run("NormalMutation", func(t *testing.T) { + node := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: "test-node"}, + Spec: corev1.NodeSpec{ /* populate as needed */ }, + } + nodeRaw, _ := json.Marshal(node) + + mockPlugin := &mockNodePlugin{} + nodeMutatingPlugins = []plugins.NodePlugin{mockPlugin} + mockRequest.Object = runtime.RawExtension{Raw: nodeRaw} + mockRequest.OldObject = runtime.RawExtension{Raw: nodeRaw} + + result := handler.Handle(context.Background(), mockRequest) + + assert.Equal(t, true, result.AdmissionResponse.Allowed) + }) + + t.Run("IgnoreRequest", func(t *testing.T) { + otherResourceRequest := mockRequest + otherResourceRequest.Resource.Resource = "pods" + + result := handler.Handle(context.Background(), otherResourceRequest) + assert.Equal(t, admission.Allowed(""), result) + }) + + t.Run("HandleDeleteOperation", func(t *testing.T) { + node := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: "test-node"}, + } + nodeRaw, _ := json.Marshal(node) + + mockRequest.OldObject = runtime.RawExtension{Raw: nodeRaw} + mockRequest.Operation = admissionv1.Delete + + result := handler.Handle(context.Background(), mockRequest) + assert.Equal(t, admission.Allowed(""), result) + }) +} diff --git a/pkg/webhook/node/mutating/webhooks.go b/pkg/webhook/node/mutating/webhooks.go new file mode 100644 index 000000000..d52ae7b48 --- /dev/null +++ b/pkg/webhook/node/mutating/webhooks.go @@ -0,0 +1,30 @@ +/* +Copyright 2022 The Koordinator Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mutating + +import ( + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +// +kubebuilder:webhook:path=/mutate-node-status,mutating=true,failurePolicy=ignore,sideEffects=None,groups="",resources=nodes/status,verbs=create;update,versions=v1,name=mnode-status.koordinator.sh,admissionReviewVersions=v1;v1beta1 + +var ( + // HandlerMap contains admission webhook handlers + HandlerMap = map[string]admission.Handler{ + "mutate-node-status": NewNodeStatusMutatingHandler(), + } +) diff --git a/pkg/webhook/node/plugins/resourceamplification/resource_amplification.go b/pkg/webhook/node/plugins/resourceamplification/resource_amplification.go new file mode 100644 index 000000000..8473e144c --- /dev/null +++ b/pkg/webhook/node/plugins/resourceamplification/resource_amplification.go @@ -0,0 +1,164 @@ +/* +Copyright 2022 The Koordinator Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package resourceamplification + +import ( + "context" + "fmt" + + admissionv1 "k8s.io/api/admission/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/klog/v2" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/koordinator-sh/koordinator/apis/extension" + "github.com/koordinator-sh/koordinator/pkg/util" + "github.com/koordinator-sh/koordinator/pkg/webhook/node/plugins" +) + +var ( + // Only supports amplification of cpu and memory resources + supportedResources = []corev1.ResourceName{ + corev1.ResourceCPU, + corev1.ResourceMemory, + } +) + +func NewPlugin() *NodeResourceAmplificationPlugin { + plugin := &NodeResourceAmplificationPlugin{} + return plugin +} + +var _ plugins.NodePlugin = &NodeResourceAmplificationPlugin{} + +type NodeResourceAmplificationPlugin struct { +} + +func (n *NodeResourceAmplificationPlugin) Name() string { + return "NodeResourceAmplificationPlugin" +} + +func (n *NodeResourceAmplificationPlugin) Validate(ctx context.Context, req admission.Request, node, oldNode *corev1.Node) error { + return fmt.Errorf("not implemented") +} + +// Admit makes an admission decision based on the request attributes +func (n *NodeResourceAmplificationPlugin) Admit(ctx context.Context, req admission.Request, node, oldNode *corev1.Node) error { + klog.V(3).Infof("enter NodeResourceAmplificationPlugin plugin admit for %s", req.AdmissionRequest.Name) + + op := req.AdmissionRequest.Operation + switch op { + case admissionv1.Create: + return nil + case admissionv1.Update: + return n.handleUpdate(oldNode, node) + } + return nil +} + +// isSupportedResourceChanged checks if the supported resources are changed +func (n *NodeResourceAmplificationPlugin) isSupportedResourceChanged(oldNode, node *corev1.Node) bool { + if oldNode == nil || node == nil { + return false + } + oldOriginalResources := corev1.ResourceList{} + newOriginalResources := corev1.ResourceList{} + for _, resourceName := range supportedResources { + oldResourceValue, ok := oldNode.Status.Allocatable[resourceName] + if ok { + oldOriginalResources[resourceName] = oldResourceValue + } + resourceValue, ok := node.Status.Allocatable[resourceName] + if ok { + newOriginalResources[resourceName] = resourceValue + } + } + return !util.IsResourceListEqual(oldOriginalResources, newOriginalResources) +} + +func (n *NodeResourceAmplificationPlugin) handleUpdate(oldNode, node *corev1.Node) error { + if node.Annotations[extension.AnnotationNodeResourceAmplificationRatio] == "" { + // When the feature is turned off, the saved raw allocatable needs to be cleaned up + delete(node.Annotations, extension.AnnotationNodeRawAllocatable) + return nil + } + if node.Status.Allocatable == nil { + // do nothing + return nil + } + + // Set original resource under two conditions + // 1. if the original resource is not set + // 2. Node allocatable are updated, such as the administrator adjusting the reserved resource + // https://kubernetes.io/docs/tasks/administer-cluster/reserve-compute-resources + // Here, we rely on an assumption that only kubelet will update allocatable, and other roles will only expand new fields and not overwrite native fields. + // FIXME + // 1. If the updated resource of kubelet happens to be the same as the amplificated allocatable, the update will not take effect. + // 2. If the webhook policy is configured to ignore, allocatable changes in the kubelet while the webhook is unavailable will not be observed. + originalResources := corev1.ResourceList{} + if !extension.HasNodeRawAllocatable(node.Annotations) || n.isSupportedResourceChanged(oldNode, node) { + // resources in other dimensions will be dynamically updated, so they are not saved here. The caller needs to be aware of this feature. + for _, resourceName := range supportedResources { + resourceValue, ok := node.Status.Allocatable[resourceName] + if ok { + originalResources[resourceName] = resourceValue + } + } + if len(originalResources) > 0 { + klog.V(2).Infof("admit for %s, set raw allocatable %v", node.Name, originalResources) + extension.SetNodeRawAllocatable(node, originalResources) + } + } else { + originalResources, err := extension.GetNodeRawAllocatable(node.Annotations) + if originalResources == nil { + klog.Errorf("admit for %s, failed to parse raw allocatable: %v", node.Name, err) + return err + } + } + + // parse resource amplification ratios from annotations + amplificationRatios, err := extension.GetNodeResourceAmplificationRatios(node.Annotations) + if err != nil { + klog.Errorf("admit for %s, failed to parse resource amplification ratios: %v", node.Name, err) + return err + } + + // update allocatable by originalResources * amplificationRatios: + // originalResources: {cpu: 1000, memory: 2000} * amplificationRatios: {cpu: 2, memory: 3} = {cpu: 2000, memory: 6000} + // If the resources saved in original are missing, the original will still prevail, and the missing dimensions will not be scaled: + // originalResources: {cpu: 1000} * amplificationRatios: {cpu: 2, memory: 3} = {cpu: 2000} + // If the resource dimensions supported by the amplification change, the latest one shall prevail, and the missing dimensions will not be amplified: + // originalResources: {cpu: 1000, memory: 2000} * amplificationRatios: {cpu: 2} = {cpu: 2000} + for _, resourceName := range supportedResources { + ratio, ok := amplificationRatios[resourceName] + if !ok { + continue + } + if ratio <= 1 { + klog.V(4).Infof("admit for %s, skip resource %v as ratio %v <= 1", node.Name, resourceName, ratio) + continue + } + resourceValue, ok := originalResources[resourceName] + if !ok { + klog.V(4).Infof("admit for %s, skip resource %v as it is not found in original resources", node.Name, resourceName) + continue + } + node.Status.Allocatable[resourceName] = util.MultiplyMilliQuant(resourceValue, float64(ratio)) + } + + return nil +} diff --git a/pkg/webhook/node/plugins/resourceamplification/resource_amplification_test.go b/pkg/webhook/node/plugins/resourceamplification/resource_amplification_test.go new file mode 100644 index 000000000..efc6974fd --- /dev/null +++ b/pkg/webhook/node/plugins/resourceamplification/resource_amplification_test.go @@ -0,0 +1,339 @@ +/* +Copyright 2022 The Koordinator Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package resourceamplification + +import ( + "context" + "testing" + + admissionv1 "k8s.io/api/admission/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/koordinator-sh/koordinator/apis/extension" +) + +func TestNodeResourceAmplificationPlugin_handleNormalization(t *testing.T) { + testCases := []struct { + name string + annotations map[string]string + resourceName corev1.ResourceName + alloatableValue int64 + expectedErr bool + expectedAllocatableValue int64 + expectedOriginalAllocatable int64 + }{ + + { + name: "normal", + annotations: map[string]string{extension.AnnotationNodeResourceAmplificationRatio: `{"cpu": 1.5}`}, + resourceName: corev1.ResourceCPU, + alloatableValue: 1000, + expectedErr: false, + expectedAllocatableValue: 1500, + expectedOriginalAllocatable: 1000, + }, + { + name: "disable1", + annotations: map[string]string{extension.AnnotationNodeResourceAmplificationRatio: `{"cpu": 0.5}`}, + resourceName: corev1.ResourceCPU, + alloatableValue: 1000, + expectedErr: false, + expectedAllocatableValue: 1000, + expectedOriginalAllocatable: 1000, + }, + { + name: "disable2", + annotations: map[string]string{extension.AnnotationNodeResourceAmplificationRatio: `{"cpu": 1}`}, + resourceName: corev1.ResourceCPU, + alloatableValue: 1000, + expectedErr: false, + expectedAllocatableValue: 1000, + expectedOriginalAllocatable: 1000, + }, + { + name: "disable amplication will remove original allocatable", + annotations: map[string]string{ + extension.AnnotationNodeRawAllocatable: "{}", + }, + resourceName: corev1.ResourceCPU, + alloatableValue: 1000, + expectedErr: false, + expectedAllocatableValue: 1000, + expectedOriginalAllocatable: -1, + }, + { + name: "unsupported ratio will be ignored", + annotations: map[string]string{extension.AnnotationNodeResourceAmplificationRatio: `{"eni": 2}`}, + resourceName: corev1.ResourceName("eni"), + alloatableValue: 1000, + expectedErr: false, + expectedAllocatableValue: 1000, + expectedOriginalAllocatable: 1000, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + plugin := &NodeResourceAmplificationPlugin{} + + node := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-node", + Annotations: tc.annotations, + }, + Status: corev1.NodeStatus{ + Allocatable: corev1.ResourceList{ + corev1.ResourceCPU: *resource.NewMilliQuantity(tc.alloatableValue, resource.DecimalSI), + }, + }, + } + err := plugin.handleUpdate(nil, node) + + // Check the result + if tc.expectedErr { + if err == nil { + t.Errorf("Expected error, but got err: nil") + } + } else { + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + + cpu := node.Status.Allocatable[corev1.ResourceCPU] + // Check the Allocatable value + allocatableValue := cpu.MilliValue() + if allocatableValue != tc.expectedAllocatableValue { + t.Errorf("Expected Allocatable value: %d, got: %d", tc.expectedAllocatableValue, allocatableValue) + } + original, err := extension.GetNodeRawAllocatable(node.Annotations) + if tc.expectedOriginalAllocatable > 0 { + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + if original.Cpu().MilliValue() != tc.expectedOriginalAllocatable { + t.Errorf("Expected original Allocatable value: %d, got: %d", tc.expectedOriginalAllocatable, original.Cpu().MilliValue()) + } + } else { + _, ok := node.Annotations[extension.AnnotationNodeRawAllocatable] + if ok { + t.Errorf("unexpected raw allocatable") + } + } + } + }) + } +} + +func TestNodeResourceAmplificationPlugin_Admit(t *testing.T) { + testCases := []struct { + name string + admissionRequest *admissionv1.AdmissionRequest + node *corev1.Node + oldNode *corev1.Node + expectedErr bool + }{ + { + name: "CreateOperation", + admissionRequest: &admissionv1.AdmissionRequest{ + Operation: admissionv1.Create, + }, + node: &corev1.Node{}, + oldNode: &corev1.Node{}, + expectedErr: false, + }, + { + name: "UpdateOperation", + admissionRequest: &admissionv1.AdmissionRequest{ + Operation: admissionv1.Update, + }, + node: &corev1.Node{}, + oldNode: &corev1.Node{}, + expectedErr: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + plugin := NewPlugin() + + request := admission.Request{ + AdmissionRequest: *tc.admissionRequest, + } + + err := plugin.Admit(context.TODO(), request, tc.node, tc.oldNode) + + if tc.expectedErr && err == nil { + t.Errorf("Expected an error, but got nil") + } else if !tc.expectedErr && err != nil { + t.Errorf("Expected no error, but got: %v", err) + } + }) + } +} + +func TestNodeResourceAmplificationPlugin_Validate(t *testing.T) { + plugin := NewPlugin() + request := admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Create, + }, + } + + err := plugin.Admit(context.TODO(), request, &corev1.Node{}, &corev1.Node{}) + if err != nil { + t.Fatal(err) + } +} + +func TestNodeResourceAmplificationPlugin_isSupportedResourceChanged(t *testing.T) { + testCases := []struct { + name string + oldNode *corev1.Node + newNode *corev1.Node + expected bool + }{ + { + name: "oldNode is nil", + oldNode: nil, + newNode: &corev1.Node{}, + expected: false, + }, + { + name: "newNode is nil", + oldNode: &corev1.Node{}, + newNode: nil, + expected: false, + }, + { + name: "oldNode & newNode is nil", + oldNode: nil, + newNode: nil, + expected: false, + }, + { + name: "resource is unchanged", + oldNode: &corev1.Node{ + Status: corev1.NodeStatus{ + Allocatable: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("4"), + corev1.ResourceMemory: resource.MustParse("8Gi"), + }, + }, + }, + newNode: &corev1.Node{ + Status: corev1.NodeStatus{ + Allocatable: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("4"), + corev1.ResourceMemory: resource.MustParse("8Gi"), + }, + }, + }, + expected: false, + }, + { + name: "add resource", + oldNode: &corev1.Node{ + Status: corev1.NodeStatus{ + Allocatable: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("4"), + }, + }, + }, + newNode: &corev1.Node{ + Status: corev1.NodeStatus{ + Allocatable: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("4"), + corev1.ResourceMemory: resource.MustParse("8Gi"), + }, + }, + }, + expected: true, + }, + { + name: "remove resource", + oldNode: &corev1.Node{ + Status: corev1.NodeStatus{ + Allocatable: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("4"), + corev1.ResourceMemory: resource.MustParse("8Gi"), + }, + }, + }, + newNode: &corev1.Node{ + Status: corev1.NodeStatus{ + Allocatable: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("4"), + }, + }, + }, + expected: true, + }, + { + name: "remove updated", + oldNode: &corev1.Node{ + Status: corev1.NodeStatus{ + Allocatable: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("4"), + corev1.ResourceMemory: resource.MustParse("8Gi"), + }, + }, + }, + newNode: &corev1.Node{ + Status: corev1.NodeStatus{ + Allocatable: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("5"), + corev1.ResourceMemory: resource.MustParse("8Gi"), + }, + }, + }, + expected: true, + }, + { + name: "unsupported resource remove", + oldNode: &corev1.Node{ + Status: corev1.NodeStatus{ + Allocatable: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("4"), + corev1.ResourceEphemeralStorage: resource.MustParse("80"), + }, + }, + }, + newNode: &corev1.Node{ + Status: corev1.NodeStatus{ + Allocatable: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("4"), + corev1.ResourceEphemeralStorage: resource.MustParse("100"), + }, + }, + }, + expected: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + plugin := &NodeResourceAmplificationPlugin{} + + if actual := plugin.isSupportedResourceChanged(tc.oldNode, tc.newNode); actual != tc.expected { + t.Errorf("Expected %v, got %v", tc.expected, actual) + } + }) + } +}