WIP

Signed-off-by: Francesco Romani <[email protected]>

pkg/manifests/schedparams.go

@@ -35,15 +35,12 @@ type ConfigCacheParams struct {
 }
 
 type ConfigParams struct {
-	Cache *ConfigCacheParams
+	ProfileName string // can't be empty, so no need for pointer
+	Cache       *ConfigCacheParams
 }
 
-func DecodeSchedulerConfigFromData(data []byte, schedulerName string) (ConfigParams, error) {
-	params := ConfigParams{}
-	if schedulerName == "" {
-		klog.InfoS("missing scheduler name")
-		return params, nil
-	}
+func DecodeSchedulerProfilesFromData(data []byte) ([]ConfigParams, error) {
+	params := []ConfigParams{}
 
 	var r unstructured.Unstructured
 	if err := yaml.Unmarshal(data, &r.Object); err != nil {
@@ -69,10 +66,6 @@ func DecodeSchedulerConfigFromData(data []byte, schedulerName string) (ConfigPar
 			return params, nil
 		}
 
-		if profileName != schedulerName {
-			continue
-		}
-
 		pluginConfigs, ok, err := unstructured.NestedSlice(profile, "pluginConfig")
 		if !ok || err != nil {
 			klog.ErrorS(err, "failed to process unstructured data", "pluginConfig", ok)
@@ -99,16 +92,34 @@ func DecodeSchedulerConfigFromData(data []byte, schedulerName string) (ConfigPar
 				return params, nil
 			}
 
-			return extractParams(args)
+			profileParams, err := extractParams(profileName, args)
+			if err != nil {
+				klog.ErrorS(err, "failed to extract params", "name", name, "profile", profileName)
+				continue
+			}
+
+			params = append(params, profileParams)
 		}
 	}
 
-	return params, fmt.Errorf("failed to find parameters for schedulerName %q", schedulerName)
+	return params, nil
+
+}
+
+func FindSchedulerProfileByName(profileParams []ConfigParams, schedulerName string) *ConfigParams {
+	for idx := range profileParams {
+		params := &profileParams[idx]
+		if params.ProfileName == schedulerName {
+			return params
+		}
+	}
+	return nil
 }
 
-func extractParams(args map[string]interface{}) (ConfigParams, error) {
+func extractParams(profileName string, args map[string]interface{}) (ConfigParams, error) {
 	params := ConfigParams{
-		Cache: &ConfigCacheParams{},
+		ProfileName: profileName,
+		Cache:       &ConfigCacheParams{},
 	}
 	// json quirk: we know it's int64, yet it's detected as float64
 	resyncPeriod, ok, err := unstructured.NestedFloat64(args, "cacheResyncPeriodSeconds")

pkg/manifests/schedparams_test.go

@@ -28,16 +28,15 @@ func TestDecodeSchedulerConfigFromData(t *testing.T) {
 		name           string
 		data           []byte
 		schedulerName  string
+		expectedFound  bool
 		expectedParams ConfigParams
-		expectedError  bool
 	}
 	testCases := []testCase{
 		{
 			name:           "nil",
 			data:           nil,
 			schedulerName:  "",
 			expectedParams: ConfigParams{},
-			expectedError:  false,
 		},
 		{
 			name: "bad scheduler name",
@@ -63,7 +62,6 @@ profiles:
 `),
 			schedulerName:  "topo-aware-scheduler",
 			expectedParams: ConfigParams{},
-			expectedError:  true,
 		},
 		{
 			name: "bad scheduler params name",
@@ -89,7 +87,6 @@ profiles:
 `),
 			schedulerName:  "topology-aware-scheduler",
 			expectedParams: ConfigParams{},
-			expectedError:  true,
 		},
 		{
 			name: "empty params",
@@ -115,9 +112,10 @@ profiles:
 `),
 			schedulerName: "topology-aware-scheduler",
 			expectedParams: ConfigParams{
-				Cache: &ConfigCacheParams{},
+				ProfileName: "topology-aware-scheduler",
+				Cache:       &ConfigCacheParams{},
 			},
-			expectedError: false,
+			expectedFound: true,
 		},
 		{
 			name: "nonzero resync period",
@@ -144,21 +142,34 @@ profiles:
 `),
 			schedulerName: "topology-aware-scheduler",
 			expectedParams: ConfigParams{
+				ProfileName: "topology-aware-scheduler",
 				Cache: &ConfigCacheParams{
 					ResyncPeriodSeconds: newInt64(5),
 				},
 			},
-			expectedError: false,
+			expectedFound: true,
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			params, err := DecodeSchedulerConfigFromData(tc.data, tc.schedulerName)
-			if (err != nil) != tc.expectedError {
-				t.Fatalf("unexpected error [%v] expected=%v", err, tc.expectedError)
+			allParams, err := DecodeSchedulerProfilesFromData(tc.data)
+			if err != nil {
+				t.Fatalf("unexpected error [%v]", err)
 			}
-			if !reflect.DeepEqual(params, tc.expectedParams) {
+			if !tc.expectedFound {
+				return // nothing else to do
+			}
+
+			if len(allParams) != 1 {
+				t.Fatalf("unexpected params: found %d", len(allParams))
+			}
+			params := FindSchedulerProfileByName(allParams, tc.schedulerName)
+			if params == nil {
+				t.Fatalf("cannot find params for %q", tc.schedulerName)
+			}
+
+			if !reflect.DeepEqual(params, &tc.expectedParams) {
 				t.Fatalf("params got %q expected %q", toJSON(params), toJSON(tc.expectedParams))
 			}
 		})

pkg/objectupdate/sched/render.go

@@ -84,6 +84,11 @@ func RenderConfig(data []byte, schedulerName string, params *manifests.ConfigPar
 			continue
 		}
 
+		if params.ProfileName != "" {
+			unstructured.SetNestedField(profile, params.ProfileName, "schedulerName")
+			updated = true
+		}
+
 		pluginConfigs, ok, err := unstructured.NestedSlice(profile, "pluginConfig")
 		if !ok || err != nil {
 			klog.ErrorS(err, "failed to process unstructured data", "pluginConfig", ok)
@@ -110,11 +115,14 @@ func RenderConfig(data []byte, schedulerName string, params *manifests.ConfigPar
 				return data, false, err
 			}
 
-			updated, err = updateArgs(args, params)
+			argsUpdated, err := updateArgs(args, params)
 			if err != nil {
 				klog.ErrorS(err, "failed to update unstructured data", "args", args, "params", params)
 				return data, false, err
 			}
+			if argsUpdated {
+				updated = true
+			}
 
 			if err := unstructured.SetNestedMap(pluginConf, args, "args"); err != nil {
 				klog.ErrorS(err, "failed to override unstructured data", "data", "args")

pkg/objectupdate/sched/render_test.go

@@ -139,6 +139,80 @@ profiles:
 			initial:       configTemplateAllValues,
 			expected:      configTemplateAllValues,
 		},
+		{
+			name: "rename scheduler schedulerName multi",
+			params: &manifests.ConfigParams{
+				ProfileName: "renamed-sched",
+			},
+			initial: configTemplateAllValuesMulti,
+			expected: `apiVersion: kubescheduler.config.k8s.io/v1beta2
+kind: KubeSchedulerConfiguration
+leaderElection:
+  leaderElect: false
+profiles:
+- plugins:
+    filter:
+      disabled:
+      - name: '*'
+      enabled:
+      - name: NodeResourceFit
+  schedulerName: onlyResourceFit
+- pluginConfig:
+  - args:
+      cacheResyncPeriodSeconds: 5
+    name: NodeResourceTopologyMatch
+  plugins:
+    filter:
+      enabled:
+      - name: NodeResourceTopologyMatch
+    reserve:
+      enabled:
+      - name: NodeResourceTopologyMatch
+    score:
+      enabled:
+      - name: NodeResourceTopologyMatch
+  schedulerName: renamed-sched
+`,
+			expectedUpdate: true,
+		},
+		{
+			name: "resync updated from empty multi",
+			params: &manifests.ConfigParams{
+				Cache: &manifests.ConfigCacheParams{
+					ResyncPeriodSeconds: newInt64(42),
+				},
+			},
+			initial: configTemplateAllValuesMulti,
+			expected: `apiVersion: kubescheduler.config.k8s.io/v1beta2
+kind: KubeSchedulerConfiguration
+leaderElection:
+  leaderElect: false
+profiles:
+- plugins:
+    filter:
+      disabled:
+      - name: '*'
+      enabled:
+      - name: NodeResourceFit
+  schedulerName: onlyResourceFit
+- pluginConfig:
+  - args:
+      cacheResyncPeriodSeconds: 42
+    name: NodeResourceTopologyMatch
+  plugins:
+    filter:
+      enabled:
+      - name: NodeResourceTopologyMatch
+    reserve:
+      enabled:
+      - name: NodeResourceTopologyMatch
+    score:
+      enabled:
+      - name: NodeResourceTopologyMatch
+  schedulerName: test-sched-name
+`,
+			expectedUpdate: true,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -208,6 +282,64 @@ profiles:
   schedulerName: test-sched-name
 `
 
+var configTemplateAllValuesMulti string = `apiVersion: kubescheduler.config.k8s.io/v1beta2
+kind: KubeSchedulerConfiguration
+leaderElection:
+  leaderElect: false
+profiles:
+- plugins:
+    filter:
+      disabled:
+      - name: '*'
+      enabled:
+      - name: NodeResourceFit
+  schedulerName: onlyResourceFit
+- pluginConfig:
+  - args:
+      cacheResyncPeriodSeconds: 5
+    name: NodeResourceTopologyMatch
+  plugins:
+    filter:
+      enabled:
+      - name: NodeResourceTopologyMatch
+    reserve:
+      enabled:
+      - name: NodeResourceTopologyMatch
+    score:
+      enabled:
+      - name: NodeResourceTopologyMatch
+  schedulerName: test-sched-name
+`
+
+var configTemplateAllValuesMultiRenamed string = `apiVersion: kubescheduler.config.k8s.io/v1beta2
+kind: KubeSchedulerConfiguration
+leaderElection:
+  leaderElect: false
+profiles:
+- plugins:
+    filter:
+      disabled:
+      - name: '*'
+      enabled:
+      - name: NodeResourceFit
+  schedulerName: onlyResourceFit
+- pluginConfig:
+  - args:
+      cacheResyncPeriodSeconds: 5
+    name: NodeResourceTopologyMatch
+  plugins:
+    filter:
+      enabled:
+      - name: NodeResourceTopologyMatch
+    reserve:
+      enabled:
+      - name: NodeResourceTopologyMatch
+    score:
+      enabled:
+      - name: NodeResourceTopologyMatch
+  schedulerName: renamed-sched
+`
+
 func newInt64(value int64) *int64 {
 	return &value
 }

test/e2e/positive.go

@@ -573,7 +573,10 @@ func expectSchedulerRunning() {
 	data, ok := confMap.Data[manifests.SchedulerConfigFileName]
 	gomega.ExpectWithOffset(1, ok).To(gomega.BeTrue(), "empty config data for %q", manifests.SchedulerConfigFileName)
 
-	params, err := manifests.DecodeSchedulerConfigFromData([]byte(data), "topology-aware-scheduler") // TODO: duplicate from YAML
+	allParams, err := manifests.DecodeSchedulerProfilesFromData([]byte(data))
+	gomega.ExpectWithOffset(1, len(allParams)).To(gomega.Equal(1), "unexpected params: %#v", allParams)
+
+	params := allParams[0] // TODO: smarter find
 	gomega.ExpectWithOffset(1, err).ToNot(gomega.HaveOccurred())
 	gomega.ExpectWithOffset(1, params.Cache).ToNot(gomega.BeNil(), "no data for scheduler cache config")
 	gomega.ExpectWithOffset(1, params.Cache.ResyncPeriodSeconds).ToNot(gomega.BeNil(), "no data for scheduler cache resync period")