Add more tests

alikhil · alikhil · commit cb19573cd6f0 · 2025-12-09T16:20:47.000+01:00
Signed-off-by: Alik Khilazhev &lt;7482065+alikhil@users.noreply.github.com&gt;
diff --git a/apis/config/scheme/scheme_test.go b/apis/config/scheme/scheme_test.go
@@ -231,6 +231,78 @@ profiles:
 				},
 			},
 		},
+		{
+			name: "v1 NodeMetadata plugin args",
+			data: []byte(`
+apiVersion: kubescheduler.config.k8s.io/v1
+kind: KubeSchedulerConfiguration
+profiles:
+- schedulerName: scheduler-plugins
+  pluginConfig:
+  - name: NodeMetadata
+    args:
+      metadataKey: "priority"
+      metadataSource: "Label"
+      metadataType: "Number"
+      scoringStrategy: "Highest"
+`),
+			wantProfiles: []schedconfig.KubeSchedulerProfile{
+				{
+					SchedulerName: "scheduler-plugins",
+					Plugins:       defaults.PluginsV1,
+					PluginConfig: []schedconfig.PluginConfig{
+						{
+							Name: "NodeMetadata",
+							Args: &config.NodeMetadataArgs{
+								MetadataKey:     "priority",
+								MetadataSource:  config.MetadataSourceLabel,
+								MetadataType:    config.MetadataTypeNumber,
+								ScoringStrategy: config.ScoringStrategyHighest,
+							},
+						},
+						{
+							Name: "DefaultPreemption",
+							Args: &schedconfig.DefaultPreemptionArgs{MinCandidateNodesPercentage: 10, MinCandidateNodesAbsolute: 100},
+						},
+						{
+							Name: "DynamicResources",
+							Args: &schedconfig.DynamicResourcesArgs{
+								FilterTimeout: ptr.To(metav1.Duration{Duration: 10 * time.Second}),
+							},
+						},
+						{
+							Name: "InterPodAffinity",
+							Args: &schedconfig.InterPodAffinityArgs{HardPodAffinityWeight: 1},
+						},
+						{
+							Name: "NodeAffinity",
+							Args: &schedconfig.NodeAffinityArgs{},
+						},
+						{
+							Name: "NodeResourcesBalancedAllocation",
+							Args: &schedconfig.NodeResourcesBalancedAllocationArgs{Resources: []schedconfig.ResourceSpec{{Name: "cpu", Weight: 1}, {Name: "memory", Weight: 1}}},
+						},
+						{
+							Name: "NodeResourcesFit",
+							Args: &schedconfig.NodeResourcesFitArgs{
+								ScoringStrategy: &schedconfig.ScoringStrategy{
+									Type:      schedconfig.LeastAllocated,
+									Resources: []schedconfig.ResourceSpec{{Name: "cpu", Weight: 1}, {Name: "memory", Weight: 1}},
+								},
+							},
+						},
+						{
+							Name: "PodTopologySpread",
+							Args: &schedconfig.PodTopologySpreadArgs{DefaultingType: schedconfig.SystemDefaulting},
+						},
+						{
+							Name: "VolumeBinding",
+							Args: &schedconfig.VolumeBindingArgs{BindTimeoutSeconds: 600},
+						},
+					},
+				},
+			},
+		},
 	}
 	decoder := Codecs.UniversalDecoder()
 	for _, tt := range testCases {
@@ -348,6 +420,15 @@ func TestCodecsEncodePluginConfig(t *testing.T) {
 									NetworkTopologyName: "net-topology-v1",
 								},
 							},
+							{
+								Name: "NodeMetadata",
+								Args: &config.NodeMetadataArgs{
+									MetadataKey:     "priority",
+									MetadataSource:  config.MetadataSourceLabel,
+									MetadataType:    config.MetadataTypeNumber,
+									ScoringStrategy: config.ScoringStrategyHighest,
+								},
+							},
 						},
 					},
 				},
@@ -445,6 +526,15 @@ profiles:
       networkTopologyName: net-topology-v1
       weightsName: netCosts
     name: NetworkOverhead
+  - args:
+      apiVersion: kubescheduler.config.k8s.io/v1
+      kind: NodeMetadataArgs
+      metadataKey: priority
+      metadataSource: Label
+      metadataType: Number
+      scoringStrategy: Highest
+      timestampFormat: ""
+    name: NodeMetadata
   schedulerName: scheduler-plugins
 `,
 		},
diff --git a/apis/config/v1/defaults.go b/apis/config/v1/defaults.go
@@ -18,10 +18,13 @@ package v1
 
 import (
 	"strconv"
+	"time"
 
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/ptr"
+
 	schedulerconfigv1 "k8s.io/kube-scheduler/config/v1"
 	k8sschedulerconfigv1 "k8s.io/kubernetes/pkg/scheduler/apis/config/v1"
 )
@@ -250,3 +253,10 @@ func SetDefaults_SySchedArgs(obj *SySchedArgs) {
 		obj.DefaultProfileName = &DefaultSySchedProfileName
 	}
 }
+
+// SetDefaults_NodeMetadataArgs sets the default parameters for NodeMetadataArgs plugin.
+func SetDefaults_NodeMetadataArgs(obj *NodeMetadataArgs) {
+	if obj.TimestampFormat == nil && obj.MetadataType != nil && *obj.MetadataType == MetadataTypeTimestamp {
+		obj.TimestampFormat = ptr.To(time.RFC3339)
+	}
+}
diff --git a/apis/config/v1/zz_generated.defaults.go b/apis/config/v1/zz_generated.defaults.go
diff --git a/apis/config/validation/validation_pluginargs.go b/apis/config/validation/validation_pluginargs.go
@@ -109,3 +109,57 @@ func ValidateCoschedulingArgs(args *config.CoschedulingArgs, _ *field.Path) erro
 	}
 	return allErrs.ToAggregate()
 }
+
+func ValidateNodeMetadataArgs(args *config.NodeMetadataArgs, path *field.Path) error {
+	var allErrs field.ErrorList
+
+	// Validate MetadataKey is not empty
+	if args.MetadataKey == "" {
+		allErrs = append(allErrs, field.Invalid(field.NewPath("metadataKey"),
+			args.MetadataKey, "metadataKey cannot be empty"))
+	}
+
+	// Validate MetadataSource
+	if args.MetadataSource != config.MetadataSourceLabel && args.MetadataSource != config.MetadataSourceAnnotation {
+		allErrs = append(allErrs, field.Invalid(field.NewPath("metadataSource"),
+			args.MetadataSource, "metadataSource must be either \"Label\" or \"Annotation\""))
+	}
+
+	// Validate MetadataType
+	if args.MetadataType != config.MetadataTypeNumber && args.MetadataType != config.MetadataTypeTimestamp {
+		allErrs = append(allErrs, field.Invalid(field.NewPath("metadataType"),
+			args.MetadataType, "metadataType must be either \"Number\" or \"Timestamp\""))
+	}
+
+	// Validate ScoringStrategy
+	validStrategies := sets.New[string](
+		string(config.ScoringStrategyHighest),
+		string(config.ScoringStrategyLowest),
+		string(config.ScoringStrategyNewest),
+		string(config.ScoringStrategyOldest),
+	)
+	if !validStrategies.Has(string(args.ScoringStrategy)) {
+		allErrs = append(allErrs, field.Invalid(field.NewPath("scoringStrategy"),
+			args.ScoringStrategy, "scoringStrategy must be one of \"Highest\", \"Lowest\", \"Newest\", or \"Oldest\""))
+	}
+
+	// Validate compatibility between MetadataType and ScoringStrategy
+	if args.MetadataType == config.MetadataTypeNumber {
+		if args.ScoringStrategy == config.ScoringStrategyNewest || args.ScoringStrategy == config.ScoringStrategyOldest {
+			allErrs = append(allErrs, field.Invalid(field.NewPath("scoringStrategy"),
+				args.ScoringStrategy, "scoringStrategy \"Newest\" and \"Oldest\" are only valid for metadataType \"Timestamp\""))
+		}
+	}
+
+	if args.MetadataType == config.MetadataTypeTimestamp {
+		if args.ScoringStrategy == config.ScoringStrategyHighest || args.ScoringStrategy == config.ScoringStrategyLowest {
+			allErrs = append(allErrs, field.Invalid(field.NewPath("scoringStrategy"),
+				args.ScoringStrategy, "scoringStrategy \"Highest\" and \"Lowest\" are only valid for metadataType \"Number\""))
+		}
+	}
+
+	if len(allErrs) == 0 {
+		return nil
+	}
+	return allErrs.ToAggregate()
+}
diff --git a/apis/config/validation/validation_plugingargs_test.go b/apis/config/validation/validation_plugingargs_test.go
@@ -193,3 +193,129 @@ func TestValidateNodeResourcesAllocatableArgs(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateNodeMetadataArgs(t *testing.T) {
+	testCases := []struct {
+		args        *config.NodeMetadataArgs
+		expectedErr error
+		description string
+	}{
+		{
+			description: "correct config with label source and numeric type",
+			args: &config.NodeMetadataArgs{
+				MetadataKey:     "priority",
+				MetadataSource:  config.MetadataSourceLabel,
+				MetadataType:    config.MetadataTypeNumber,
+				ScoringStrategy: config.ScoringStrategyHighest,
+			},
+			expectedErr: nil,
+		},
+		{
+			description: "correct config with annotation source and timestamp type",
+			args: &config.NodeMetadataArgs{
+				MetadataKey:     "lastUpdate",
+				MetadataSource:  config.MetadataSourceAnnotation,
+				MetadataType:    config.MetadataTypeTimestamp,
+				ScoringStrategy: config.ScoringStrategyNewest,
+			},
+			expectedErr: nil,
+		},
+		{
+			description: "missing MetadataKey",
+			args: &config.NodeMetadataArgs{
+				MetadataKey:     "",
+				MetadataSource:  config.MetadataSourceLabel,
+				MetadataType:    config.MetadataTypeNumber,
+				ScoringStrategy: config.ScoringStrategyHighest,
+			},
+			expectedErr: fmt.Errorf("metadataKey cannot be empty"),
+		},
+		{
+			description: "invalid MetadataSource",
+			args: &config.NodeMetadataArgs{
+				MetadataKey:     "priority",
+				MetadataSource:  "InvalidSource",
+				MetadataType:    config.MetadataTypeNumber,
+				ScoringStrategy: config.ScoringStrategyHighest,
+			},
+			expectedErr: fmt.Errorf("metadataSource must be either \"Label\" or \"Annotation\""),
+		},
+		{
+			description: "invalid MetadataType",
+			args: &config.NodeMetadataArgs{
+				MetadataKey:     "priority",
+				MetadataSource:  config.MetadataSourceLabel,
+				MetadataType:    "InvalidType",
+				ScoringStrategy: config.ScoringStrategyHighest,
+			},
+			expectedErr: fmt.Errorf("metadataType must be either \"Number\" or \"Timestamp\""),
+		},
+		{
+			description: "invalid ScoringStrategy",
+			args: &config.NodeMetadataArgs{
+				MetadataKey:     "priority",
+				MetadataSource:  config.MetadataSourceLabel,
+				MetadataType:    config.MetadataTypeNumber,
+				ScoringStrategy: "InvalidStrategy",
+			},
+			expectedErr: fmt.Errorf("scoringStrategy must be one of \"Highest\", \"Lowest\", \"Newest\", or \"Oldest\""),
+		},
+		{
+			description: "numeric type with Newest strategy (mismatch)",
+			args: &config.NodeMetadataArgs{
+				MetadataKey:     "priority",
+				MetadataSource:  config.MetadataSourceLabel,
+				MetadataType:    config.MetadataTypeNumber,
+				ScoringStrategy: config.ScoringStrategyNewest,
+			},
+			expectedErr: fmt.Errorf("scoringStrategy \"Newest\" and \"Oldest\" are only valid for metadataType \"Timestamp\""),
+		},
+		{
+			description: "timestamp type with Highest strategy (mismatch)",
+			args: &config.NodeMetadataArgs{
+				MetadataKey:     "lastUpdate",
+				MetadataSource:  config.MetadataSourceAnnotation,
+				MetadataType:    config.MetadataTypeTimestamp,
+				ScoringStrategy: config.ScoringStrategyHighest,
+			},
+			expectedErr: fmt.Errorf("scoringStrategy \"Highest\" and \"Lowest\" are only valid for metadataType \"Number\""),
+		},
+		{
+			description: "all valid combinations - Lowest with Number",
+			args: &config.NodeMetadataArgs{
+				MetadataKey:     "cost",
+				MetadataSource:  config.MetadataSourceAnnotation,
+				MetadataType:    config.MetadataTypeNumber,
+				ScoringStrategy: config.ScoringStrategyLowest,
+			},
+			expectedErr: nil,
+		},
+		{
+			description: "all valid combinations - Oldest with Timestamp",
+			args: &config.NodeMetadataArgs{
+				MetadataKey:     "provisionedAt",
+				MetadataSource:  config.MetadataSourceLabel,
+				MetadataType:    config.MetadataTypeTimestamp,
+				ScoringStrategy: config.ScoringStrategyOldest,
+			},
+			expectedErr: nil,
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.description, func(t *testing.T) {
+			err := ValidateNodeMetadataArgs(testCase.args, nil)
+			if testCase.expectedErr != nil {
+				if err == nil {
+					t.Fatalf("expected err to equal %v not nil", testCase.expectedErr)
+				}
+				if !strings.Contains(err.Error(), testCase.expectedErr.Error()) {
+					t.Fatalf("expected err to contain %s in error message: %s", testCase.expectedErr.Error(), err.Error())
+				}
+			}
+			if testCase.expectedErr == nil && err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+		})
+	}
+}
diff --git a/pkg/nodemetadata/node_metadata.go b/pkg/nodemetadata/node_metadata.go
@@ -29,6 +29,7 @@ import (
 	"k8s.io/kubernetes/pkg/scheduler/framework"
 
 	"sigs.k8s.io/scheduler-plugins/apis/config"
+	"sigs.k8s.io/scheduler-plugins/apis/config/validation"
 )
 
 // NodeMetadata is a plugin that scores nodes based on their metadata (labels or annotations)
@@ -199,7 +200,7 @@ func New(ctx context.Context, obj runtime.Object, h framework.Handle) (framework
 	}
 
 	// Validate arguments
-	if err := validateArgs(args); err != nil {
+	if err := validation.ValidateNodeMetadataArgs(args, nil); err != nil {
 		return nil, fmt.Errorf("invalid NodeMetadataArgs: %w", err)
 	}
 
@@ -209,34 +210,3 @@ func New(ctx context.Context, obj runtime.Object, h framework.Handle) (framework
 		args:   args,
 	}, nil
 }
-
-// validateArgs validates the plugin arguments
-func validateArgs(args *config.NodeMetadataArgs) error {
-	if args.MetadataKey == "" {
-		return fmt.Errorf("metadataKey cannot be empty")
-	}
-
-	if args.MetadataSource != config.MetadataSourceLabel && args.MetadataSource != config.MetadataSourceAnnotation {
-		return fmt.Errorf("metadataSource must be either %q or %q", config.MetadataSourceLabel, config.MetadataSourceAnnotation)
-	}
-
-	if args.MetadataType != config.MetadataTypeNumber && args.MetadataType != config.MetadataTypeTimestamp {
-		return fmt.Errorf("metadataType must be either %q or %q", config.MetadataTypeNumber, config.MetadataTypeTimestamp)
-	}
-
-	// Validate scoring strategy based on metadata type
-	if args.MetadataType == config.MetadataTypeNumber {
-		if args.ScoringStrategy != config.ScoringStrategyHighest && args.ScoringStrategy != config.ScoringStrategyLowest {
-			return fmt.Errorf("for numeric metadata, scoringStrategy must be either %q or %q", config.ScoringStrategyHighest, config.ScoringStrategyLowest)
-		}
-	} else if args.MetadataType == config.MetadataTypeTimestamp {
-		if args.ScoringStrategy != config.ScoringStrategyNewest && args.ScoringStrategy != config.ScoringStrategyOldest {
-			return fmt.Errorf("for timestamp metadata, scoringStrategy must be either %q or %q", config.ScoringStrategyNewest, config.ScoringStrategyOldest)
-		}
-		if args.TimestampFormat == "" {
-			return fmt.Errorf("timestampFormat cannot be empty when metadataType is %q", config.MetadataTypeTimestamp)
-		}
-	}
-
-	return nil
-}
diff --git a/test/integration/nodemetadata_test.go b/test/integration/nodemetadata_test.go
