Skip to content

Commit b99e2e5

Browse files
zhujian7claude
andcommitted
Refactor template-based addon handling to avoid breaking interface changes
- Move templateBasedAddOn parameter from interface to concrete implementation - Add SetTemplateBasedAddOn() method to BaseAddonManager interface - Add templateBasedAddOn field to BaseAddonManagerImpl struct - Revert StartWithInformers interface to original signature - Update all callers to use the original interface method - Maintain backward compatibility while providing new functionality This allows users to configure template-based addon handling through the public API without breaking existing external implementations: manager, _ := addonmanager.New(config) manager.SetTemplateBasedAddOn(true) // Enable template filtering manager.StartWithInformers(ctx, ...) // Uses configured setting 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> Signed-off-by: zhujian <[email protected]>
1 parent a0000a9 commit b99e2e5

File tree

4 files changed

+25
-19
lines changed

4 files changed

+25
-19
lines changed

pkg/addonmanager/base_manager.go

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,11 @@ import (
2929
// BaseAddonManagerImpl is the base implementation of BaseAddonManager
3030
// that manages the addon agents and configs.
3131
type BaseAddonManagerImpl struct {
32-
addonAgents map[string]agent.AgentAddon
33-
addonConfigs map[schema.GroupVersionResource]bool
34-
config *rest.Config
35-
syncContexts []factory.SyncContext
32+
addonAgents map[string]agent.AgentAddon
33+
addonConfigs map[schema.GroupVersionResource]bool
34+
config *rest.Config
35+
syncContexts []factory.SyncContext
36+
templateBasedAddOn bool
3637
}
3738

3839
// NewBaseAddonManagerImpl creates a new BaseAddonManagerImpl instance with the given config.
@@ -71,18 +72,30 @@ func (a *BaseAddonManagerImpl) Trigger(clusterName, addonName string) {
7172
}
7273
}
7374

75+
// SetTemplateBasedAddOn configures whether the manager is handling template-based addons.
76+
// - true: all ManagedClusterAddOn controllers except "addon-config-controller" will only process addons
77+
// when the referenced AddOnTemplate resources in their status.configReferences are properly set;
78+
// the "addon-config-controller" is responsible for setting these values
79+
// - false: process all addons without waiting for template configuration
80+
//
81+
// This prevents premature processing of template-based addons before their configurations
82+
// are fully ready, avoiding unnecessary errors and retries.
83+
// See https://github.com/open-cluster-management-io/ocm/issues/1181 for more context.
84+
func (a *BaseAddonManagerImpl) SetTemplateBasedAddOn(templateBasedAddOn bool) {
85+
a.templateBasedAddOn = templateBasedAddOn
86+
}
87+
7488
func (a *BaseAddonManagerImpl) StartWithInformers(ctx context.Context,
7589
workClient workclientset.Interface,
7690
workInformers workv1informers.ManifestWorkInformer,
7791
kubeInformers kubeinformers.SharedInformerFactory,
7892
addonInformers addoninformers.SharedInformerFactory,
7993
clusterInformers clusterv1informers.SharedInformerFactory,
8094
dynamicInformers dynamicinformer.DynamicSharedInformerFactory,
81-
templateBasedAddOn bool,
8295
) error {
83-
// Determine the appropriate filter function based on templateBasedAddOn flag
96+
// Determine the appropriate filter function based on templateBasedAddOn field
8497
mcaFilterFunc := utils.AllowAllAddOns
85-
if templateBasedAddOn {
98+
if a.templateBasedAddOn {
8699
mcaFilterFunc = utils.FilterTemplateBasedAddOns
87100
}
88101

pkg/addonmanager/cloudevents/manager.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ func (a *cloudeventsAddonManager) Start(ctx context.Context) error {
177177
}
178178

179179
err = a.StartWithInformers(ctx, workClient, workInformers, kubeInformers, addonInformers, clusterInformers,
180-
dynamicInformers, false)
180+
dynamicInformers)
181181
if err != nil {
182182
return err
183183
}

pkg/addonmanager/interface.go

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,24 +22,17 @@ type BaseAddonManager interface {
2222
// only trigger the deploy controller.
2323
Trigger(clusterName, addonName string)
2424

25+
// SetTemplateBasedAddOn configures whether the manager is handling template-based addons.
26+
SetTemplateBasedAddOn(templateBasedAddOn bool)
27+
2528
// StartWithInformers starts all registered addon agent with the given informers.
26-
//
27-
// templateBasedAddOn controls whether the manager is handling template-based addons:
28-
// - true: all ManagedClusterAddOn controllers except "addon-config-controller" will only process addons
29-
// when the referenced AddOnTemplate resources in their status.configReferences are properly set;
30-
// the "addon-config-controller" is responsible for setting these values
31-
// - false: process all addons without waiting for template configuration
32-
// This filtering prevents premature processing of template-based addons before their configurations
33-
// are fully ready, avoiding unnecessary errors and retries.
34-
// See https://github.com/open-cluster-management-io/ocm/issues/1181 for more context.
3529
StartWithInformers(ctx context.Context,
3630
workClient workclientset.Interface,
3731
workInformers workv1informers.ManifestWorkInformer,
3832
kubeInformers kubeinformers.SharedInformerFactory,
3933
addonInformers addoninformers.SharedInformerFactory,
4034
clusterInformers clusterv1informers.SharedInformerFactory,
4135
dynamicInformers dynamicinformer.DynamicSharedInformerFactory,
42-
templateBasedAddOn bool,
4336
) error
4437
}
4538

pkg/addonmanager/manager.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ func (a *addonManager) Start(ctx context.Context) error {
123123
}
124124

125125
err = a.StartWithInformers(ctx, workClient, workInformers.Work().V1().ManifestWorks(), kubeInformers,
126-
addonInformers, clusterInformers, dynamicInformers, false)
126+
addonInformers, clusterInformers, dynamicInformers)
127127
if err != nil {
128128
return err
129129
}

0 commit comments

Comments
 (0)