From f2a3752d53a0c2f1299fec00b2aaee8719a7f79d Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Tue, 28 Nov 2023 08:40:27 +0100 Subject: [PATCH 1/3] e2e: relax running timeout The test ``` [PositiveFlow] Deployer execution with a running cluster without any components when deployed with resource-topology-exporter as the updater [It] should verify a test pod scheduled with the topology aware scheduler goes running ``` started to flake lately with the symptoms matching a timeout due to slow CI. Relax the timeout and add more logs to (try to) deflake the issue. Signed-off-by: Francesco Romani --- test/e2e/manifests.go | 3 ++- test/e2e/positive.go | 5 ++--- test/e2e/utils/pods/pods.go | 20 ++++++++++++++------ 3 files changed, 18 insertions(+), 10 deletions(-) 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) { From 9e5ce1a303021fda1b18639c1fc4c7d295758532 Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Mon, 27 Nov 2023 17:01:11 +0100 Subject: [PATCH 2/3] manifests: objectupdate: cache fine-tuning support Add support to gather and set cache fine-tuning parameters: ForeignPodsDetectMode and CacheResyncMethod, both exposed in the `cache` subfield of the plugin parameters (at least cacheResyncPeriodSeconds also belong there, but this is another story for another day). Also add validation support for the parameters. Signed-off-by: Francesco Romani --- pkg/manifests/schedparams.go | 79 +++++++++- pkg/manifests/schedparams_test.go | 109 ++++++++++++++ pkg/objectupdate/sched/render.go | 54 +++++++ pkg/objectupdate/sched/render_test.go | 205 ++++++++++++++++++++++++++ 4 files changed, 440 insertions(+), 7 deletions(-) diff --git a/pkg/manifests/schedparams.go b/pkg/manifests/schedparams.go index 0359aa4f..047c6388 100644 --- a/pkg/manifests/schedparams.go +++ b/pkg/manifests/schedparams.go @@ -30,8 +30,48 @@ const ( SchedulerPluginName = "NodeResourceTopologyMatch" ) +const ( + ForeignPodsDetectNone = "None" + ForeignPodsDetectAll = "All" + ForeignPodsDetectOnlyExclusiveResources = "OnlyExclusiveResources" +) + +const ( + CacheResyncAutodetect = "Autodetect" + CacheResyncAll = "All" + CacheResyncOnlyExclusiveResources = "OnlyExclusiveResources" +) + +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) + } +} + type ConfigCacheParams struct { - ResyncPeriodSeconds *int64 + ResyncPeriodSeconds *int64 + ResyncMethod *string + ForeignPodsDetectMode *string } type ConfigParams struct { @@ -122,15 +162,40 @@ 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 + } + } return params, nil } diff --git a/pkg/manifests/schedparams_test.go b/pkg/manifests/schedparams_test.go index d6cdd542..bc0baacd 100644 --- a/pkg/manifests/schedparams_test.go +++ b/pkg/manifests/schedparams_test.go @@ -149,6 +149,111 @@ 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 + 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"), + }, + }, + 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", + 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, + }, } for _, tc := range testCases { @@ -187,3 +292,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..390fb871 100644 --- a/pkg/objectupdate/sched/render.go +++ b/pkg/objectupdate/sched/render.go @@ -167,9 +167,63 @@ 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 + } + + 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 != nil { + 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++ + } + } + + 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..27fe663a 100644 --- a/pkg/objectupdate/sched/render_test.go +++ b/pkg/objectupdate/sched/render_test.go @@ -210,6 +210,182 @@ 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"), + }, + }, + initial: configTemplateEmpty, + expected: `apiVersion: kubescheduler.config.k8s.io/v1beta3 +kind: KubeSchedulerConfiguration +leaderElection: + leaderElect: false +profiles: +- pluginConfig: + - args: + cache: + foreignPodsDetect: OnlyExclusiveResources + 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"), + }, + }, + initial: configTemplateAllValuesFineTuned, + expected: `apiVersion: kubescheduler.config.k8s.io/v1beta3 +kind: KubeSchedulerConfiguration +leaderElection: + leaderElect: false +profiles: +- pluginConfig: + - args: + cache: + foreignPodsDetect: None + 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 + 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, }, @@ -282,6 +458,31 @@ 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 + 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 +544,7 @@ profiles: func newInt64(value int64) *int64 { return &value } + +func newString(value string) *string { + return &value +} From 24fdae7eb8e8206cd6175016ffa2f047111b1960 Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Fri, 19 Jan 2024 14:40:42 +0100 Subject: [PATCH 3/3] sched: support for informerMode option The scheduler plugin gained support for the external informer, necessary to overcome mismatch between the scheduler and the ode views wrt the pods on nodes. Expose support in the deployer libraries to change this setting. Signed-off-by: Francesco Romani --- pkg/manifests/schedparams.go | 28 ++++++++++++ pkg/manifests/schedparams_test.go | 37 ++++++++++++++- pkg/objectupdate/sched/render.go | 65 ++++++++++++++++----------- pkg/objectupdate/sched/render_test.go | 9 +++- 4 files changed, 111 insertions(+), 28 deletions(-) diff --git a/pkg/manifests/schedparams.go b/pkg/manifests/schedparams.go index 047c6388..59488f1f 100644 --- a/pkg/manifests/schedparams.go +++ b/pkg/manifests/schedparams.go @@ -42,6 +42,11 @@ const ( CacheResyncOnlyExclusiveResources = "OnlyExclusiveResources" ) +const ( + CacheInformerShared = "Shared" + CacheInformerDedicated = "Dedicated" +) + func ValidateForeignPodsDetectMode(value string) error { switch value { case ForeignPodsDetectNone: @@ -68,10 +73,22 @@ func ValidateCacheResyncMethod(value string) error { } } +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 ResyncMethod *string ForeignPodsDetectMode *string + InformerMode *string } type ConfigParams struct { @@ -196,6 +213,17 @@ func extractParams(profileName string, args map[string]interface{}) (ConfigParam } 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 bc0baacd..56e148ac 100644 --- a/pkg/manifests/schedparams_test.go +++ b/pkg/manifests/schedparams_test.go @@ -161,6 +161,7 @@ profiles: cache: foreignPodsDetect: None resyncMethod: Autodetect + informerMode: Dedicated cacheResyncPeriodSeconds: 5 name: NodeResourceTopologyMatch plugins: @@ -182,6 +183,7 @@ profiles: ResyncPeriodSeconds: newInt64(5), ResyncMethod: newString("Autodetect"), ForeignPodsDetectMode: newString("None"), + InformerMode: newString("Dedicated"), }, }, expectedFound: true, @@ -222,7 +224,7 @@ profiles: expectedFound: true, }, { - name: "zero resync period and some cache params", + name: "zero resync period and some cache params - 2", data: []byte(`apiVersion: kubescheduler.config.k8s.io/v1beta3 kind: KubeSchedulerConfiguration leaderElection: @@ -254,6 +256,39 @@ profiles: }, 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 { diff --git a/pkg/objectupdate/sched/render.go b/pkg/objectupdate/sched/render.go index 390fb871..86e316da 100644 --- a/pkg/objectupdate/sched/render.go +++ b/pkg/objectupdate/sched/render.go @@ -176,9 +176,12 @@ func updateArgs(args map[string]interface{}, params *manifests.ConfigParams) (bo return updated > 0, err } - cacheArgsUpdated, err := updateCacheArgs(cacheArgs, params) - 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 @@ -194,31 +197,41 @@ func updateCacheArgs(args map[string]interface{}, params *manifests.ConfigParams var updated int var err error - if params.Cache != nil { - 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.ResyncMethod != nil { + resyncMethod := *params.Cache.ResyncMethod // shortcut + err = manifests.ValidateCacheResyncMethod(resyncMethod) + if err != nil { + return updated, err } - 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++ + 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 diff --git a/pkg/objectupdate/sched/render_test.go b/pkg/objectupdate/sched/render_test.go index 27fe663a..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" ) @@ -220,6 +221,7 @@ profiles: ResyncPeriodSeconds: newInt64(42), ResyncMethod: newString("OnlyExclusiveResources"), ForeignPodsDetectMode: newString("OnlyExclusiveResources"), + InformerMode: newString("Dedicated"), }, }, initial: configTemplateEmpty, @@ -232,6 +234,7 @@ profiles: - args: cache: foreignPodsDetect: OnlyExclusiveResources + informerMode: Dedicated resyncMethod: OnlyExclusiveResources cacheResyncPeriodSeconds: 42 name: NodeResourceTopologyMatch @@ -292,6 +295,7 @@ profiles: ResyncPeriodSeconds: newInt64(7), ResyncMethod: newString("Autodetect"), ForeignPodsDetectMode: newString("None"), + InformerMode: newString("Shared"), }, }, initial: configTemplateAllValuesFineTuned, @@ -304,6 +308,7 @@ profiles: - args: cache: foreignPodsDetect: None + informerMode: Shared resyncMethod: Autodetect cacheResyncPeriodSeconds: 7 name: NodeResourceTopologyMatch @@ -372,6 +377,7 @@ profiles: - args: cache: foreignPodsDetect: All + informerMode: Dedicated resyncMethod: OnlyExclusiveResources cacheResyncPeriodSeconds: 5 name: NodeResourceTopologyMatch @@ -409,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)) } }) } @@ -467,6 +473,7 @@ profiles: - args: cache: foreignPodsDetect: OnlyExclusiveResources + informerMode: Dedicated resyncMethod: OnlyExclusiveResources cacheResyncPeriodSeconds: 5 name: NodeResourceTopologyMatch