Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ addon: add support for multiple GVK #585

Merged
merged 3 commits into from
Aug 13, 2024
Merged

✨ addon: add support for multiple GVK #585

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b5a03dc
addon: add support for multiple GVK
JoaoBraveCoding Jul 2, 2024
4c7ce51
add more UT
haoqing0110 Jul 30, 2024
ca344f4
modify code
haoqing0110 Aug 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 31 additions & 22 deletions pkg/addon/controllers/addonconfiguration/addon_configuration_reconciler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add some comments for each for loop

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added.

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
}
280 changes: 272 additions & 8 deletions pkg/addon/controllers/addonconfiguration/addon_configuration_reconciler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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"},
Expand All @@ -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{
Expand All @@ -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"},
Expand Down Expand Up @@ -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),
},
Expand Down Expand Up @@ -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),
},
Expand Down Expand Up @@ -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,
}})
},
Expand Down
Loading
Loading