diff --git a/pkg/manifests/schedparams.go b/pkg/manifests/schedparams.go index 0359aa4f..59488f1f 100644 --- a/pkg/manifests/schedparams.go +++ b/pkg/manifests/schedparams.go @@ -30,8 +30,65 @@ const ( SchedulerPluginName = "NodeResourceTopologyMatch" ) +const ( + ForeignPodsDetectNone = "None" + ForeignPodsDetectAll = "All" + ForeignPodsDetectOnlyExclusiveResources = "OnlyExclusiveResources" +) + +const ( + CacheResyncAutodetect = "Autodetect" + CacheResyncAll = "All" + CacheResyncOnlyExclusiveResources = "OnlyExclusiveResources" +) + +const ( + CacheInformerShared = "Shared" + CacheInformerDedicated = "Dedicated" +) + +func ValidateForeignPodsDetectMode(value string) error { + switch value { + case ForeignPodsDetectNone: + return nil + case ForeignPodsDetectAll: + return nil + case ForeignPodsDetectOnlyExclusiveResources: + return nil + default: + return fmt.Errorf("unsupported foreignPodsDetectMode: %v", value) + } +} + +func ValidateCacheResyncMethod(value string) error { + switch value { + case CacheResyncAutodetect: + return nil + case CacheResyncAll: + return nil + case CacheResyncOnlyExclusiveResources: + return nil + default: + return fmt.Errorf("unsupported cacheResyncMethod: %v", value) + } +} + +func ValidateCacheInformerMode(value string) error { + switch value { + case CacheInformerShared: + return nil + case CacheInformerDedicated: + return nil + default: + return fmt.Errorf("unsupported cacheInformerMode: %v", value) + } +} + type ConfigCacheParams struct { - ResyncPeriodSeconds *int64 + ResyncPeriodSeconds *int64 + ResyncMethod *string + ForeignPodsDetectMode *string + InformerMode *string } type ConfigParams struct { @@ -122,15 +179,51 @@ func extractParams(profileName string, args map[string]interface{}) (ConfigParam } // json quirk: we know it's int64, yet it's detected as float64 resyncPeriod, ok, err := unstructured.NestedFloat64(args, "cacheResyncPeriodSeconds") - if !ok { - // nothing to do - return params, nil - } if err != nil { return params, fmt.Errorf("cannot process field cacheResyncPeriodSeconds: %w", err) } + if ok { + val := int64(resyncPeriod) + params.Cache.ResyncPeriodSeconds = &val + } + + cacheArgs, ok, err := unstructured.NestedMap(args, "cache") + if err != nil { + return params, fmt.Errorf("cannot process field cache: %w", err) + } + if ok { + resyncMethod, cacheOk, err := unstructured.NestedString(cacheArgs, "resyncMethod") + if err != nil { + return params, fmt.Errorf("cannot process field cache.resyncMethod: %w", err) + } + if cacheOk { + if err := ValidateCacheResyncMethod(resyncMethod); err != nil { + return params, err + } + params.Cache.ResyncMethod = &resyncMethod + } - val := int64(resyncPeriod) - params.Cache.ResyncPeriodSeconds = &val + foreignPodsDetect, cacheOk, err := unstructured.NestedString(cacheArgs, "foreignPodsDetect") + if err != nil { + return params, fmt.Errorf("cannot process field cache.foreignPodsDetect: %w", err) + } + if cacheOk { + if err := ValidateForeignPodsDetectMode(foreignPodsDetect); err != nil { + return params, err + } + params.Cache.ForeignPodsDetectMode = &foreignPodsDetect + } + + informerMode, cacheOk, err := unstructured.NestedString(cacheArgs, "informerMode") + if err != nil { + return params, fmt.Errorf("cannot process field cache.informerMode: %w", err) + } + if cacheOk { + if err := ValidateCacheInformerMode(informerMode); err != nil { + return params, err + } + params.Cache.InformerMode = &informerMode + } + } return params, nil } diff --git a/pkg/manifests/schedparams_test.go b/pkg/manifests/schedparams_test.go index d6cdd542..56e148ac 100644 --- a/pkg/manifests/schedparams_test.go +++ b/pkg/manifests/schedparams_test.go @@ -149,6 +149,146 @@ profiles: }, expectedFound: true, }, + { + name: "nonzero resync period and all cache params", + data: []byte(`apiVersion: kubescheduler.config.k8s.io/v1beta3 +kind: KubeSchedulerConfiguration +leaderElection: + leaderElect: false +profiles: +- pluginConfig: + - args: + cache: + foreignPodsDetect: None + resyncMethod: Autodetect + informerMode: Dedicated + cacheResyncPeriodSeconds: 5 + name: NodeResourceTopologyMatch + plugins: + filter: + enabled: + - name: NodeResourceTopologyMatch + reserve: + enabled: + - name: NodeResourceTopologyMatch + score: + enabled: + - name: NodeResourceTopologyMatch + schedulerName: topology-aware-scheduler +`), + schedulerName: "topology-aware-scheduler", + expectedParams: ConfigParams{ + ProfileName: "topology-aware-scheduler", + Cache: &ConfigCacheParams{ + ResyncPeriodSeconds: newInt64(5), + ResyncMethod: newString("Autodetect"), + ForeignPodsDetectMode: newString("None"), + InformerMode: newString("Dedicated"), + }, + }, + expectedFound: true, + }, + { + name: "nonzero resync period and some cache params", + data: []byte(`apiVersion: kubescheduler.config.k8s.io/v1beta3 +kind: KubeSchedulerConfiguration +leaderElection: + leaderElect: false +profiles: +- pluginConfig: + - args: + cache: + resyncMethod: OnlyExclusiveResources + cacheResyncPeriodSeconds: 5 + name: NodeResourceTopologyMatch + plugins: + filter: + enabled: + - name: NodeResourceTopologyMatch + reserve: + enabled: + - name: NodeResourceTopologyMatch + score: + enabled: + - name: NodeResourceTopologyMatch + schedulerName: topology-aware-scheduler +`), + schedulerName: "topology-aware-scheduler", + expectedParams: ConfigParams{ + ProfileName: "topology-aware-scheduler", + Cache: &ConfigCacheParams{ + ResyncPeriodSeconds: newInt64(5), + ResyncMethod: newString("OnlyExclusiveResources"), + }, + }, + expectedFound: true, + }, + { + name: "zero resync period and some cache params - 2", + data: []byte(`apiVersion: kubescheduler.config.k8s.io/v1beta3 +kind: KubeSchedulerConfiguration +leaderElection: + leaderElect: false +profiles: +- pluginConfig: + - args: + cache: + foreignPodsDetect: OnlyExclusiveResources + name: NodeResourceTopologyMatch + plugins: + filter: + enabled: + - name: NodeResourceTopologyMatch + reserve: + enabled: + - name: NodeResourceTopologyMatch + score: + enabled: + - name: NodeResourceTopologyMatch + schedulerName: topology-aware-scheduler +`), + schedulerName: "topology-aware-scheduler", + expectedParams: ConfigParams{ + ProfileName: "topology-aware-scheduler", + Cache: &ConfigCacheParams{ + ForeignPodsDetectMode: newString("OnlyExclusiveResources"), + }, + }, + expectedFound: true, + }, + { + name: "zero resync period and some cache params - 3", + data: []byte(`apiVersion: kubescheduler.config.k8s.io/v1beta3 +kind: KubeSchedulerConfiguration +leaderElection: + leaderElect: false +profiles: +- pluginConfig: + - args: + cache: + informerMode: Shared + name: NodeResourceTopologyMatch + plugins: + filter: + enabled: + - name: NodeResourceTopologyMatch + reserve: + enabled: + - name: NodeResourceTopologyMatch + score: + enabled: + - name: NodeResourceTopologyMatch + schedulerName: topology-aware-scheduler +`), + schedulerName: "topology-aware-scheduler", + expectedParams: ConfigParams{ + ProfileName: "topology-aware-scheduler", + Cache: &ConfigCacheParams{ + InformerMode: newString("Shared"), + }, + }, + expectedFound: true, + }, } for _, tc := range testCases { @@ -187,3 +327,7 @@ func toJSON(v any) string { func newInt64(value int64) *int64 { return &value } + +func newString(value string) *string { + return &value +} diff --git a/pkg/objectupdate/sched/render.go b/pkg/objectupdate/sched/render.go index 419924f9..86e316da 100644 --- a/pkg/objectupdate/sched/render.go +++ b/pkg/objectupdate/sched/render.go @@ -167,9 +167,76 @@ func updateArgs(args map[string]interface{}, params *manifests.ConfigParams) (bo updated++ } } + + cacheArgs, ok, err := unstructured.NestedMap(args, "cache") + if !ok { + cacheArgs = make(map[string]interface{}) + } + if err != nil { + return updated > 0, err + } + + var cacheArgsUpdated int + if params.Cache != nil { + cacheArgsUpdated, err = updateCacheArgs(cacheArgs, params) + if err != nil { + return updated > 0, err + } + } + updated += cacheArgsUpdated + + if cacheArgsUpdated > 0 { + if err := unstructured.SetNestedMap(args, cacheArgs, "cache"); err != nil { + return updated > 0, err + } + } return updated > 0, ensureBackwardCompatibility(args) } +func updateCacheArgs(args map[string]interface{}, params *manifests.ConfigParams) (int, error) { + var updated int + var err error + + if params.Cache.ResyncMethod != nil { + resyncMethod := *params.Cache.ResyncMethod // shortcut + err = manifests.ValidateCacheResyncMethod(resyncMethod) + if err != nil { + return updated, err + } + err = unstructured.SetNestedField(args, resyncMethod, "resyncMethod") + if err != nil { + return updated, err + } + updated++ + } + if params.Cache.ForeignPodsDetectMode != nil { + foreignPodsMode := *params.Cache.ForeignPodsDetectMode // shortcut + err = manifests.ValidateForeignPodsDetectMode(foreignPodsMode) + if err != nil { + return updated, err + } + err = unstructured.SetNestedField(args, foreignPodsMode, "foreignPodsDetect") + if err != nil { + return updated, err + } + updated++ + } + if params.Cache.InformerMode != nil { + informerMode := *params.Cache.InformerMode // shortcut + err = manifests.ValidateCacheInformerMode(informerMode) + if err != nil { + return updated, err + } + err = unstructured.SetNestedField(args, informerMode, "informerMode") + if err != nil { + return updated, err + } + updated++ + } + + return updated, nil +} + func ensureBackwardCompatibility(args map[string]interface{}) error { resyncPeriod, ok, err := unstructured.NestedInt64(args, "cacheResyncPeriodSeconds") if !ok { diff --git a/pkg/objectupdate/sched/render_test.go b/pkg/objectupdate/sched/render_test.go index d68b6b87..25c5108d 100644 --- a/pkg/objectupdate/sched/render_test.go +++ b/pkg/objectupdate/sched/render_test.go @@ -19,6 +19,7 @@ package sched import ( "testing" + "github.com/google/go-cmp/cmp" "github.com/k8stopologyawareschedwg/deployer/pkg/manifests" ) @@ -210,6 +211,187 @@ profiles: enabled: - name: NodeResourceTopologyMatch schedulerName: test-sched-name +`, + expectedUpdate: true, + }, + { + name: "all params updated from empty", + params: &manifests.ConfigParams{ + Cache: &manifests.ConfigCacheParams{ + ResyncPeriodSeconds: newInt64(42), + ResyncMethod: newString("OnlyExclusiveResources"), + ForeignPodsDetectMode: newString("OnlyExclusiveResources"), + InformerMode: newString("Dedicated"), + }, + }, + initial: configTemplateEmpty, + expected: `apiVersion: kubescheduler.config.k8s.io/v1beta3 +kind: KubeSchedulerConfiguration +leaderElection: + leaderElect: false +profiles: +- pluginConfig: + - args: + cache: + foreignPodsDetect: OnlyExclusiveResources + informerMode: Dedicated + resyncMethod: OnlyExclusiveResources + cacheResyncPeriodSeconds: 42 + name: NodeResourceTopologyMatch + plugins: + filter: + enabled: + - name: NodeResourceTopologyMatch + reserve: + enabled: + - name: NodeResourceTopologyMatch + score: + enabled: + - name: NodeResourceTopologyMatch + schedulerName: test-sched-name +`, + expectedUpdate: true, + }, + { + name: "cache params updated from empty", + params: &manifests.ConfigParams{ + Cache: &manifests.ConfigCacheParams{ + ResyncPeriodSeconds: newInt64(11), + ResyncMethod: newString("OnlyExclusiveResources"), + ForeignPodsDetectMode: newString("OnlyExclusiveResources"), + }, + }, + initial: configTemplateAllValues, + expected: `apiVersion: kubescheduler.config.k8s.io/v1beta3 +kind: KubeSchedulerConfiguration +leaderElection: + leaderElect: false +profiles: +- pluginConfig: + - args: + cache: + foreignPodsDetect: OnlyExclusiveResources + resyncMethod: OnlyExclusiveResources + cacheResyncPeriodSeconds: 11 + name: NodeResourceTopologyMatch + plugins: + filter: + enabled: + - name: NodeResourceTopologyMatch + reserve: + enabled: + - name: NodeResourceTopologyMatch + score: + enabled: + - name: NodeResourceTopologyMatch + schedulerName: test-sched-name +`, + expectedUpdate: true, + }, + { + name: "all params updated from nonempty", + params: &manifests.ConfigParams{ + Cache: &manifests.ConfigCacheParams{ + ResyncPeriodSeconds: newInt64(7), + ResyncMethod: newString("Autodetect"), + ForeignPodsDetectMode: newString("None"), + InformerMode: newString("Shared"), + }, + }, + initial: configTemplateAllValuesFineTuned, + expected: `apiVersion: kubescheduler.config.k8s.io/v1beta3 +kind: KubeSchedulerConfiguration +leaderElection: + leaderElect: false +profiles: +- pluginConfig: + - args: + cache: + foreignPodsDetect: None + informerMode: Shared + resyncMethod: Autodetect + cacheResyncPeriodSeconds: 7 + name: NodeResourceTopologyMatch + plugins: + filter: + enabled: + - name: NodeResourceTopologyMatch + reserve: + enabled: + - name: NodeResourceTopologyMatch + score: + enabled: + - name: NodeResourceTopologyMatch + schedulerName: test-sched-name +`, + expectedUpdate: true, + }, + { + name: "partial cache params updated from empty", + params: &manifests.ConfigParams{ + Cache: &manifests.ConfigCacheParams{ + ResyncPeriodSeconds: newInt64(42), + ForeignPodsDetectMode: newString("None"), + }, + }, + initial: configTemplateEmpty, + expected: `apiVersion: kubescheduler.config.k8s.io/v1beta3 +kind: KubeSchedulerConfiguration +leaderElection: + leaderElect: false +profiles: +- pluginConfig: + - args: + cache: + foreignPodsDetect: None + cacheResyncPeriodSeconds: 42 + name: NodeResourceTopologyMatch + plugins: + filter: + enabled: + - name: NodeResourceTopologyMatch + reserve: + enabled: + - name: NodeResourceTopologyMatch + score: + enabled: + - name: NodeResourceTopologyMatch + schedulerName: test-sched-name +`, + expectedUpdate: true, + }, + { + name: "partial params updated from nonempty", + params: &manifests.ConfigParams{ + Cache: &manifests.ConfigCacheParams{ + ForeignPodsDetectMode: newString("All"), + }, + }, + initial: configTemplateAllValuesFineTuned, + expected: `apiVersion: kubescheduler.config.k8s.io/v1beta3 +kind: KubeSchedulerConfiguration +leaderElection: + leaderElect: false +profiles: +- pluginConfig: + - args: + cache: + foreignPodsDetect: All + informerMode: Dedicated + resyncMethod: OnlyExclusiveResources + cacheResyncPeriodSeconds: 5 + name: NodeResourceTopologyMatch + plugins: + filter: + enabled: + - name: NodeResourceTopologyMatch + reserve: + enabled: + - name: NodeResourceTopologyMatch + score: + enabled: + - name: NodeResourceTopologyMatch + schedulerName: test-sched-name `, expectedUpdate: true, }, @@ -233,7 +415,7 @@ profiles: rendered := string(data) if rendered != tc.expected { - t.Errorf("rendering failed.\nrendered=[%s]\nexpected=[%s]\n", rendered, tc.expected) + t.Errorf("rendering failed.\nrendered=[%s]\nexpected=[%s]\ndiff=[%s]\n", rendered, tc.expected, cmp.Diff(rendered, tc.expected)) } }) } @@ -282,6 +464,32 @@ profiles: schedulerName: test-sched-name ` +var configTemplateAllValuesFineTuned string = `apiVersion: kubescheduler.config.k8s.io/v1beta3 +kind: KubeSchedulerConfiguration +leaderElection: + leaderElect: false +profiles: +- pluginConfig: + - args: + cache: + foreignPodsDetect: OnlyExclusiveResources + informerMode: Dedicated + resyncMethod: OnlyExclusiveResources + cacheResyncPeriodSeconds: 5 + name: NodeResourceTopologyMatch + plugins: + filter: + enabled: + - name: NodeResourceTopologyMatch + reserve: + enabled: + - name: NodeResourceTopologyMatch + score: + enabled: + - name: NodeResourceTopologyMatch + schedulerName: test-sched-name +` + var configTemplateAllValuesMulti string = `apiVersion: kubescheduler.config.k8s.io/v1beta3 kind: KubeSchedulerConfiguration leaderElection: @@ -343,3 +551,7 @@ profiles: func newInt64(value int64) *int64 { return &value } + +func newString(value string) *string { + return &value +} diff --git a/test/e2e/manifests.go b/test/e2e/manifests.go index 6857f773..47ef0230 100644 --- a/test/e2e/manifests.go +++ b/test/e2e/manifests.go @@ -19,6 +19,7 @@ package e2e import ( "context" "fmt" + "time" "github.com/go-logr/logr" @@ -139,7 +140,7 @@ var _ = ginkgo.Describe("[ManifestFlow] Deployer rendering", func() { gomega.Expect(err).ToNot(gomega.HaveOccurred()) ginkgo.By("checking the pod goes running") - e2epods.WaitForPodToBeRunning(cli, testPod.Namespace, testPod.Name) + e2epods.WaitForPodToBeRunning(cli, testPod.Namespace, testPod.Name, 2*time.Minute) }) }) }) diff --git a/test/e2e/positive.go b/test/e2e/positive.go index 539b349a..9cf812ae 100644 --- a/test/e2e/positive.go +++ b/test/e2e/positive.go @@ -331,7 +331,7 @@ var _ = ginkgo.Describe("[PositiveFlow] Deployer execution", func() { gomega.Expect(err).ToNot(gomega.HaveOccurred()) ginkgo.By("checking the pod goes running") - e2epods.WaitForPodToBeRunning(cli, testPod.Namespace, testPod.Name) + e2epods.WaitForPodToBeRunning(cli, testPod.Namespace, testPod.Name, 2*time.Minute) }) }) @@ -426,9 +426,8 @@ var _ = ginkgo.Describe("[PositiveFlow] Deployer execution", func() { gomega.Expect(err).ToNot(gomega.HaveOccurred()) ginkgo.By("checking the pod goes running") - e2epods.WaitForPodToBeRunning(cli, testPod.Namespace, testPod.Name) + e2epods.WaitForPodToBeRunning(cli, testPod.Namespace, testPod.Name, 2*time.Minute) }) - }) }) }) diff --git a/test/e2e/utils/pods/pods.go b/test/e2e/utils/pods/pods.go index 3480dc7f..4c4594c6 100644 --- a/test/e2e/utils/pods/pods.go +++ b/test/e2e/utils/pods/pods.go @@ -18,6 +18,7 @@ package pods import ( "context" + "errors" "fmt" "regexp" "time" @@ -68,9 +69,10 @@ func GuaranteedSleeperPod(namespace, schedulerName string) *corev1.Pod { } } -func WaitForPodToBeRunning(cli *kubernetes.Clientset, podNamespace, podName string) *corev1.Pod { +func WaitForPodToBeRunning(cli *kubernetes.Clientset, podNamespace, podName string, timeout time.Duration) *corev1.Pod { var err error var pod *corev1.Pod + startTime := time.Now() gomega.EventuallyWithOffset(1, func() error { pod, err = cli.CoreV1().Pods(podNamespace).Get(context.TODO(), podName, metav1.GetOptions{}) if err != nil { @@ -80,11 +82,13 @@ func WaitForPodToBeRunning(cli *kubernetes.Clientset, podNamespace, podName stri case corev1.PodFailed, corev1.PodSucceeded: return fmt.Errorf("pod %q status %q which is unexpected", podName, pod.Status.Phase) case corev1.PodRunning: - fmt.Fprintf(ginkgo.GinkgoWriter, "Pod %q is running!\n", podName) + fmt.Fprintf(ginkgo.GinkgoWriter, "Pod %q is running! (took %v)\n", podName, time.Since(startTime)) return nil } - return fmt.Errorf("pod %q status %q, waiting for it to be Running (with Ready = true)", podName, pod.Status.Phase) - }, 1*time.Minute, 15*time.Second).ShouldNot(gomega.HaveOccurred()) + msg := fmt.Sprintf("pod %q status %q, waiting for it to be Running (with Ready = true)", podName, pod.Status.Phase) + fmt.Fprintln(ginkgo.GinkgoWriter, msg) + return errors.New(msg) + }, timeout, 10*time.Second).ShouldNot(gomega.HaveOccurred()) return pod } @@ -92,6 +96,7 @@ func WaitPodsToBeRunningByRegex(pattern string) { cs, err := clientutil.New() gomega.Expect(err).ToNot(gomega.HaveOccurred()) + startTime := time.Now() gomega.EventuallyWithOffset(1, func() error { pods, err := GetByRegex(cs, fmt.Sprintf("%s-*", pattern)) if err != nil { @@ -103,11 +108,14 @@ func WaitPodsToBeRunningByRegex(pattern string) { for _, pod := range pods { if pod.Status.Phase != corev1.PodRunning { - return fmt.Errorf("pod %q is not in %v state", pod.Name, corev1.PodRunning) + msg := fmt.Sprintf("pod %q is not in %v state", pod.Name, corev1.PodRunning) + fmt.Println(ginkgo.GinkgoWriter, msg) + return errors.New(msg) } } + fmt.Fprintf(ginkgo.GinkgoWriter, "all pods running! (took %v)\n", time.Since(startTime)) return nil - }, 1*time.Minute, 15*time.Second).ShouldNot(gomega.HaveOccurred()) + }, 1*time.Minute, 10*time.Second).ShouldNot(gomega.HaveOccurred()) } func GetByRegex(cs client.Client, reg string) ([]*corev1.Pod, error) {