Skip to content

Add custom informer for ManagedClusterAddOns #73

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

Merged
merged 5 commits into from
Oct 16, 2025
Merged

Add custom informer for ManagedClusterAddOns #73

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b6b7e9e
Add custom informer for ManagedClusterAddOns
fxiang1 Oct 16, 2025
d95aa87
Add error handling
fxiang1 Oct 16, 2025
77d8a6c
Improve error handling
fxiang1 Oct 16, 2025
cc57a26
Handle error
fxiang1 Oct 16, 2025
0e04278
Handle event handler error
fxiang1 Oct 16, 2025
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
152 changes: 79 additions & 73 deletions controllers/clusterpermission_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,8 @@ import (
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/util/retry"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

corev1 "k8s.io/api/core/v1"
Expand All @@ -54,43 +50,94 @@ const VALIDATION_MW_RETRY_INTERVAL = 10 * time.Second
type ClusterPermissionReconciler struct {
client.Client
Scheme *runtime.Scheme
// customInformer is the custom informer for ManagedClusterAddOn resources
customInformer *ManagedClusterAddOnInformer
}

// SetupWithManager sets up the controller with the Manager.
func (r *ClusterPermissionReconciler) SetupWithManager(mgr ctrl.Manager) error {
// Create custom informer for ManagedClusterAddOn resources
config := mgr.GetConfig()
eventHandler := r.createCustomInformerEventHandler()

customInformer, err := NewManagedClusterAddOnInformer(config, eventHandler)
if err != nil {
return err
}

// Store the custom informer in the reconciler
r.customInformer = customInformer

// Start the custom informer in a goroutine
go func() {
if err := customInformer.Start(); err != nil {
log.Log.Error(err, "Failed to start custom ManagedClusterAddOn informer")
Copy link
Member

@mikeshng mikeshng Oct 16, 2025

Choose a reason for hiding this comment

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

We might need to exit/panic here or mca won't be watched and it will be hard to detect afterwards.
Does it make sense to retry a few times then exit out?

Copy link
Collaborator Author

@fxiang1 fxiang1 Oct 16, 2025

Choose a reason for hiding this comment

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

Thanks Mike! Yes, the AI suggested to retry.

Yes, it makes sense to add retry logic here! The informer's Start() method can fail if the cache doesn't sync, which could happen due to:
1. Temporary API server unavailability during startup
2. Network issues
3. The ManagedClusterAddOn CRD not being installed yet

}
}()

// Setup the controller without the built-in ManagedClusterAddOn watching
// since we're using the custom informer instead
return ctrl.NewControllerManagedBy(mgr).
For(&cpv1alpha1.ClusterPermission{}).
Watches(&addonv1alpha1.ManagedClusterAddOn{},
r.managedClusterAddOnEventHandler(),
builder.WithPredicates(predicate.Funcs{
CreateFunc: func(e event.CreateEvent) bool {
return false // Don't process Create events
},
UpdateFunc: func(e event.UpdateEvent) bool {
// Only process Update events for managed-serviceaccount addon when status.namespace changes
oldAddon, oldOk := e.ObjectOld.(*addonv1alpha1.ManagedClusterAddOn)
newAddon, newOk := e.ObjectNew.(*addonv1alpha1.ManagedClusterAddOn)
Complete(r)
}

if !oldOk || !newOk {
return false
}
// Stop stops the custom informer if it exists
func (r *ClusterPermissionReconciler) Stop() {
if r.customInformer != nil {
r.customInformer.Stop()
}
}

// Only process if this is the managed-serviceaccount addon
if newAddon.Name != msacommon.AddonName {
return false
}
// createCustomInformerEventHandler creates an event handler for the custom informer
func (r *ClusterPermissionReconciler) createCustomInformerEventHandler() func(obj *addonv1alpha1.ManagedClusterAddOn) {
return func(addon *addonv1alpha1.ManagedClusterAddOn) {
log := log.Log.WithName("CustomInformerEventHandler")

// Only process if status.namespace has changed
return oldAddon.Status.Namespace != newAddon.Status.Namespace
},
DeleteFunc: func(e event.DeleteEvent) bool {
return false // Don't process Delete events
},
GenericFunc: func(e event.GenericEvent) bool {
return false // Don't process Generic events
},
})).
Complete(r)
// Find all ClusterPermissions in this addon's namespace that have ManagedServiceAccount subjects
ctx := context.Background()
var clusterPermissions cpv1alpha1.ClusterPermissionList
err := r.List(ctx, &clusterPermissions, &client.ListOptions{
Namespace: addon.Namespace,
})
if err != nil {
log.Error(err, "failed to list ClusterPermissions", "namespace", addon.Namespace)
return
}

// Process each ClusterPermission that uses ManagedServiceAccount
for _, cp := range clusterPermissions.Items {
if r.clusterPermissionUsesManagedServiceAccount(&cp) {
log.Info("Triggering reconciliation for ClusterPermission due to ManagedClusterAddOn change",
"clusterPermission", cp.Name,
"namespace", cp.Namespace,
"addonNamespace", addon.Status.Namespace,
)

// Trigger reconciliation by updating the ClusterPermission
r.reconcileClusterPermission(ctx, &cp)
}
}
}
}

// reconcileClusterPermission triggers reconciliation of a specific ClusterPermission
func (r *ClusterPermissionReconciler) reconcileClusterPermission(ctx context.Context, cp *cpv1alpha1.ClusterPermission) {
log := log.FromContext(ctx)

// Create a reconcile request
req := reconcile.Request{
NamespacedName: types.NamespacedName{
Name: cp.Name,
Namespace: cp.Namespace,
},
}

// Call the reconcile function directly
_, err := r.Reconcile(ctx, req)
if err != nil {
log.Error(err, "Failed to reconcile ClusterPermission", "name", cp.Name, "namespace", cp.Namespace)
Copy link
Member

@mikeshng mikeshng Oct 16, 2025

Choose a reason for hiding this comment

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

When reconcile error the general pattern should be requeue. I don't see it here or the caller side.

Copy link
Collaborator Author

@fxiang1 fxiang1 Oct 16, 2025

Choose a reason for hiding this comment

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

AI said since we are calling Reconcile manually from the custom informer event handler (not through the normal controller queue), we can only retry here. So AI added retries for this as well 😅

}
}

//+kubebuilder:rbac:groups=rbac.open-cluster-management.io,resources=clusterpermissions,verbs=get;list;watch;create;update;patch;delete
Expand Down Expand Up @@ -694,47 +741,6 @@ func joinStrings(strings []string, separator string) string {
return result
}

// managedClusterAddOnEventHandler returns an event handler that reconciles ClusterPermissions
// when a ManagedClusterAddOn's status.namespace changes
func (r *ClusterPermissionReconciler) managedClusterAddOnEventHandler() handler.EventHandler {
return handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, obj client.Object) []reconcile.Request {
log := log.FromContext(ctx)

addon, ok := obj.(*addonv1alpha1.ManagedClusterAddOn)
if !ok {
log.Error(nil, "object is not a ManagedClusterAddOn", "object", obj)
return []reconcile.Request{}
}

// Find all ClusterPermissions in this addon's namespace that have ManagedServiceAccount subjects
var clusterPermissions cpv1alpha1.ClusterPermissionList
err := r.List(ctx, &clusterPermissions, &client.ListOptions{
Namespace: addon.Namespace,
})
if err != nil {
log.Error(err, "failed to list ClusterPermissions", "namespace", addon.Namespace)
return []reconcile.Request{}
}

var requests []reconcile.Request
for _, cp := range clusterPermissions.Items {
if r.clusterPermissionUsesManagedServiceAccount(&cp) {
requests = append(requests, reconcile.Request{
NamespacedName: types.NamespacedName{
Name: cp.Name,
Namespace: cp.Namespace,
},
})
}
}

log.Info("ManagedClusterAddOn status.namespace changed, reconciling ClusterPermissions",
"addon", addon.Name, "namespace", addon.Namespace, "requests", len(requests))

return requests
})
}

// clusterPermissionUsesManagedServiceAccount checks if a ClusterPermission uses ManagedServiceAccount subjects
func (r *ClusterPermissionReconciler) clusterPermissionUsesManagedServiceAccount(cp *cpv1alpha1.ClusterPermission) bool {
// Check ClusterRoleBinding
Expand Down
Loading