From b7913300db413d8f0adcf62521162555bc3724ea Mon Sep 17 00:00:00 2001 From: zwzhang Date: Wed, 16 Aug 2023 10:50:36 +0800 Subject: [PATCH] koord-manager: fix node slo extension got overwriten (#1552) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: 佑祎 --- .../nodeslo/extender_config_test.go | 2 +- pkg/slo-controller/nodeslo/extender_plugin.go | 19 ++++++++++--------- .../nodeslo/extender_plugin_test.go | 16 +++++++++++++++- .../nodeslo/nodeslo_controller.go | 2 +- .../nodeslo/nodeslo_controller_test.go | 2 +- 5 files changed, 28 insertions(+), 13 deletions(-) diff --git a/pkg/slo-controller/nodeslo/extender_config_test.go b/pkg/slo-controller/nodeslo/extender_config_test.go index 004341ed5..b16856f24 100644 --- a/pkg/slo-controller/nodeslo/extender_config_test.go +++ b/pkg/slo-controller/nodeslo/extender_config_test.go @@ -29,7 +29,7 @@ import ( func getDefaultExtensionStrategy() *slov1alpha1.ExtensionsMap { defaultCfg := getDefaultExtensionCfg() - defaultStrategy := slov1alpha1.ExtensionsMap{} + defaultStrategy := slov1alpha1.ExtensionsMap{Object: map[string]interface{}{}} if defaultCfg == nil || defaultCfg.Object == nil { return &defaultStrategy } diff --git a/pkg/slo-controller/nodeslo/extender_plugin.go b/pkg/slo-controller/nodeslo/extender_plugin.go index a9c1ab84d..ed0cba075 100644 --- a/pkg/slo-controller/nodeslo/extender_plugin.go +++ b/pkg/slo-controller/nodeslo/extender_plugin.go @@ -62,10 +62,13 @@ func calculateExtensionsCfgMerged(oldCfgMap configuration.ExtensionCfgMap, confi return mergedCfgMap } -func getExtensionsConfigSpec(node *corev1.Node, cfgMap *configuration.ExtensionCfgMap) *slov1alpha1.ExtensionsMap { - extMap := slov1alpha1.ExtensionsMap{} +func getExtensionsConfigSpec(node *corev1.Node, oldSpec *slov1alpha1.NodeSLOSpec, cfgMap *configuration.ExtensionCfgMap) *slov1alpha1.ExtensionsMap { + extMap := &slov1alpha1.ExtensionsMap{Object: map[string]interface{}{}} + if oldSpec != nil && oldSpec.Extensions != nil && oldSpec.Extensions.Object != nil { + extMap = oldSpec.Extensions.DeepCopy() + } if cfgMap == nil || cfgMap.Object == nil { - return &extMap + return extMap } for name, extender := range globalNodeSLOMergedExtender { extKey, extStrategy, err := extender.GetNodeSLOExtension(node, cfgMap) @@ -75,16 +78,14 @@ func getExtensionsConfigSpec(node *corev1.Node, cfgMap *configuration.ExtensionC continue } if extStrategy == nil { - continue - } - if extMap.Object == nil { - extMap.Object = make(map[string]interface{}) + delete(extMap.Object, extKey) + } else { + extMap.Object[extKey] = extStrategy } - extMap.Object[extKey] = extStrategy metrics.RecordNodeSLOSpecParseCount(true, "getNodeSLOExtension") klog.V(5).Infof("run get nodeSLO extender %v success, extMap %v", name, extMap) } - return &extMap + return extMap } func UnregisterNodeSLOMergedExtender(name string) { diff --git a/pkg/slo-controller/nodeslo/extender_plugin_test.go b/pkg/slo-controller/nodeslo/extender_plugin_test.go index 0e92766bb..5edd162e6 100644 --- a/pkg/slo-controller/nodeslo/extender_plugin_test.go +++ b/pkg/slo-controller/nodeslo/extender_plugin_test.go @@ -25,6 +25,7 @@ import ( "k8s.io/client-go/tools/record" "github.com/koordinator-sh/koordinator/apis/configuration" + slov1alpha1 "github.com/koordinator-sh/koordinator/apis/slo/v1alpha1" ) const ( @@ -62,6 +63,15 @@ func Test_NodeMergedExtender(t *testing.T) { testExtKey: testExtIF, }, } + oldOtherExtKey := "old-other-ext-key" + oldOtherExtVal := "old-other-ext-val" + oldSpec := &slov1alpha1.NodeSLOSpec{ + Extensions: &slov1alpha1.ExtensionsMap{ + Object: map[string]interface{}{ + oldOtherExtKey: oldOtherExtVal, + }, + }, + } node := &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{}, @@ -69,11 +79,15 @@ func Test_NodeMergedExtender(t *testing.T) { } cfgMap := configuration.ExtensionCfgMap{} newCfg := calculateExtensionsCfgMerged(cfgMap, configMap, &record.FakeRecorder{}) - extMap := getExtensionsConfigSpec(node, &newCfg) + extMap := getExtensionsConfigSpec(node, oldSpec, &newCfg) gotIf := extMap.Object[testExtKey].(string) if gotIf != testExtIF { t.Errorf("run NodeMergedExtender got ext key %s, want %s", gotIf, testExtIF) } + gotOtherIf := extMap.Object[oldOtherExtKey].(string) + if gotOtherIf != oldOtherExtVal { + t.Errorf("run NodeMergedExtender got other ext key %s, want %s", gotOtherIf, oldOtherExtVal) + } UnregisterNodeSLOMergedExtender(pluginName) }) } diff --git a/pkg/slo-controller/nodeslo/nodeslo_controller.go b/pkg/slo-controller/nodeslo/nodeslo_controller.go index 1ef58f26b..3f8183bfc 100644 --- a/pkg/slo-controller/nodeslo/nodeslo_controller.go +++ b/pkg/slo-controller/nodeslo/nodeslo_controller.go @@ -107,7 +107,7 @@ func (r *NodeSLOReconciler) getNodeSLOSpec(node *corev1.Node, oldSpec *slov1alph metrics.RecordNodeSLOSpecParseCount(true, "getSystemConfigSpec") } - nodeSLOSpec.Extensions = getExtensionsConfigSpec(node, &sloCfg.ExtensionCfgMerged) + nodeSLOSpec.Extensions = getExtensionsConfigSpec(node, oldSpec, &sloCfg.ExtensionCfgMerged) return nodeSLOSpec, nil } diff --git a/pkg/slo-controller/nodeslo/nodeslo_controller_test.go b/pkg/slo-controller/nodeslo/nodeslo_controller_test.go index 1d5b2ca56..6f5402a2e 100644 --- a/pkg/slo-controller/nodeslo/nodeslo_controller_test.go +++ b/pkg/slo-controller/nodeslo/nodeslo_controller_test.go @@ -43,7 +43,7 @@ import ( // plugins in koordlet/resmanager will get the interface and // change it to strategy individually func getExtensionsIfMap(in slov1alpha1.ExtensionsMap) (*slov1alpha1.ExtensionsMap, error) { - extensionsMap := &slov1alpha1.ExtensionsMap{} + extensionsMap := &slov1alpha1.ExtensionsMap{Object: map[string]interface{}{}} for extkey, extIf := range in.Object { //marshal unmarshal to extStr, err := json.Marshal(extIf)