Skip to content

Commit 6d68522

Browse files
authored
fix the mca override cma configs issues (#649)
Signed-off-by: haoqing0110 <qhao@redhat.com>
1 parent 33b409d commit 6d68522

File tree

2 files changed

+91
-14
lines changed

2 files changed

+91
-14
lines changed

pkg/addon/controllers/addonconfiguration/addon_configuration_reconciler_test.go

+76-8
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,45 @@ func TestAddonConfigReconcile(t *testing.T) {
312312
},
313313
}, nil, nil),
314314
addontesting.NewAddon("test", "cluster2"),
315+
// cluster3 already has configs synced to status before spec hash is updated
316+
newManagedClusterAddon("test", "cluster3", []addonv1alpha1.AddOnConfig{
317+
{
318+
ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"},
319+
ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"},
320+
},
321+
{
322+
ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"},
323+
ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test3"},
324+
},
325+
}, []addonv1alpha1.ConfigReference{
326+
{
327+
ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"},
328+
ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"},
329+
DesiredConfig: &addonv1alpha1.ConfigSpecHash{
330+
ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"},
331+
SpecHash: "",
332+
},
333+
LastObservedGeneration: 0,
334+
},
335+
{
336+
ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"},
337+
ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"},
338+
DesiredConfig: &addonv1alpha1.ConfigSpecHash{
339+
ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"},
340+
SpecHash: "",
341+
},
342+
LastObservedGeneration: 0,
343+
},
344+
{
345+
ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"},
346+
ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test3"},
347+
DesiredConfig: &addonv1alpha1.ConfigSpecHash{
348+
ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test3"},
349+
SpecHash: "",
350+
},
351+
LastObservedGeneration: 0,
352+
},
353+
}, nil),
315354
},
316355
placements: []runtime.Object{
317356
&clusterv1beta1.Placement{ObjectMeta: metav1.ObjectMeta{Name: "test-placement", Namespace: "default"}},
@@ -327,7 +366,7 @@ func TestAddonConfigReconcile(t *testing.T) {
327366
},
328367
},
329368
Status: clusterv1beta1.PlacementDecisionStatus{
330-
Decisions: []clusterv1beta1.ClusterDecision{{ClusterName: "cluster1"}, {ClusterName: "cluster2"}},
369+
Decisions: []clusterv1beta1.ClusterDecision{{ClusterName: "cluster1"}, {ClusterName: "cluster2"}, {ClusterName: "cluster3"}},
331370
},
332371
},
333372
},
@@ -343,7 +382,7 @@ func TestAddonConfigReconcile(t *testing.T) {
343382
ConfigGroupResource: v1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"},
344383
DesiredConfig: &v1alpha1.ConfigSpecHash{
345384
ConfigReferent: v1alpha1.ConfigReferent{Name: "test"},
346-
SpecHash: "hash",
385+
SpecHash: "<core-foo-test-hash>",
347386
},
348387
}).WithPlacementStrategy(addonv1alpha1.PlacementStrategy{
349388
PlacementRef: addonv1alpha1.PlacementRef{Name: "test-placement", Namespace: "default"},
@@ -355,28 +394,28 @@ func TestAddonConfigReconcile(t *testing.T) {
355394
ConfigGroupResource: v1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"},
356395
DesiredConfig: &v1alpha1.ConfigSpecHash{
357396
ConfigReferent: v1alpha1.ConfigReferent{Name: "test1"},
358-
SpecHash: "hash1",
397+
SpecHash: "<core-bar-test1-hash>",
359398
},
360399
},
361400
{
362401
ConfigGroupResource: v1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"},
363402
DesiredConfig: &v1alpha1.ConfigSpecHash{
364403
ConfigReferent: v1alpha1.ConfigReferent{Name: "test1"},
365-
SpecHash: "hash1",
404+
SpecHash: "<core-foo-test1-hash>",
366405
},
367406
},
368407
},
369408
}).Build(),
370409
validateAddonActions: func(t *testing.T, actions []clienttesting.Action) {
371-
addontesting.AssertActions(t, actions, "patch", "patch")
410+
addontesting.AssertActions(t, actions, "patch", "patch", "patch")
372411
sort.Sort(byPatchName(actions))
373412
expectPatchConfigurationAction(t, actions[0], []addonv1alpha1.ConfigReference{
374413
{
375414
ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"},
376415
ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"},
377416
DesiredConfig: &addonv1alpha1.ConfigSpecHash{
378417
ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"},
379-
SpecHash: "hash1",
418+
SpecHash: "<core-bar-test1-hash>",
380419
},
381420
LastObservedGeneration: 0,
382421
},
@@ -405,7 +444,7 @@ func TestAddonConfigReconcile(t *testing.T) {
405444
ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"},
406445
DesiredConfig: &addonv1alpha1.ConfigSpecHash{
407446
ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"},
408-
SpecHash: "hash1",
447+
SpecHash: "<core-bar-test1-hash>",
409448
},
410449
LastObservedGeneration: 0,
411450
},
@@ -414,10 +453,39 @@ func TestAddonConfigReconcile(t *testing.T) {
414453
ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"},
415454
DesiredConfig: &addonv1alpha1.ConfigSpecHash{
416455
ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"},
417-
SpecHash: "hash1",
456+
SpecHash: "<core-foo-test1-hash>",
418457
},
419458
LastObservedGeneration: 0,
420459
}})
460+
expectPatchConfigurationAction(t, actions[2], []addonv1alpha1.ConfigReference{
461+
{
462+
ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Bar"},
463+
ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"},
464+
DesiredConfig: &addonv1alpha1.ConfigSpecHash{
465+
ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test1"},
466+
SpecHash: "<core-bar-test1-hash>",
467+
},
468+
LastObservedGeneration: 0,
469+
},
470+
{
471+
ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"},
472+
ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"},
473+
DesiredConfig: &addonv1alpha1.ConfigSpecHash{
474+
ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test2"},
475+
SpecHash: "",
476+
},
477+
LastObservedGeneration: 0,
478+
},
479+
{
480+
ConfigGroupResource: addonv1alpha1.ConfigGroupResource{Group: "core", Resource: "Foo"},
481+
ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test3"},
482+
DesiredConfig: &addonv1alpha1.ConfigSpecHash{
483+
ConfigReferent: addonv1alpha1.ConfigReferent{Name: "test3"},
484+
SpecHash: "",
485+
},
486+
LastObservedGeneration: 0,
487+
},
488+
})
421489
expectPatchConditionAction(t, actions[0], metav1.ConditionTrue)
422490
expectPatchConditionAction(t, actions[1], metav1.ConditionTrue)
423491
},

pkg/addon/controllers/addonconfiguration/graph.go

+15-6
Original file line numberDiff line numberDiff line change
@@ -369,12 +369,21 @@ func (n *installStrategyNode) addNode(addon *addonv1alpha1.ManagedClusterAddOn)
369369
// TODO we should also filter out the configs which are not supported configs.
370370
overrideConfigMapByAddOnConfigs(n.children[addon.Namespace].desiredConfigs, addon.Spec.Configs)
371371

372-
// copy the spechash from mca status
373-
for _, configRef := range addon.Status.ConfigReferences {
374-
if configRef.DesiredConfig == nil {
375-
continue
376-
}
377-
if nodeDesiredConfigArray, ok := n.children[addon.Namespace].desiredConfigs[configRef.ConfigGroupResource]; ok {
372+
// go through mca spec configs and copy the specHash from status if they match
373+
for _, config := range addon.Spec.Configs {
374+
for _, configRef := range addon.Status.ConfigReferences {
375+
if configRef.DesiredConfig == nil {
376+
continue
377+
}
378+
// compare the ConfigGroupResource and ConfigReferent
379+
if configRef.ConfigGroupResource != config.ConfigGroupResource || configRef.DesiredConfig.ConfigReferent != config.ConfigReferent {
380+
continue
381+
}
382+
// copy the spec hash to desired configs
383+
nodeDesiredConfigArray, ok := n.children[addon.Namespace].desiredConfigs[configRef.ConfigGroupResource]
384+
if !ok {
385+
continue
386+
}
378387
for i, nodeDesiredConfig := range nodeDesiredConfigArray {
379388
if nodeDesiredConfig.DesiredConfig.ConfigReferent == configRef.DesiredConfig.ConfigReferent {
380389
n.children[addon.Namespace].desiredConfigs[configRef.ConfigGroupResource][i].DesiredConfig.SpecHash = configRef.DesiredConfig.SpecHash

0 commit comments

Comments
 (0)