diff --git a/pkg/addon/controllers/addonconfiguration/addon_configuration_reconciler.go b/pkg/addon/controllers/addonconfiguration/addon_configuration_reconciler.go index 084557307..25ba2c2b4 100644 --- a/pkg/addon/controllers/addonconfiguration/addon_configuration_reconciler.go +++ b/pkg/addon/controllers/addonconfiguration/addon_configuration_reconciler.go @@ -47,7 +47,9 @@ func (d *managedClusterAddonConfigurationReconciler) mergeAddonConfig( mcaCopy := mca.DeepCopy() mergedConfigs := make(addonConfigMap) - // remove configs that are not desired + // 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 { @@ -58,7 +60,10 @@ func (d *managedClusterAddonConfigurationReconciler) mergeAddonConfig( } } - // append or update configs + // 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 { diff --git a/pkg/addon/controllers/addonconfiguration/graph.go b/pkg/addon/controllers/addonconfiguration/graph.go index 27d97d9a4..561a7f069 100644 --- a/pkg/addon/controllers/addonconfiguration/graph.go +++ b/pkg/addon/controllers/addonconfiguration/graph.go @@ -103,10 +103,16 @@ func (n *addonNode) setRolloutStatus() { } func (m addonConfigMap) copy() addonConfigMap { - output := make(addonConfigMap) + output := make(addonConfigMap, len(m)) for k, v := range m { - output[k] = make([]addonv1alpha1.ConfigReference, len(v)) - copy(output[k], v) + // 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 } @@ -254,7 +260,7 @@ func (g *configurationGraph) addPlacementNode( // overrides configuration by install strategy if len(installConfigReference) > 0 { node.desiredConfigs = node.desiredConfigs.copy() - overrideConfigMapByInstallConfigRef(installConfigReference, node.desiredConfigs) + overrideConfigMapByInstallConfigRef(node.desiredConfigs, installConfigReference) } // remove addon in defaults and other placements. @@ -329,7 +335,7 @@ 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. - overrideConfigMapByAddOnConfigs(addon.Spec.Configs, n.children[addon.Namespace].desiredConfigs) + overrideConfigMapByAddOnConfigs(n.children[addon.Namespace].desiredConfigs, addon.Spec.Configs) // copy the spechash from mca status for _, configRef := range addon.Status.ConfigReferences { @@ -496,21 +502,26 @@ func rolloutStatusHasCluster(clusterRolloutStatus []clustersdkv1alpha1.ClusterRo return false } +// Override the desired addonConfigMap by a slice of InstallConfigReference (from cma status), func overrideConfigMapByInstallConfigRef( - installConfigRefs []addonv1alpha1.InstallConfigReference, desiredConfigs addonConfigMap, + installConfigRefs []addonv1alpha1.InstallConfigReference, ) { - gvkOverwritten := make(map[addonv1alpha1.ConfigGroupResource]struct{}) + 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 _, exist := gvkOverwritten[gr]; !exist { + if !gvkOverwritten.Has(gr) { desiredConfigs[gr] = []addonv1alpha1.ConfigReference{} - gvkOverwritten[gr] = struct{}{} + 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, @@ -521,18 +532,23 @@ func overrideConfigMapByInstallConfigRef( } } +// Override the desired addonConfigMap by a slice of InstallConfigReference (from mca spec), func overrideConfigMapByAddOnConfigs( - addOnConfigs []addonv1alpha1.AddOnConfig, desiredConfigs addonConfigMap, + addOnConfigs []addonv1alpha1.AddOnConfig, ) { - gvkOverwritten := make(map[addonv1alpha1.ConfigGroupResource]struct{}) + 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 _, exist := gvkOverwritten[gr]; !exist { + if !gvkOverwritten.Has(gr) { desiredConfigs[gr] = []addonv1alpha1.ConfigReference{} - gvkOverwritten[gr] = struct{}{} + 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, diff --git a/pkg/addon/controllers/cmainstallprogression/controller.go b/pkg/addon/controllers/cmainstallprogression/controller.go index b9ccc5f3a..bff1fdc21 100644 --- a/pkg/addon/controllers/cmainstallprogression/controller.go +++ b/pkg/addon/controllers/cmainstallprogression/controller.go @@ -7,6 +7,7 @@ import ( "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" @@ -132,15 +133,15 @@ func setInstallProgression(supportedConfigs []addonv1alpha1.ConfigMeta, placemen } // set config references as default configuration - 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] = []addonv1alpha1.ConfigReferent{*config.DefaultConfig} + installConfigReferencesMap[config.ConfigGroupResource] = sets.New[addonv1alpha1.ConfigReferent](*config.DefaultConfig) } } // override the default configuration for each placement - overrideConfigMapByAddOnConfigs(placementStrategy.Configs, installConfigReferencesMap) + overrideConfigMapByAddOnConfigs(installConfigReferencesMap, placementStrategy.Configs) // sort gvk gvks := []addonv1alpha1.ConfigGroupResource{} @@ -158,7 +159,7 @@ func setInstallProgression(supportedConfigs []addonv1alpha1.ConfigMeta, placemen installConfigReferences := []addonv1alpha1.InstallConfigReference{} for _, gvk := range gvks { if configRefs, ok := installConfigReferencesMap[gvk]; ok { - for _, configRef := range configRefs { + for configRef := range configRefs { installConfigReferences = append(installConfigReferences, addonv1alpha1.InstallConfigReference{ ConfigGroupResource: gvk, @@ -218,27 +219,24 @@ 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, - desiredConfigs map[addonv1alpha1.ConfigGroupResource][]addonv1alpha1.ConfigReferent, ) { - gvkOverwritten := make(map[addonv1alpha1.ConfigGroupResource]struct{}) + 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 _, exists := gvkOverwritten[gr]; !exists { - desiredConfigs[gr] = []addonv1alpha1.ConfigReferent{} - gvkOverwritten[gr] = struct{}{} + if !gvkOverwritten.Has(gr) { + desiredConfigs[gr] = sets.New[addonv1alpha1.ConfigReferent]() + gvkOverwritten.Insert(gr) } - // check and avoid adding duplicate configs - var exist bool - for _, configReferent := range desiredConfigs[gr] { - if configReferent == config.ConfigReferent { - exist = true - break - } - } - if !exist { - desiredConfigs[gr] = append(desiredConfigs[gr], config.ConfigReferent) + // 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) } } }