From 3a749f8ad9219e7496d273d5bd48e8d2a856edfd Mon Sep 17 00:00:00 2001 From: Qing Hao Date: Fri, 18 Oct 2024 11:08:21 +0800 Subject: [PATCH] fix the mca override cma configs issues (#126) Signed-off-by: haoqing0110 --- .../addon_configuration_reconciler_test.go | 113 ++++++++++++++---- .../controllers/addonconfiguration/graph.go | 9 +- 2 files changed, 101 insertions(+), 21 deletions(-) diff --git a/pkg/addon/controllers/addonconfiguration/addon_configuration_reconciler_test.go b/pkg/addon/controllers/addonconfiguration/addon_configuration_reconciler_test.go index b7142092a..0917f35fb 100644 --- a/pkg/addon/controllers/addonconfiguration/addon_configuration_reconciler_test.go +++ b/pkg/addon/controllers/addonconfiguration/addon_configuration_reconciler_test.go @@ -161,6 +161,30 @@ func TestAddonConfigReconcile(t *testing.T) { ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, }}, nil, nil), addontesting.NewAddon("test", "cluster2"), + // cluster3 already has configs synced to status before spec hash is updated + newManagedClusterAddon("test", "cluster3", []addonv1alpha1.AddOnConfig{{ + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + }}, []addonv1alpha1.ConfigReference{ + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "", + }, + LastObservedGeneration: 0, + }, + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + SpecHash: "", + }, + LastObservedGeneration: 0, + }, + }, nil), }, placements: []runtime.Object{ &clusterv1beta1.Placement{ObjectMeta: metav1.ObjectMeta{Name: "test-placement", Namespace: "default"}}, @@ -176,7 +200,7 @@ func TestAddonConfigReconcile(t *testing.T) { }, }, Status: clusterv1beta1.PlacementDecisionStatus{ - Decisions: []clusterv1beta1.ClusterDecision{{ClusterName: "cluster1"}, {ClusterName: "cluster2"}}, + Decisions: []clusterv1beta1.ClusterDecision{{ClusterName: "cluster1"}, {ClusterName: "cluster2"}, {ClusterName: "cluster3"}}, }, }, }, @@ -187,7 +211,7 @@ func TestAddonConfigReconcile(t *testing.T) { ConfigGroupResource: v1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, DesiredConfig: &v1alpha1.ConfigSpecHash{ ConfigReferent: v1alpha1.ConfigReferent{Name: "test"}, - SpecHash: "hash", + SpecHash: "", }, }).WithPlacementStrategy(addonv1alpha1.PlacementStrategy{ PlacementRef: addonv1alpha1.PlacementRef{Name: "test-placement", Namespace: "default"}, @@ -199,32 +223,81 @@ func TestAddonConfigReconcile(t *testing.T) { ConfigGroupResource: v1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, DesiredConfig: &v1alpha1.ConfigSpecHash{ ConfigReferent: v1alpha1.ConfigReferent{Name: "test1"}, - SpecHash: "hash1", + SpecHash: "", + }, + }, + { + ConfigGroupResource: v1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + DesiredConfig: &v1alpha1.ConfigSpecHash{ + ConfigReferent: v1alpha1.ConfigReferent{Name: "test1"}, + SpecHash: "", }, }, }, }).Build(), validateAddonActions: func(t *testing.T, actions []clienttesting.Action) { - addontesting.AssertActions(t, actions, "patch", "patch") + addontesting.AssertActions(t, actions, "patch", "patch", "patch") sort.Sort(byPatchName(actions)) - expectPatchConfigurationAction(t, actions[0], []addonv1alpha1.ConfigReference{{ - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, - SpecHash: "", + expectPatchConfigurationAction(t, actions[0], []addonv1alpha1.ConfigReference{ + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "", + }, + LastObservedGeneration: 0, }, - LastObservedGeneration: 0, - }}) - expectPatchConfigurationAction(t, actions[1], []addonv1alpha1.ConfigReference{{ - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, - SpecHash: "hash1", + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + SpecHash: "", + }, + LastObservedGeneration: 0, }, - LastObservedGeneration: 0, - }}) + }) + expectPatchConfigurationAction(t, actions[1], []addonv1alpha1.ConfigReference{ + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + SpecHash: "", + }, + LastObservedGeneration: 0, + }, + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + SpecHash: "", + }, + LastObservedGeneration: 0, + }, + }) + expectPatchConfigurationAction(t, actions[2], []addonv1alpha1.ConfigReference{ + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "", + }, + LastObservedGeneration: 0, + }, + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + SpecHash: "", + }, + LastObservedGeneration: 0, + }, + }) }, }, { diff --git a/pkg/addon/controllers/addonconfiguration/graph.go b/pkg/addon/controllers/addonconfiguration/graph.go index 32700f244..e2b79f5df 100644 --- a/pkg/addon/controllers/addonconfiguration/graph.go +++ b/pkg/addon/controllers/addonconfiguration/graph.go @@ -297,6 +297,7 @@ func (n *installStrategyNode) addNode(addon *addonv1alpha1.ManagedClusterAddOn) n.children[addon.Namespace].desiredConfigs = n.children[addon.Namespace].desiredConfigs.copy() // TODO we should also filter out the configs which are not supported configs. for _, config := range addon.Spec.Configs { + // go through mca spec configs update the desired configs n.children[addon.Namespace].desiredConfigs[config.ConfigGroupResource] = addonv1alpha1.ConfigReference{ ConfigGroupResource: config.ConfigGroupResource, ConfigReferent: config.ConfigReferent, @@ -304,11 +305,17 @@ func (n *installStrategyNode) addNode(addon *addonv1alpha1.ManagedClusterAddOn) ConfigReferent: config.ConfigReferent, }, } - // copy the spechash from mca status + + // go through mca spec configs and status, and copy the specHash if they match for _, configRef := range addon.Status.ConfigReferences { if configRef.DesiredConfig == nil { continue } + // compare the ConfigGroupResource and ConfigReferent + if configRef.ConfigGroupResource != config.ConfigGroupResource || configRef.DesiredConfig.ConfigReferent != config.ConfigReferent { + continue + } + // copy the spec hash to desired configs nodeDesiredConfig, ok := n.children[addon.Namespace].desiredConfigs[configRef.ConfigGroupResource] if ok && (nodeDesiredConfig.DesiredConfig.ConfigReferent == configRef.DesiredConfig.ConfigReferent) { nodeDesiredConfig.DesiredConfig.SpecHash = configRef.DesiredConfig.SpecHash