diff --git a/pkg/addon/controllers/addonconfiguration/addon_configuration_reconciler.go b/pkg/addon/controllers/addonconfiguration/addon_configuration_reconciler.go index 25a91a68b..25ba2c2b4 100644 --- a/pkg/addon/controllers/addonconfiguration/addon_configuration_reconciler.go +++ b/pkg/addon/controllers/addonconfiguration/addon_configuration_reconciler.go @@ -46,36 +46,45 @@ func (d *managedClusterAddonConfigurationReconciler) mergeAddonConfig( mca *addonv1alpha1.ManagedClusterAddOn, desiredConfigMap addonConfigMap) *addonv1alpha1.ManagedClusterAddOn { mcaCopy := mca.DeepCopy() - var mergedConfigs []addonv1alpha1.ConfigReference - // remove configs that are not desired - for _, config := range mcaCopy.Status.ConfigReferences { - if _, ok := desiredConfigMap[config.ConfigGroupResource]; ok { - mergedConfigs = append(mergedConfigs, config) + mergedConfigs := make(addonConfigMap) + // First go through the configReferences listed in mca status, + // if the existing config (gvk + namespace + name) is also in the desiredConfigMap, append it to mergedConfigs, + // this will save the LastAppliedConfig and LastObservedGeneration from mca status. + for _, configRef := range mcaCopy.Status.ConfigReferences { + gr := configRef.ConfigGroupResource + if _, ok := mergedConfigs[gr]; !ok { + mergedConfigs[gr] = []addonv1alpha1.ConfigReference{} + } + if _, ok := desiredConfigMap.containsConfig(gr, configRef.DesiredConfig.ConfigReferent); ok { + mergedConfigs[gr] = append(mergedConfigs[gr], configRef) } } - // append or update configs - for _, config := range desiredConfigMap { - var match bool - for i := range mergedConfigs { - if mergedConfigs[i].ConfigGroupResource != config.ConfigGroupResource { - continue + // Then go through the desiredConfigMap, for each configReference, + // if the desired config (gvk + namespace + name) is aleady in the mergedConfigs, + // update the ConfigReferent and DesiredConfig (including desired spechash) to mergedConfigs. + // else just append it to the mergedConfigs. + for gr, configReferences := range desiredConfigMap { + for _, configRef := range configReferences { + if _, ok := mergedConfigs[gr]; !ok { + mergedConfigs[gr] = []addonv1alpha1.ConfigReference{} } - match = true - // set LastObservedGeneration to 0 when config name/namespace changes - if mergedConfigs[i].DesiredConfig != nil && (mergedConfigs[i].DesiredConfig.ConfigReferent != config.DesiredConfig.ConfigReferent) { - mergedConfigs[i].LastObservedGeneration = 0 + referent := configRef.DesiredConfig.ConfigReferent + if i, exist := mergedConfigs.containsConfig(gr, referent); exist { + mergedConfigs[gr][i].ConfigReferent = configRef.ConfigReferent + mergedConfigs[gr][i].DesiredConfig = configRef.DesiredConfig.DeepCopy() + } else { + mergedConfigs[gr] = append(mergedConfigs[gr], configRef) } - mergedConfigs[i].ConfigReferent = config.ConfigReferent - mergedConfigs[i].DesiredConfig = config.DesiredConfig.DeepCopy() - } - - if !match { - mergedConfigs = append(mergedConfigs, config) } } - mcaCopy.Status.ConfigReferences = mergedConfigs + // sort by gvk and set the final config references + configRefs := []addonv1alpha1.ConfigReference{} + for _, gvk := range mergedConfigs.orderedKeys() { + configRefs = append(configRefs, mergedConfigs[gvk]...) + } + mcaCopy.Status.ConfigReferences = configRefs return mcaCopy } diff --git a/pkg/addon/controllers/addonconfiguration/addon_configuration_reconciler_test.go b/pkg/addon/controllers/addonconfiguration/addon_configuration_reconciler_test.go index b7142092a..05d71d440 100644 --- a/pkg/addon/controllers/addonconfiguration/addon_configuration_reconciler_test.go +++ b/pkg/addon/controllers/addonconfiguration/addon_configuration_reconciler_test.go @@ -154,12 +154,142 @@ func TestAddonConfigReconcile(t *testing.T) { }, }, { - name: "mca override", + name: "cma install strategy override with multiple same-GVK", managedClusteraddon: []runtime.Object{ - newManagedClusterAddon("test", "cluster1", []addonv1alpha1.AddOnConfig{{ + addontesting.NewAddon("test", "cluster1"), + addontesting.NewAddon("test", "cluster2"), + }, + placements: []runtime.Object{ + &clusterv1beta1.Placement{ObjectMeta: metav1.ObjectMeta{Name: "test-placement", Namespace: "default"}}, + }, + placementDecisions: []runtime.Object{ + &clusterv1beta1.PlacementDecision{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-placement", + Namespace: "default", + Labels: map[string]string{ + clusterv1beta1.PlacementLabel: "test-placement", + clusterv1beta1.DecisionGroupIndexLabel: "0", + }, + }, + Status: clusterv1beta1.PlacementDecisionStatus{ + Decisions: []clusterv1beta1.ClusterDecision{{ClusterName: "cluster2"}}, + }, + }, + }, + clusterManagementAddon: addontesting.NewClusterManagementAddon("test", "", "").WithSupportedConfigs( + addonv1alpha1.ConfigMeta{ ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, - }}, nil, nil), + DefaultConfig: &addonv1alpha1.ConfigReferent{Name: "test"}, + }, + addonv1alpha1.ConfigMeta{ + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + DefaultConfig: &addonv1alpha1.ConfigReferent{Name: "test"}, + }, + ).WithDefaultConfigReferences( + addonv1alpha1.DefaultConfigReference{ + ConfigGroupResource: v1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + DesiredConfig: &v1alpha1.ConfigSpecHash{ + ConfigReferent: v1alpha1.ConfigReferent{Name: "test"}, + SpecHash: "hash", + }, + }, + addonv1alpha1.DefaultConfigReference{ + ConfigGroupResource: v1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + DesiredConfig: &v1alpha1.ConfigSpecHash{ + ConfigReferent: v1alpha1.ConfigReferent{Name: "test"}, + SpecHash: "hash", + }, + }, + ).WithPlacementStrategy(addonv1alpha1.PlacementStrategy{ + PlacementRef: addonv1alpha1.PlacementRef{Name: "test-placement", Namespace: "default"}, + RolloutStrategy: clusterv1alpha1.RolloutStrategy{Type: clusterv1alpha1.All}, + }).WithInstallProgression(addonv1alpha1.InstallProgression{ + PlacementRef: addonv1alpha1.PlacementRef{Name: "test-placement", Namespace: "default"}, + ConfigReferences: []addonv1alpha1.InstallConfigReference{ + { + ConfigGroupResource: v1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + DesiredConfig: &v1alpha1.ConfigSpecHash{ + ConfigReferent: v1alpha1.ConfigReferent{Name: "test1"}, + SpecHash: "hash1", + }, + }, + { + ConfigGroupResource: v1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + DesiredConfig: &v1alpha1.ConfigSpecHash{ + ConfigReferent: v1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "hash2", + }, + }, + }, + }).Build(), + validateAddonActions: func(t *testing.T, actions []clienttesting.Action) { + addontesting.AssertActions(t, actions, "patch", "patch") + sort.Sort(byPatchName(actions)) + expectPatchConfigurationAction(t, actions[0], []addonv1alpha1.ConfigReference{ + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, + SpecHash: "hash", + }, + LastObservedGeneration: 0, + }, + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, + SpecHash: "hash", + }, + LastObservedGeneration: 0, + }, + }) + expectPatchConfigurationAction(t, actions[1], []addonv1alpha1.ConfigReference{ + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, + SpecHash: "hash", + }, + LastObservedGeneration: 0, + }, + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + SpecHash: "hash1", + }, + LastObservedGeneration: 0, + }, + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "hash2", + }, + LastObservedGeneration: 0, + }, + }) + }, + }, + { + name: "mca override with multiple same-GVK", + managedClusteraddon: []runtime.Object{ + newManagedClusterAddon("test", "cluster1", []addonv1alpha1.AddOnConfig{ + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + }, + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test3"}, + }, + }, nil, nil), addontesting.NewAddon("test", "cluster2"), }, placements: []runtime.Object{ @@ -180,10 +310,15 @@ func TestAddonConfigReconcile(t *testing.T) { }, }, }, - clusterManagementAddon: addontesting.NewClusterManagementAddon("test", "", "").WithSupportedConfigs(addonv1alpha1.ConfigMeta{ - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, - DefaultConfig: &addonv1alpha1.ConfigReferent{Name: "test"}, - }).WithDefaultConfigReferences(addonv1alpha1.DefaultConfigReference{ + clusterManagementAddon: addontesting.NewClusterManagementAddon("test", "", "").WithSupportedConfigs( + addonv1alpha1.ConfigMeta{ + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + DefaultConfig: &addonv1alpha1.ConfigReferent{Name: "test"}, + }, + addonv1alpha1.ConfigMeta{ + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + }, + ).WithDefaultConfigReferences(addonv1alpha1.DefaultConfigReference{ ConfigGroupResource: v1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, DesiredConfig: &v1alpha1.ConfigSpecHash{ ConfigReferent: v1alpha1.ConfigReferent{Name: "test"}, @@ -195,6 +330,13 @@ func TestAddonConfigReconcile(t *testing.T) { }).WithInstallProgression(addonv1alpha1.InstallProgression{ PlacementRef: addonv1alpha1.PlacementRef{Name: "test-placement", Namespace: "default"}, ConfigReferences: []addonv1alpha1.InstallConfigReference{ + { + ConfigGroupResource: v1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + DesiredConfig: &v1alpha1.ConfigSpecHash{ + ConfigReferent: v1alpha1.ConfigReferent{Name: "test1"}, + SpecHash: "hash1", + }, + }, { ConfigGroupResource: v1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, DesiredConfig: &v1alpha1.ConfigSpecHash{ @@ -207,6 +349,116 @@ func TestAddonConfigReconcile(t *testing.T) { validateAddonActions: func(t *testing.T, actions []clienttesting.Action) { addontesting.AssertActions(t, actions, "patch", "patch") sort.Sort(byPatchName(actions)) + expectPatchConfigurationAction(t, actions[0], []addonv1alpha1.ConfigReference{ + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + SpecHash: "hash1", + }, + LastObservedGeneration: 0, + }, + { + 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: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test3"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test3"}, + SpecHash: "", + }, + LastObservedGeneration: 0, + }, + }) + expectPatchConfigurationAction(t, actions[1], []addonv1alpha1.ConfigReference{ + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + SpecHash: "hash1", + }, + LastObservedGeneration: 0, + }, + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + SpecHash: "hash1", + }, + LastObservedGeneration: 0, + }}) + }, + }, + { + name: "duplicate configs", + managedClusteraddon: []runtime.Object{ + newManagedClusterAddon("test", "cluster1", []addonv1alpha1.AddOnConfig{ + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + }, + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + }, + }, nil, nil), + addontesting.NewAddon("test", "cluster2"), + }, + placements: []runtime.Object{ + &clusterv1beta1.Placement{ObjectMeta: metav1.ObjectMeta{Name: "test-placement", Namespace: "default"}}, + }, + placementDecisions: []runtime.Object{ + &clusterv1beta1.PlacementDecision{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-placement", + Namespace: "default", + Labels: map[string]string{ + clusterv1beta1.PlacementLabel: "test-placement", + clusterv1beta1.DecisionGroupIndexLabel: "0", + }, + }, + Status: clusterv1beta1.PlacementDecisionStatus{ + Decisions: []clusterv1beta1.ClusterDecision{{ClusterName: "cluster1"}, {ClusterName: "cluster2"}}, + }, + }, + }, + clusterManagementAddon: addontesting.NewClusterManagementAddon("test", "", "").WithSupportedConfigs(addonv1alpha1.ConfigMeta{ + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + DefaultConfig: &addonv1alpha1.ConfigReferent{Name: "test"}, + }).WithPlacementStrategy(addonv1alpha1.PlacementStrategy{ + PlacementRef: addonv1alpha1.PlacementRef{Name: "test-placement", Namespace: "default"}, + RolloutStrategy: clusterv1alpha1.RolloutStrategy{Type: clusterv1alpha1.All}, + }).WithInstallProgression(addonv1alpha1.InstallProgression{ + PlacementRef: addonv1alpha1.PlacementRef{Name: "test-placement", Namespace: "default"}, + ConfigReferences: []addonv1alpha1.InstallConfigReference{ + { + ConfigGroupResource: v1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + DesiredConfig: &v1alpha1.ConfigSpecHash{ + ConfigReferent: v1alpha1.ConfigReferent{Name: "test1"}, + SpecHash: "hash1", + }, + }, + { + ConfigGroupResource: v1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + DesiredConfig: &v1alpha1.ConfigSpecHash{ + ConfigReferent: v1alpha1.ConfigReferent{Name: "test1"}, + SpecHash: "hash1", + }, + }, + }, + }).Build(), + validateAddonActions: func(t *testing.T, actions []clienttesting.Action) { + addontesting.AssertActions(t, actions, "patch", "patch") expectPatchConfigurationAction(t, actions[0], []addonv1alpha1.ConfigReference{{ ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, @@ -237,6 +489,10 @@ func TestAddonConfigReconcile(t *testing.T) { ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, SpecHash: "hash1", }, + LastAppliedConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + SpecHash: "hash1", + }, LastObservedGeneration: 1, }}, nil), }, @@ -299,6 +555,10 @@ func TestAddonConfigReconcile(t *testing.T) { ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, SpecHash: "hash1", }, + LastAppliedConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + SpecHash: "hash1", + }, LastObservedGeneration: 1, }}, nil), }, @@ -350,6 +610,10 @@ func TestAddonConfigReconcile(t *testing.T) { ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, SpecHash: "hash1new", }, + LastAppliedConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + SpecHash: "hash1", + }, LastObservedGeneration: 1, }}) }, diff --git a/pkg/addon/controllers/addonconfiguration/cma_progressing_reconciler_test.go b/pkg/addon/controllers/addonconfiguration/cma_progressing_reconciler_test.go index 46586749a..bf436b88f 100644 --- a/pkg/addon/controllers/addonconfiguration/cma_progressing_reconciler_test.go +++ b/pkg/addon/controllers/addonconfiguration/cma_progressing_reconciler_test.go @@ -130,6 +130,13 @@ func TestMgmtAddonProgressingReconcile(t *testing.T) { SpecHash: "hash1", }, }, + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "hash2", + }, + }, } return addon }()}, @@ -147,6 +154,13 @@ func TestMgmtAddonProgressingReconcile(t *testing.T) { SpecHash: "hash1", }, }, + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "hash2", + }, + }, }, }).Build()}, placements: []runtime.Object{ @@ -209,6 +223,17 @@ func TestMgmtAddonProgressingReconcile(t *testing.T) { SpecHash: "hash1", }, }, + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "hash2", + }, + LastAppliedConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "hash2", + }, + }, } return addon }()}, @@ -226,6 +251,13 @@ func TestMgmtAddonProgressingReconcile(t *testing.T) { SpecHash: "hash1", }, }, + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "hash2", + }, + }, }, }).Build()}, placements: []runtime.Object{ @@ -288,6 +320,13 @@ func TestMgmtAddonProgressingReconcile(t *testing.T) { SpecHash: "hash1", }, }, + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "hash2", + }, + }, } return addon }()}, @@ -309,6 +348,17 @@ func TestMgmtAddonProgressingReconcile(t *testing.T) { SpecHash: "hash", }, }, + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "hash2", + }, + LastAppliedConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "hash", + }, + }, }, }).Build()}, placements: []runtime.Object{ @@ -368,6 +418,17 @@ func TestMgmtAddonProgressingReconcile(t *testing.T) { SpecHash: "hash1", }, }, + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "hash2", + }, + LastAppliedConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "hash2", + }, + }, } return addon }()}, @@ -389,6 +450,17 @@ func TestMgmtAddonProgressingReconcile(t *testing.T) { SpecHash: "hash", }, }, + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "hash2", + }, + LastAppliedConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "hash", + }, + }, }, }).Build()}, placements: []runtime.Object{ diff --git a/pkg/addon/controllers/addonconfiguration/graph.go b/pkg/addon/controllers/addonconfiguration/graph.go index 32700f244..561a7f069 100644 --- a/pkg/addon/controllers/addonconfiguration/graph.go +++ b/pkg/addon/controllers/addonconfiguration/graph.go @@ -48,14 +48,14 @@ type addonNode struct { status *clustersdkv1alpha1.ClusterRolloutStatus } -type addonConfigMap map[addonv1alpha1.ConfigGroupResource]addonv1alpha1.ConfigReference +type addonConfigMap map[addonv1alpha1.ConfigGroupResource][]addonv1alpha1.ConfigReference // set addon rollout status func (n *addonNode) setRolloutStatus() { n.status = &clustersdkv1alpha1.ClusterRolloutStatus{ClusterName: n.mca.Namespace} // desired configs doesn't match actual configs, set to ToApply - if len(n.mca.Status.ConfigReferences) != len(n.desiredConfigs) { + if len(n.mca.Status.ConfigReferences) != n.desiredConfigs.len() { n.status.Status = clustersdkv1alpha1.ToApply return } @@ -69,9 +69,9 @@ func (n *addonNode) setRolloutStatus() { } for _, actual := range n.mca.Status.ConfigReferences { - if desired, ok := n.desiredConfigs[actual.ConfigGroupResource]; ok { + if index, exist := n.desiredConfigs.containsConfig(actual.ConfigGroupResource, actual.DesiredConfig.ConfigReferent); exist { // desired config spec hash doesn't match actual, set to ToApply - if !equality.Semantic.DeepEqual(desired.DesiredConfig, actual.DesiredConfig) { + if !equality.Semantic.DeepEqual(n.desiredConfigs[actual.ConfigGroupResource][index].DesiredConfig, actual.DesiredConfig) { n.status.Status = clustersdkv1alpha1.ToApply return // desired config spec hash matches actual, but last applied config spec hash doesn't match actual @@ -88,6 +88,7 @@ func (n *addonNode) setRolloutStatus() { } return } + } else { n.status.Status = clustersdkv1alpha1.ToApply return @@ -101,19 +102,61 @@ func (n *addonNode) setRolloutStatus() { } } -func (d addonConfigMap) copy() addonConfigMap { - output := addonConfigMap{} - for k, v := range d { - output[k] = v +func (m addonConfigMap) copy() addonConfigMap { + output := make(addonConfigMap, len(m)) + for k, v := range m { + // copy the key ConfigGroupResource + newk := k.DeepCopy() + // copy the slice of ConfigReference + newv := make([]addonv1alpha1.ConfigReference, len(v)) + for i, cr := range v { + newv[i] = *cr.DeepCopy() + } + output[*newk] = newv } return output } +func (m addonConfigMap) len() int { + totalLength := 0 + for _, v := range m { + totalLength += len(v) + } + return totalLength +} + +func (m addonConfigMap) containsConfig(expectConfigGr addonv1alpha1.ConfigGroupResource, + expectConfigRef addonv1alpha1.ConfigReferent) (int, bool) { + if existArray, ok := m[expectConfigGr]; ok { + for i, e := range existArray { + if expectConfigRef == e.DesiredConfig.ConfigReferent { + return i, true + } + } + } + + return -1, false +} + +func (m addonConfigMap) orderedKeys() []addonv1alpha1.ConfigGroupResource { + gvks := []addonv1alpha1.ConfigGroupResource{} + for gvk := range m { + gvks = append(gvks, gvk) + } + sort.Slice(gvks, func(i, j int) bool { + if gvks[i].Group == gvks[j].Group { + return gvks[i].Resource < gvks[j].Resource + } + return gvks[i].Group < gvks[j].Group + }) + return gvks +} + func newGraph(supportedConfigs []addonv1alpha1.ConfigMeta, defaultConfigReferences []addonv1alpha1.DefaultConfigReference) *configurationGraph { graph := &configurationGraph{ nodes: []*installStrategyNode{}, defaults: &installStrategyNode{ - desiredConfigs: map[addonv1alpha1.ConfigGroupResource]addonv1alpha1.ConfigReference{}, + desiredConfigs: addonConfigMap{}, children: map[string]*addonNode{}, }, } @@ -121,11 +164,13 @@ func newGraph(supportedConfigs []addonv1alpha1.ConfigMeta, defaultConfigReferenc // init graph.defaults.desiredConfigs with supportedConfigs for _, config := range supportedConfigs { if config.DefaultConfig != nil { - graph.defaults.desiredConfigs[config.ConfigGroupResource] = addonv1alpha1.ConfigReference{ - ConfigGroupResource: config.ConfigGroupResource, - ConfigReferent: *config.DefaultConfig, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: *config.DefaultConfig, + graph.defaults.desiredConfigs[config.ConfigGroupResource] = []addonv1alpha1.ConfigReference{ + { + ConfigGroupResource: config.ConfigGroupResource, + ConfigReferent: *config.DefaultConfig, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: *config.DefaultConfig, + }, }, } } @@ -135,9 +180,12 @@ func newGraph(supportedConfigs []addonv1alpha1.ConfigMeta, defaultConfigReferenc if configRef.DesiredConfig == nil { continue } - defaultsDesiredConfig, ok := graph.defaults.desiredConfigs[configRef.ConfigGroupResource] - if ok && (defaultsDesiredConfig.DesiredConfig.ConfigReferent == configRef.DesiredConfig.ConfigReferent) { - defaultsDesiredConfig.DesiredConfig.SpecHash = configRef.DesiredConfig.SpecHash + if defaultsDesiredConfigArray, ok := graph.defaults.desiredConfigs[configRef.ConfigGroupResource]; ok { + for i, defaultsDesiredConfig := range defaultsDesiredConfigArray { + if defaultsDesiredConfig.DesiredConfig.ConfigReferent == configRef.DesiredConfig.ConfigReferent { + graph.defaults.desiredConfigs[configRef.ConfigGroupResource][i].DesiredConfig.SpecHash = configRef.DesiredConfig.SpecHash + } + } } } @@ -212,16 +260,7 @@ func (g *configurationGraph) addPlacementNode( // overrides configuration by install strategy if len(installConfigReference) > 0 { node.desiredConfigs = node.desiredConfigs.copy() - for _, configRef := range installConfigReference { - if configRef.DesiredConfig == nil { - continue - } - node.desiredConfigs[configRef.ConfigGroupResource] = addonv1alpha1.ConfigReference{ - ConfigGroupResource: configRef.ConfigGroupResource, - ConfigReferent: configRef.DesiredConfig.ConfigReferent, - DesiredConfig: configRef.DesiredConfig.DeepCopy(), - } - } + overrideConfigMapByInstallConfigRef(node.desiredConfigs, installConfigReference) } // remove addon in defaults and other placements. @@ -296,22 +335,18 @@ func (n *installStrategyNode) addNode(addon *addonv1alpha1.ManagedClusterAddOn) if len(addon.Spec.Configs) > 0 { 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 { - n.children[addon.Namespace].desiredConfigs[config.ConfigGroupResource] = addonv1alpha1.ConfigReference{ - ConfigGroupResource: config.ConfigGroupResource, - ConfigReferent: config.ConfigReferent, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: config.ConfigReferent, - }, + overrideConfigMapByAddOnConfigs(n.children[addon.Namespace].desiredConfigs, addon.Spec.Configs) + + // copy the spechash from mca status + for _, configRef := range addon.Status.ConfigReferences { + if configRef.DesiredConfig == nil { + continue } - // copy the spechash from mca status - for _, configRef := range addon.Status.ConfigReferences { - if configRef.DesiredConfig == nil { - continue - } - nodeDesiredConfig, ok := n.children[addon.Namespace].desiredConfigs[configRef.ConfigGroupResource] - if ok && (nodeDesiredConfig.DesiredConfig.ConfigReferent == configRef.DesiredConfig.ConfigReferent) { - nodeDesiredConfig.DesiredConfig.SpecHash = configRef.DesiredConfig.SpecHash + if nodeDesiredConfigArray, ok := n.children[addon.Namespace].desiredConfigs[configRef.ConfigGroupResource]; ok { + for i, nodeDesiredConfig := range nodeDesiredConfigArray { + if nodeDesiredConfig.DesiredConfig.ConfigReferent == configRef.DesiredConfig.ConfigReferent { + n.children[addon.Namespace].desiredConfigs[configRef.ConfigGroupResource][i].DesiredConfig.SpecHash = configRef.DesiredConfig.SpecHash + } } } } @@ -445,9 +480,14 @@ func desiredConfigsEqual(a, b addonConfigMap) bool { } for configgrA := range a { - if a[configgrA] != b[configgrA] { + if len(a[configgrA]) != len(b[configgrA]) { return false } + for i := range a[configgrA] { + if a[configgrA][i] != b[configgrA][i] { + return false + } + } } return true @@ -461,3 +501,63 @@ func rolloutStatusHasCluster(clusterRolloutStatus []clustersdkv1alpha1.ClusterRo } return false } + +// Override the desired addonConfigMap by a slice of InstallConfigReference (from cma status), +func overrideConfigMapByInstallConfigRef( + desiredConfigs addonConfigMap, + installConfigRefs []addonv1alpha1.InstallConfigReference, +) { + gvkOverwritten := sets.New[addonv1alpha1.ConfigGroupResource]() + // Go through the cma installConfigReferences, + // for a group of configs with same gvk, cma installConfigReferences override the desiredConfigs. + for _, configRef := range installConfigRefs { + if configRef.DesiredConfig == nil { + continue + } + gr := configRef.ConfigGroupResource + referent := configRef.DesiredConfig.ConfigReferent + if !gvkOverwritten.Has(gr) { + desiredConfigs[gr] = []addonv1alpha1.ConfigReference{} + gvkOverwritten.Insert(gr) + } + // If a config not exist in the desiredConfigs, append it. + // This is to avoid adding duplicate configs (name + namespace). + if _, exist := desiredConfigs.containsConfig(gr, referent); !exist { + desiredConfigs[gr] = append(desiredConfigs[gr], addonv1alpha1.ConfigReference{ + ConfigGroupResource: gr, + ConfigReferent: referent, + DesiredConfig: configRef.DesiredConfig.DeepCopy(), + }) + } + } +} + +// Override the desired addonConfigMap by a slice of InstallConfigReference (from mca spec), +func overrideConfigMapByAddOnConfigs( + desiredConfigs addonConfigMap, + addOnConfigs []addonv1alpha1.AddOnConfig, +) { + gvkOverwritten := sets.New[addonv1alpha1.ConfigGroupResource]() + // Go through the mca addOnConfigs, + // for a group of configs with same gvk, mca addOnConfigs override the desiredConfigs. + for _, config := range addOnConfigs { + gr := config.ConfigGroupResource + referent := config.ConfigReferent + if !gvkOverwritten.Has(gr) { + desiredConfigs[gr] = []addonv1alpha1.ConfigReference{} + gvkOverwritten.Insert(gr) + } + // If a config not exist in the desiredConfigs, append it. + // This is to avoid adding duplicate configs (name + namespace). + if _, exist := desiredConfigs.containsConfig(gr, referent); !exist { + desiredConfigs[gr] = append(desiredConfigs[gr], addonv1alpha1.ConfigReference{ + ConfigGroupResource: gr, + ConfigReferent: referent, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: referent, + }, + }) + + } + } +} diff --git a/pkg/addon/controllers/addonconfiguration/graph_test.go b/pkg/addon/controllers/addonconfiguration/graph_test.go index 8313925b5..2b2175e7a 100644 --- a/pkg/addon/controllers/addonconfiguration/graph_test.go +++ b/pkg/addon/controllers/addonconfiguration/graph_test.go @@ -56,13 +56,15 @@ func TestConfigurationGraph(t *testing.T) { }, expected: []*addonNode{ { - desiredConfigs: map[addonv1alpha1.ConfigGroupResource]addonv1alpha1.ConfigReference{ + desiredConfigs: map[addonv1alpha1.ConfigGroupResource][]addonv1alpha1.ConfigReference{ {Group: "core", Resource: "Foo"}: { - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, - SpecHash: "", + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, + SpecHash: "", + }, }, }, }, @@ -72,13 +74,15 @@ func TestConfigurationGraph(t *testing.T) { Status: clustersdkv1alpha1.ToApply}, }, { - desiredConfigs: map[addonv1alpha1.ConfigGroupResource]addonv1alpha1.ConfigReference{ + desiredConfigs: map[addonv1alpha1.ConfigGroupResource][]addonv1alpha1.ConfigReference{ {Group: "core", Resource: "Foo"}: { - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, - SpecHash: "", + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, + SpecHash: "", + }, }, }, }, @@ -135,21 +139,25 @@ func TestConfigurationGraph(t *testing.T) { }, expected: []*addonNode{ { - desiredConfigs: map[addonv1alpha1.ConfigGroupResource]addonv1alpha1.ConfigReference{ + desiredConfigs: map[addonv1alpha1.ConfigGroupResource][]addonv1alpha1.ConfigReference{ {Group: "core", Resource: "Bar"}: { - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, - SpecHash: "", + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + SpecHash: "", + }, }, }, {Group: "core", Resource: "Foo"}: { - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, - SpecHash: "", + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, + SpecHash: "", + }, }, }, }, @@ -159,21 +167,25 @@ func TestConfigurationGraph(t *testing.T) { Status: clustersdkv1alpha1.ToApply}, }, { - desiredConfigs: map[addonv1alpha1.ConfigGroupResource]addonv1alpha1.ConfigReference{ + desiredConfigs: map[addonv1alpha1.ConfigGroupResource][]addonv1alpha1.ConfigReference{ {Group: "core", Resource: "Bar"}: { - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, - SpecHash: "", + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "", + }, }, }, {Group: "core", Resource: "Foo"}: { - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, - SpecHash: "", + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "", + }, }, }, }, @@ -184,21 +196,25 @@ func TestConfigurationGraph(t *testing.T) { }, }, { - desiredConfigs: map[addonv1alpha1.ConfigGroupResource]addonv1alpha1.ConfigReference{ + desiredConfigs: map[addonv1alpha1.ConfigGroupResource][]addonv1alpha1.ConfigReference{ {Group: "core", Resource: "Bar"}: { - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, - SpecHash: "", + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, + SpecHash: "", + }, }, }, {Group: "core", Resource: "Foo"}: { - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, - SpecHash: "", + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, + SpecHash: "", + }, }, }, }, @@ -311,13 +327,15 @@ func TestConfigurationGraph(t *testing.T) { }, expected: []*addonNode{ { - desiredConfigs: map[addonv1alpha1.ConfigGroupResource]addonv1alpha1.ConfigReference{ + desiredConfigs: map[addonv1alpha1.ConfigGroupResource][]addonv1alpha1.ConfigReference{ {Group: "core", Resource: "Bar"}: { - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, - SpecHash: "", + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + SpecHash: "", + }, }, }, }, @@ -329,13 +347,15 @@ func TestConfigurationGraph(t *testing.T) { }, }, { - desiredConfigs: map[addonv1alpha1.ConfigGroupResource]addonv1alpha1.ConfigReference{ + desiredConfigs: map[addonv1alpha1.ConfigGroupResource][]addonv1alpha1.ConfigReference{ {Group: "core", Resource: "Bar"}: { - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, - SpecHash: "", + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + SpecHash: "", + }, }, }, }, @@ -347,13 +367,15 @@ func TestConfigurationGraph(t *testing.T) { }, }, { - desiredConfigs: map[addonv1alpha1.ConfigGroupResource]addonv1alpha1.ConfigReference{ + desiredConfigs: map[addonv1alpha1.ConfigGroupResource][]addonv1alpha1.ConfigReference{ {Group: "core", Resource: "Bar"}: { - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, - SpecHash: "", + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + SpecHash: "", + }, }, }, }, @@ -411,21 +433,25 @@ func TestConfigurationGraph(t *testing.T) { }, expected: []*addonNode{ { - desiredConfigs: map[addonv1alpha1.ConfigGroupResource]addonv1alpha1.ConfigReference{ + desiredConfigs: map[addonv1alpha1.ConfigGroupResource][]addonv1alpha1.ConfigReference{ {Group: "core", Resource: "Bar"}: { - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, - SpecHash: "", + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + SpecHash: "", + }, }, }, {Group: "core", Resource: "Foo"}: { - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, - SpecHash: "", + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, + SpecHash: "", + }, }, }, }, @@ -435,21 +461,25 @@ func TestConfigurationGraph(t *testing.T) { Status: clustersdkv1alpha1.ToApply}, }, { - desiredConfigs: map[addonv1alpha1.ConfigGroupResource]addonv1alpha1.ConfigReference{ + desiredConfigs: map[addonv1alpha1.ConfigGroupResource][]addonv1alpha1.ConfigReference{ {Group: "core", Resource: "Bar"}: { - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, - SpecHash: "", + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "", + }, }, }, {Group: "core", Resource: "Foo"}: { - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, - SpecHash: "", + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "", + }, }, }, }, @@ -459,21 +489,25 @@ func TestConfigurationGraph(t *testing.T) { Status: clustersdkv1alpha1.ToApply}, }, { - desiredConfigs: map[addonv1alpha1.ConfigGroupResource]addonv1alpha1.ConfigReference{ + desiredConfigs: map[addonv1alpha1.ConfigGroupResource][]addonv1alpha1.ConfigReference{ {Group: "core", Resource: "Bar"}: { - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, - SpecHash: "", + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "", + }, }, }, {Group: "core", Resource: "Foo"}: { - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, - SpecHash: "", + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "", + }, }, }, }, @@ -533,21 +567,25 @@ func TestConfigurationGraph(t *testing.T) { }, expected: []*addonNode{ { - desiredConfigs: map[addonv1alpha1.ConfigGroupResource]addonv1alpha1.ConfigReference{ + desiredConfigs: map[addonv1alpha1.ConfigGroupResource][]addonv1alpha1.ConfigReference{ {Group: "core", Resource: "Bar"}: { - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, - SpecHash: "", + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + SpecHash: "", + }, }, }, {Group: "core", Resource: "Foo"}: { - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, - SpecHash: "", + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + SpecHash: "", + }, }, }, }, @@ -560,21 +598,25 @@ func TestConfigurationGraph(t *testing.T) { Status: clustersdkv1alpha1.ToApply}, }, { - desiredConfigs: map[addonv1alpha1.ConfigGroupResource]addonv1alpha1.ConfigReference{ + desiredConfigs: map[addonv1alpha1.ConfigGroupResource][]addonv1alpha1.ConfigReference{ {Group: "core", Resource: "Bar"}: { - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, - SpecHash: "", + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "", + }, }, }, {Group: "core", Resource: "Foo"}: { - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, - SpecHash: "", + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "", + }, }, }, }, @@ -584,21 +626,25 @@ func TestConfigurationGraph(t *testing.T) { Status: clustersdkv1alpha1.ToApply}, }, { - desiredConfigs: map[addonv1alpha1.ConfigGroupResource]addonv1alpha1.ConfigReference{ + desiredConfigs: map[addonv1alpha1.ConfigGroupResource][]addonv1alpha1.ConfigReference{ {Group: "core", Resource: "Bar"}: { - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, - SpecHash: "", + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, + SpecHash: "", + }, }, }, {Group: "core", Resource: "Foo"}: { - ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, - SpecHash: "", + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, + SpecHash: "", + }, }, }, }, @@ -609,6 +655,118 @@ func TestConfigurationGraph(t *testing.T) { }, }, }, + { + name: "placement strategy with multiple same-GVKs", + defaultConfigs: []addonv1alpha1.ConfigMeta{ + {ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + DefaultConfig: &addonv1alpha1.ConfigReferent{Name: "test"}}, + {ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + DefaultConfig: &addonv1alpha1.ConfigReferent{Name: "test"}}, + }, + defaultConfigReference: []addonv1alpha1.DefaultConfigReference{ + newDefaultConfigReference("core", "Bar", "test", ""), + newDefaultConfigReference("core", "Foo", "test", ""), + }, + addons: []*addonv1alpha1.ManagedClusterAddOn{ + addontesting.NewAddon("test", "cluster1"), + addontesting.NewAddon("test", "cluster2"), + }, + placementDesicions: []placementDesicion{ + {PlacementRef: addonv1alpha1.PlacementRef{Name: "placement1", Namespace: "test"}, + clusters: []clusterv1beta1.ClusterDecision{{ClusterName: "cluster1"}}}, + {PlacementRef: addonv1alpha1.PlacementRef{Name: "placement2", Namespace: "test"}, + clusters: []clusterv1beta1.ClusterDecision{{ClusterName: "cluster2"}}}, + }, + placementStrategies: []addonv1alpha1.PlacementStrategy{ + {PlacementRef: addonv1alpha1.PlacementRef{Name: "placement1", Namespace: "test"}, + RolloutStrategy: clusterv1alpha1.RolloutStrategy{Type: clusterv1alpha1.All}}, + {PlacementRef: addonv1alpha1.PlacementRef{Name: "placement2", Namespace: "test"}, + RolloutStrategy: clusterv1alpha1.RolloutStrategy{Type: clusterv1alpha1.All}}, + }, + installProgressions: []addonv1alpha1.InstallProgression{ + { + PlacementRef: addonv1alpha1.PlacementRef{Name: "placement1", Namespace: "test"}, + ConfigReferences: []addonv1alpha1.InstallConfigReference{ + newInstallConfigReference("core", "Bar", "test1", ""), + newInstallConfigReference("core", "Bar", "test2", ""), + }, + }, + { + PlacementRef: addonv1alpha1.PlacementRef{Name: "placement2", Namespace: "test"}, + ConfigReferences: []addonv1alpha1.InstallConfigReference{ + newInstallConfigReference("core", "Bar", "test2", ""), + newInstallConfigReference("core", "Foo", "test2", ""), + }, + }, + }, + expected: []*addonNode{ + { + desiredConfigs: map[addonv1alpha1.ConfigGroupResource][]addonv1alpha1.ConfigReference{ + {Group: "core", Resource: "Bar"}: { + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"}, + SpecHash: "", + }, + }, + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "", + }, + }, + }, + {Group: "core", Resource: "Foo"}: { + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test"}, + SpecHash: "", + }, + }, + }, + }, + mca: addontesting.NewAddon("test", "cluster1"), + status: &clustersdkv1alpha1.ClusterRolloutStatus{ + ClusterName: "cluster1", + Status: clustersdkv1alpha1.ToApply}, + }, + { + desiredConfigs: map[addonv1alpha1.ConfigGroupResource][]addonv1alpha1.ConfigReference{ + {Group: "core", Resource: "Bar"}: { + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "", + }, + }, + }, + {Group: "core", Resource: "Foo"}: { + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"}, + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"}, + SpecHash: "", + }, + }, + }, + }, + mca: addontesting.NewAddon("test", "cluster2"), + status: &clustersdkv1alpha1.ClusterRolloutStatus{ + ClusterName: "cluster2", + Status: clustersdkv1alpha1.ToApply, + }, + }, + }, + }, } for _, c := range cases { diff --git a/pkg/addon/controllers/addonprogressing/controller_test.go b/pkg/addon/controllers/addonprogressing/controller_test.go index 7ef3b28a1..320a59f8c 100644 --- a/pkg/addon/controllers/addonprogressing/controller_test.go +++ b/pkg/addon/controllers/addonprogressing/controller_test.go @@ -115,11 +115,22 @@ func TestReconcile(t *testing.T) { { ConfigGroupResource: v1alpha1.ConfigGroupResource{Group: "core", Resource: "foo"}, DesiredConfig: &v1alpha1.ConfigSpecHash{ - ConfigReferent: v1alpha1.ConfigReferent{Name: "test", Namespace: "open-cluster-management"}, - SpecHash: "hashnew", + ConfigReferent: v1alpha1.ConfigReferent{Name: "test1", Namespace: "open-cluster-management"}, + SpecHash: "hash1new", }, LastAppliedConfig: &v1alpha1.ConfigSpecHash{ - ConfigReferent: v1alpha1.ConfigReferent{Name: "test", Namespace: "open-cluster-management"}, + ConfigReferent: v1alpha1.ConfigReferent{Name: "test1", Namespace: "open-cluster-management"}, + SpecHash: "", + }, + }, + { + ConfigGroupResource: v1alpha1.ConfigGroupResource{Group: "core", Resource: "foo"}, + DesiredConfig: &v1alpha1.ConfigSpecHash{ + ConfigReferent: v1alpha1.ConfigReferent{Name: "test2", Namespace: "open-cluster-management"}, + SpecHash: "hash2new", + }, + LastAppliedConfig: &v1alpha1.ConfigSpecHash{ + ConfigReferent: v1alpha1.ConfigReferent{Name: "test2", Namespace: "open-cluster-management"}, SpecHash: "", }, }, @@ -255,11 +266,22 @@ func TestReconcile(t *testing.T) { { ConfigGroupResource: v1alpha1.ConfigGroupResource{Group: "core", Resource: "foo"}, DesiredConfig: &v1alpha1.ConfigSpecHash{ - ConfigReferent: v1alpha1.ConfigReferent{Name: "test", Namespace: "open-cluster-management"}, - SpecHash: "hashnew", + ConfigReferent: v1alpha1.ConfigReferent{Name: "test1", Namespace: "open-cluster-management"}, + SpecHash: "hash1new", }, LastAppliedConfig: &v1alpha1.ConfigSpecHash{ - ConfigReferent: v1alpha1.ConfigReferent{Name: "test", Namespace: "open-cluster-management"}, + ConfigReferent: v1alpha1.ConfigReferent{Name: "test1", Namespace: "open-cluster-management"}, + SpecHash: "hash", + }, + }, + { + ConfigGroupResource: v1alpha1.ConfigGroupResource{Group: "core", Resource: "foo"}, + DesiredConfig: &v1alpha1.ConfigSpecHash{ + ConfigReferent: v1alpha1.ConfigReferent{Name: "test2", Namespace: "open-cluster-management"}, + SpecHash: "hash2new", + }, + LastAppliedConfig: &v1alpha1.ConfigSpecHash{ + ConfigReferent: v1alpha1.ConfigReferent{Name: "test2", Namespace: "open-cluster-management"}, SpecHash: "hash", }, }, @@ -395,11 +417,22 @@ func TestReconcile(t *testing.T) { { ConfigGroupResource: v1alpha1.ConfigGroupResource{Group: "core", Resource: "foo"}, DesiredConfig: &v1alpha1.ConfigSpecHash{ - ConfigReferent: v1alpha1.ConfigReferent{Name: "test", Namespace: "open-cluster-management"}, - SpecHash: "hashnew", + ConfigReferent: v1alpha1.ConfigReferent{Name: "test1", Namespace: "open-cluster-management"}, + SpecHash: "hash1new", }, LastAppliedConfig: &v1alpha1.ConfigSpecHash{ - ConfigReferent: v1alpha1.ConfigReferent{Name: "test", Namespace: "open-cluster-management"}, + ConfigReferent: v1alpha1.ConfigReferent{Name: "test1", Namespace: "open-cluster-management"}, + SpecHash: "", + }, + }, + { + ConfigGroupResource: v1alpha1.ConfigGroupResource{Group: "core", Resource: "foo"}, + DesiredConfig: &v1alpha1.ConfigSpecHash{ + ConfigReferent: v1alpha1.ConfigReferent{Name: "test2", Namespace: "open-cluster-management"}, + SpecHash: "hash2new", + }, + LastAppliedConfig: &v1alpha1.ConfigSpecHash{ + ConfigReferent: v1alpha1.ConfigReferent{Name: "test2", Namespace: "open-cluster-management"}, SpecHash: "", }, }, @@ -424,7 +457,8 @@ func TestReconcile(t *testing.T) { addonapiv1alpha1.AddonLabelKey: "test", }) work.SetAnnotations(map[string]string{ - workapiv1.ManifestConfigSpecHashAnnotationKey: "{\"foo.core/open-cluster-management/test\":\"hashnew\"}", + workapiv1.ManifestConfigSpecHashAnnotationKey: "{\"foo.core/open-cluster-management/test1\":\"hash1new\"," + + "\"foo.core/open-cluster-management/test2\":\"hash2new\"}", }) work.Status.Conditions = []metav1.Condition{ { @@ -451,7 +485,7 @@ func TestReconcile(t *testing.T) { if !(configCond != nil && configCond.Reason == addonapiv1alpha1.ProgressingReasonCompleted && configCond.Status == metav1.ConditionFalse) { t.Errorf("Condition Progressing is incorrect") } - if len(addOn.Status.ConfigReferences) != 1 { + if len(addOn.Status.ConfigReferences) != 2 { t.Errorf("ConfigReferences object is not correct: %v", addOn.Status.ConfigReferences) } if addOn.Status.ConfigReferences[0].LastAppliedConfig.SpecHash != addOn.Status.ConfigReferences[0].DesiredConfig.SpecHash { @@ -468,11 +502,22 @@ func TestReconcile(t *testing.T) { { ConfigGroupResource: v1alpha1.ConfigGroupResource{Group: "core", Resource: "foo"}, DesiredConfig: &v1alpha1.ConfigSpecHash{ - ConfigReferent: v1alpha1.ConfigReferent{Name: "test", Namespace: "open-cluster-management"}, - SpecHash: "hashnew", + ConfigReferent: v1alpha1.ConfigReferent{Name: "test1", Namespace: "open-cluster-management"}, + SpecHash: "hash1new", }, LastAppliedConfig: &v1alpha1.ConfigSpecHash{ - ConfigReferent: v1alpha1.ConfigReferent{Name: "test", Namespace: "open-cluster-management"}, + ConfigReferent: v1alpha1.ConfigReferent{Name: "test1", Namespace: "open-cluster-management"}, + SpecHash: "hash", + }, + }, + { + ConfigGroupResource: v1alpha1.ConfigGroupResource{Group: "core", Resource: "foo"}, + DesiredConfig: &v1alpha1.ConfigSpecHash{ + ConfigReferent: v1alpha1.ConfigReferent{Name: "test2", Namespace: "open-cluster-management"}, + SpecHash: "hash2new", + }, + LastAppliedConfig: &v1alpha1.ConfigSpecHash{ + ConfigReferent: v1alpha1.ConfigReferent{Name: "test2", Namespace: "open-cluster-management"}, SpecHash: "hash", }, }, @@ -497,7 +542,8 @@ func TestReconcile(t *testing.T) { addonapiv1alpha1.AddonLabelKey: "test", }) work.SetAnnotations(map[string]string{ - workapiv1.ManifestConfigSpecHashAnnotationKey: "{\"foo.core/open-cluster-management/test\":\"hashnew\"}", + workapiv1.ManifestConfigSpecHashAnnotationKey: "{\"foo.core/open-cluster-management/test1\":\"hash1new\"," + + "\"foo.core/open-cluster-management/test2\":\"hash2new\"}", }) work.Status.Conditions = []metav1.Condition{ { @@ -524,7 +570,7 @@ func TestReconcile(t *testing.T) { if !(configCond != nil && configCond.Reason == addonapiv1alpha1.ProgressingReasonCompleted && configCond.Status == metav1.ConditionFalse) { t.Errorf("Condition Progressing is incorrect") } - if len(addOn.Status.ConfigReferences) != 1 { + if len(addOn.Status.ConfigReferences) != 2 { t.Errorf("ConfigReferences object is not correct: %v", addOn.Status.ConfigReferences) } if addOn.Status.ConfigReferences[0].LastAppliedConfig.SpecHash != addOn.Status.ConfigReferences[0].DesiredConfig.SpecHash { diff --git a/pkg/addon/controllers/cmainstallprogression/controller.go b/pkg/addon/controllers/cmainstallprogression/controller.go index 597d4adae..bff1fdc21 100644 --- a/pkg/addon/controllers/cmainstallprogression/controller.go +++ b/pkg/addon/controllers/cmainstallprogression/controller.go @@ -2,10 +2,12 @@ package cmainstallprogression import ( "context" + "sort" "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/events" "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/klog/v2" addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" @@ -131,30 +133,45 @@ func setInstallProgression(supportedConfigs []addonv1alpha1.ConfigMeta, placemen } // set config references as default configuration - installConfigReferences := []addonv1alpha1.InstallConfigReference{} - installConfigReferencesMap := map[addonv1alpha1.ConfigGroupResource]addonv1alpha1.ConfigReferent{} + installConfigReferencesMap := map[addonv1alpha1.ConfigGroupResource]sets.Set[addonv1alpha1.ConfigReferent]{} for _, config := range supportedConfigs { if config.DefaultConfig != nil { - installConfigReferencesMap[config.ConfigGroupResource] = *config.DefaultConfig + installConfigReferencesMap[config.ConfigGroupResource] = sets.New[addonv1alpha1.ConfigReferent](*config.DefaultConfig) } } // override the default configuration for each placement - for _, config := range placementStrategy.Configs { - installConfigReferencesMap[config.ConfigGroupResource] = config.ConfigReferent + overrideConfigMapByAddOnConfigs(installConfigReferencesMap, placementStrategy.Configs) + + // sort gvk + gvks := []addonv1alpha1.ConfigGroupResource{} + for gvk := range installConfigReferencesMap { + gvks = append(gvks, gvk) } + sort.Slice(gvks, func(i, j int) bool { + if gvks[i].Group == gvks[j].Group { + return gvks[i].Resource < gvks[j].Resource + } + return gvks[i].Group < gvks[j].Group + }) // set the config references for each install progression - for k, v := range installConfigReferencesMap { - installConfigReferences = append(installConfigReferences, - addonv1alpha1.InstallConfigReference{ - ConfigGroupResource: k, - DesiredConfig: &addonv1alpha1.ConfigSpecHash{ - ConfigReferent: v, - }, - }, - ) + installConfigReferences := []addonv1alpha1.InstallConfigReference{} + for _, gvk := range gvks { + if configRefs, ok := installConfigReferencesMap[gvk]; ok { + for configRef := range configRefs { + installConfigReferences = append(installConfigReferences, + addonv1alpha1.InstallConfigReference{ + ConfigGroupResource: gvk, + DesiredConfig: &addonv1alpha1.ConfigSpecHash{ + ConfigReferent: configRef, + }, + }, + ) + } + } } + installProgression.ConfigReferences = installConfigReferences // if the config group resource already exists in status, merge the install progression @@ -172,7 +189,7 @@ func findInstallProgression(newobj *addonv1alpha1.InstallProgression, oldobjs [] count := 0 for _, oldconfig := range oldobj.ConfigReferences { for _, newconfig := range newobj.ConfigReferences { - if oldconfig.ConfigGroupResource == newconfig.ConfigGroupResource { + if oldconfig.ConfigGroupResource == newconfig.ConfigGroupResource && oldconfig.DesiredConfig.ConfigReferent == newconfig.DesiredConfig.ConfigReferent { count += 1 } } @@ -189,8 +206,9 @@ func mergeInstallProgression(newobj, oldobj *addonv1alpha1.InstallProgression) { // merge config reference for i := range newobj.ConfigReferences { for _, oldconfig := range oldobj.ConfigReferences { - if newobj.ConfigReferences[i].ConfigGroupResource == oldconfig.ConfigGroupResource { - if newobj.ConfigReferences[i].DesiredConfig.ConfigReferent == oldconfig.DesiredConfig.ConfigReferent { + if newobj.ConfigReferences[i].ConfigGroupResource == oldconfig.ConfigGroupResource && + newobj.ConfigReferences[i].DesiredConfig.ConfigReferent == oldconfig.DesiredConfig.ConfigReferent { + if oldconfig.DesiredConfig.SpecHash != "" { newobj.ConfigReferences[i].DesiredConfig.SpecHash = oldconfig.DesiredConfig.SpecHash } newobj.ConfigReferences[i].LastAppliedConfig = oldconfig.LastAppliedConfig.DeepCopy() @@ -200,3 +218,25 @@ func mergeInstallProgression(newobj, oldobj *addonv1alpha1.InstallProgression) { } newobj.Conditions = oldobj.Conditions } + +// Override the desired configs by a slice of AddOnConfig (from cma install strategy), +func overrideConfigMapByAddOnConfigs( + desiredConfigs map[addonv1alpha1.ConfigGroupResource]sets.Set[addonv1alpha1.ConfigReferent], + addOnConfigs []addonv1alpha1.AddOnConfig, +) { + gvkOverwritten := sets.New[addonv1alpha1.ConfigGroupResource]() + // Go through the cma install strategy configs, + // for a group of configs with same gvk, install strategy configs override the desiredConfigs. + for _, config := range addOnConfigs { + gr := config.ConfigGroupResource + if !gvkOverwritten.Has(gr) { + desiredConfigs[gr] = sets.New[addonv1alpha1.ConfigReferent]() + gvkOverwritten.Insert(gr) + } + // If a config not exist in the desiredConfigs, append it. + // This is to avoid adding duplicate configs (name + namespace). + if !desiredConfigs[gr].Has(config.ConfigReferent) { + desiredConfigs[gr].Insert(config.ConfigReferent) + } + } +} diff --git a/pkg/addon/controllers/cmainstallprogression/controller_test.go b/pkg/addon/controllers/cmainstallprogression/controller_test.go index 4f39e51c9..cc5477da8 100644 --- a/pkg/addon/controllers/cmainstallprogression/controller_test.go +++ b/pkg/addon/controllers/cmainstallprogression/controller_test.go @@ -87,7 +87,7 @@ func TestReconcile(t *testing.T) { validateAddonActions: addontesting.AssertNoActions, }, { - name: "update clustermanagementaddon status with type placements", + name: "update clustermanagementaddon status with type placements with multiple same-GVK", syncKey: "test", managedClusteraddon: []runtime.Object{}, clusterManagementAddon: []runtime.Object{addontesting.NewClusterManagementAddon("test", "testcrd", "testcr").WithPlacementStrategy( @@ -115,6 +115,62 @@ func TestReconcile(t *testing.T) { }, }, }, + addonapiv1alpha1.PlacementStrategy{ + PlacementRef: addonapiv1alpha1.PlacementRef{ + Name: "placement3", + Namespace: "test", + }, + Configs: []addonapiv1alpha1.AddOnConfig{ + { + ConfigGroupResource: addonapiv1alpha1.ConfigGroupResource{ + Group: "addon.open-cluster-management.io", + Resource: "addondeploymentconfigs", + }, + ConfigReferent: addonapiv1alpha1.ConfigReferent{ + Name: "test", + Namespace: "test", + }, + }, + { + ConfigGroupResource: addonapiv1alpha1.ConfigGroupResource{ + Group: "addon.open-cluster-management.io", + Resource: "addondeploymentconfigs", + }, + ConfigReferent: addonapiv1alpha1.ConfigReferent{ + Name: "test", + Namespace: "test1", + }, + }, + }, + }, + addonapiv1alpha1.PlacementStrategy{ + PlacementRef: addonapiv1alpha1.PlacementRef{ + Name: "placement4", + Namespace: "test", + }, + Configs: []addonapiv1alpha1.AddOnConfig{ + { + ConfigGroupResource: addonapiv1alpha1.ConfigGroupResource{ + Group: "addon.open-cluster-management.io", + Resource: "addondeploymentconfigs", + }, + ConfigReferent: addonapiv1alpha1.ConfigReferent{ + Name: "test", + Namespace: "test", + }, + }, + { + ConfigGroupResource: addonapiv1alpha1.ConfigGroupResource{ + Group: "addon.open-cluster-management.io", + Resource: "addondeploymentconfigs", + }, + ConfigReferent: addonapiv1alpha1.ConfigReferent{ + Name: "test", + Namespace: "test", + }, + }, + }, + }, ).Build()}, validateAddonActions: func(t *testing.T, actions []clienttesting.Action) { addontesting.AssertActions(t, actions, "patch") @@ -128,7 +184,7 @@ func TestReconcile(t *testing.T) { if len(cma.Status.DefaultConfigReferences) != 0 { t.Errorf("DefaultConfigReferences object is not correct: %v", cma.Status.DefaultConfigReferences) } - if len(cma.Status.InstallProgressions) != 2 { + if len(cma.Status.InstallProgressions) != 4 { t.Errorf("InstallProgressions object is not correct: %v", cma.Status.InstallProgressions) } if len(cma.Status.InstallProgressions[0].ConfigReferences) != 0 { @@ -137,11 +193,16 @@ func TestReconcile(t *testing.T) { if len(cma.Status.InstallProgressions[1].ConfigReferences) != 1 { t.Errorf("InstallProgressions ConfigReferences object is not correct: %v", cma.Status.InstallProgressions[0].ConfigReferences) } - + if len(cma.Status.InstallProgressions[2].ConfigReferences) != 2 { + t.Errorf("InstallProgressions ConfigReferences object is not correct: %v", cma.Status.InstallProgressions[0].ConfigReferences) + } + if len(cma.Status.InstallProgressions[3].ConfigReferences) != 1 { + t.Errorf("InstallProgressions ConfigReferences object is not correct: %v", cma.Status.InstallProgressions[0].ConfigReferences) + } }, }, { - name: "update clustermanagementaddon status with type placements and default configs", + name: "update clustermanagementaddon status with type placements with multiple same-GVK and default configs", syncKey: "test", managedClusteraddon: []runtime.Object{}, clusterManagementAddon: []runtime.Object{addontesting.NewClusterManagementAddon("test", "testcrd", "testcr").WithPlacementStrategy( @@ -168,6 +229,34 @@ func TestReconcile(t *testing.T) { Name: "placement2", Namespace: "test", }, + Configs: []addonapiv1alpha1.AddOnConfig{ + { + ConfigGroupResource: addonapiv1alpha1.ConfigGroupResource{ + Group: "addon.open-cluster-management.io", + Resource: "addonhubconfigs", + }, + ConfigReferent: addonapiv1alpha1.ConfigReferent{ + Name: "test1", + Namespace: "test", + }, + }, + { + ConfigGroupResource: addonapiv1alpha1.ConfigGroupResource{ + Group: "addon.open-cluster-management.io", + Resource: "addonhubconfigs", + }, + ConfigReferent: addonapiv1alpha1.ConfigReferent{ + Name: "test2", + Namespace: "test", + }, + }, + }, + }, + addonapiv1alpha1.PlacementStrategy{ + PlacementRef: addonapiv1alpha1.PlacementRef{ + Name: "placement3", + Namespace: "test", + }, Configs: []addonapiv1alpha1.AddOnConfig{ { ConfigGroupResource: addonapiv1alpha1.ConfigGroupResource{ @@ -175,7 +264,17 @@ func TestReconcile(t *testing.T) { Resource: "addondeploymentconfigs", }, ConfigReferent: addonapiv1alpha1.ConfigReferent{ - Name: "test", + Name: "test3", + Namespace: "test", + }, + }, + { + ConfigGroupResource: addonapiv1alpha1.ConfigGroupResource{ + Group: "addon.open-cluster-management.io", + Resource: "addondeploymentconfigs", + }, + ConfigReferent: addonapiv1alpha1.ConfigReferent{ + Name: "test4", Namespace: "test", }, }, @@ -204,7 +303,7 @@ func TestReconcile(t *testing.T) { if len(cma.Status.DefaultConfigReferences) != 1 { t.Errorf("DefaultConfigReferences object is not correct: %v", cma.Status.DefaultConfigReferences) } - if len(cma.Status.InstallProgressions) != 2 { + if len(cma.Status.InstallProgressions) != 3 { t.Errorf("InstallProgressions object is not correct: %v", cma.Status.InstallProgressions) } if len(cma.Status.InstallProgressions[0].ConfigReferences) != 1 { @@ -216,6 +315,9 @@ func TestReconcile(t *testing.T) { if len(cma.Status.InstallProgressions[1].ConfigReferences) != 2 { t.Errorf("InstallProgressions ConfigReferences object is not correct: %v", cma.Status.InstallProgressions[0].ConfigReferences) } + if len(cma.Status.InstallProgressions[2].ConfigReferences) != 3 { + t.Errorf("InstallProgressions ConfigReferences object is not correct: %v", cma.Status.InstallProgressions[0].ConfigReferences) + } }, }, } diff --git a/test/integration/addon/addon_manager_upgrade_test.go b/test/integration/addon/addon_manager_upgrade_test.go index 8d0a9a4ce..17d9ff8e4 100644 --- a/test/integration/addon/addon_manager_upgrade_test.go +++ b/test/integration/addon/addon_manager_upgrade_test.go @@ -396,13 +396,6 @@ var _ = ginkgo.Describe("Addon upgrade", func() { }, SpecHash: addOnTest2ConfigSpecHash, }, - LastAppliedConfig: &addonapiv1alpha1.ConfigSpecHash{ - ConfigReferent: addonapiv1alpha1.ConfigReferent{ - Namespace: configDefaultNamespace, - Name: configDefaultName, - }, - SpecHash: addOnTest1ConfigSpecHash, - }, }) assertManagedClusterAddOnConditions(testAddOnConfigsImpl.name, clusterNames[i], metav1.Condition{ Type: addonapiv1alpha1.ManagedClusterAddOnConditionProgressing, @@ -461,20 +454,6 @@ var _ = ginkgo.Describe("Addon upgrade", func() { }, SpecHash: addOnTest2ConfigSpecHash, }, - LastAppliedConfig: &addonapiv1alpha1.ConfigSpecHash{ - ConfigReferent: addonapiv1alpha1.ConfigReferent{ - Namespace: configDefaultNamespace, - Name: configDefaultName, - }, - SpecHash: addOnTest1ConfigSpecHash, - }, - LastKnownGoodConfig: &addonapiv1alpha1.ConfigSpecHash{ - ConfigReferent: addonapiv1alpha1.ConfigReferent{ - Namespace: configDefaultNamespace, - Name: configDefaultName, - }, - SpecHash: addOnTest1ConfigSpecHash, - }, }, }, })