Skip to content

CM-735: Fix IstioCSR cache sync race condition by using unified manager cache #324

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 1 commit into from
Oct 21, 2025
Merged

CM-735: Fix IstioCSR cache sync race condition by using unified manager cache #324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 5 additions & 5 deletions pkg/controller/istiocsr/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ type ctrlClient interface {
}

func NewClient(m manager.Manager) (ctrlClient, error) {
c, err := BuildCustomClient(m)
if err != nil {
return nil, fmt.Errorf("failed to build custom client: %w", err)
}
// Use the manager's client directly instead of creating a custom client.
// The manager's client uses the manager's cache, which ensures the reconciler
// reads from the same cache that the controller's watches use, preventing
// cache mismatch issues.
return &ctrlClientImpl{
Client: c,
Client: m.GetClient(),
}, nil
}

Expand Down
145 changes: 45 additions & 100 deletions pkg/controller/istiocsr/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/selection"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/record"

ctrl "sigs.k8s.io/controller-runtime"
Expand Down Expand Up @@ -56,6 +57,50 @@ type Reconciler struct {
// +kubebuilder:rbac:groups=operator.openshift.io,resources=istiocsrs/status,verbs=get;update;patch
// +kubebuilder:rbac:groups=operator.openshift.io,resources=istiocsrs/finalizers,verbs=update

// NewCacheBuilder returns a cache builder function configured with label selectors
// for managed resources. This function is used by the manager to create its cache
// to ensure the reconciler reads from the same cache that the controller's watches use.
func NewCacheBuilder(config *rest.Config, opts cache.Options) (cache.Cache, error) {
managedResourceLabelReq, err := labels.NewRequirement(requestEnqueueLabelKey, selection.Equals, []string{requestEnqueueLabelValue})
if err != nil {
return nil, fmt.Errorf("invalid cache label requirement for %q: %w", requestEnqueueLabelKey, err)
}
managedResourceLabelReqSelector := labels.NewSelector().Add(*managedResourceLabelReq)

// Configure cache with label selectors for managed resources
opts.ByObject = map[client.Object]cache.ByObject{
// Explicitly include IstioCSR to ensure the cache properly watches and syncs all IstioCSR objects
&v1alpha1.IstioCSR{}: {},
// Resources managed by controller (with label selectors)
&certmanagerv1.Certificate{}: {
Label: managedResourceLabelReqSelector,
},
&appsv1.Deployment{}: {
Label: managedResourceLabelReqSelector,
},
&rbacv1.ClusterRole{}: {
Label: managedResourceLabelReqSelector,
},
&rbacv1.ClusterRoleBinding{}: {
Label: managedResourceLabelReqSelector,
},
&rbacv1.Role{}: {
Label: managedResourceLabelReqSelector,
},
&rbacv1.RoleBinding{}: {
Label: managedResourceLabelReqSelector,
},
&corev1.Service{}: {
Label: managedResourceLabelReqSelector,
},
&corev1.ServiceAccount{}: {
Label: managedResourceLabelReqSelector,
},
}

return cache.New(config, opts)
}

// New returns a new Reconciler instance.
func New(mgr ctrl.Manager) (*Reconciler, error) {
c, err := NewClient(mgr)
Expand All @@ -71,106 +116,6 @@ func New(mgr ctrl.Manager) (*Reconciler, error) {
}, nil
}

func BuildCustomClient(mgr ctrl.Manager) (client.Client, error) {
managedResourceLabelReq, _ := labels.NewRequirement(requestEnqueueLabelKey, selection.Equals, []string{requestEnqueueLabelValue})
managedResourceLabelReqSelector := labels.NewSelector().Add(*managedResourceLabelReq)

customCacheOpts := cache.Options{
HTTPClient: mgr.GetHTTPClient(),
Scheme: mgr.GetScheme(),
Mapper: mgr.GetRESTMapper(),
ByObject: map[client.Object]cache.ByObject{
&certmanagerv1.Certificate{}: {
Label: managedResourceLabelReqSelector,
},
&appsv1.Deployment{}: {
Label: managedResourceLabelReqSelector,
},
&rbacv1.ClusterRole{}: {
Label: managedResourceLabelReqSelector,
},
&rbacv1.ClusterRoleBinding{}: {
Label: managedResourceLabelReqSelector,
},
&rbacv1.Role{}: {
Label: managedResourceLabelReqSelector,
},
&rbacv1.RoleBinding{}: {
Label: managedResourceLabelReqSelector,
},
&corev1.Service{}: {
Label: managedResourceLabelReqSelector,
},
&corev1.ServiceAccount{}: {
Label: managedResourceLabelReqSelector,
},
},
ReaderFailOnMissingInformer: true,
}
customCache, err := cache.New(mgr.GetConfig(), customCacheOpts)
if err != nil {
return nil, err
}
if _, err = customCache.GetInformer(context.Background(), &v1alpha1.IstioCSR{}); err != nil {
return nil, err
}
if _, err = customCache.GetInformer(context.Background(), &certmanagerv1.Certificate{}); err != nil {
return nil, err
}
if _, err = customCache.GetInformer(context.Background(), &appsv1.Deployment{}); err != nil {
return nil, err
}
if _, err = customCache.GetInformer(context.Background(), &rbacv1.ClusterRole{}); err != nil {
return nil, err
}
if _, err = customCache.GetInformer(context.Background(), &rbacv1.ClusterRoleBinding{}); err != nil {
return nil, err
}
if _, err = customCache.GetInformer(context.Background(), &rbacv1.Role{}); err != nil {
return nil, err
}
if _, err = customCache.GetInformer(context.Background(), &rbacv1.RoleBinding{}); err != nil {
return nil, err
}
if _, err = customCache.GetInformer(context.Background(), &corev1.Service{}); err != nil {
return nil, err
}
if _, err = customCache.GetInformer(context.Background(), &corev1.ServiceAccount{}); err != nil {
return nil, err
}
if _, err = customCache.GetInformer(context.Background(), &corev1.Secret{}); err != nil {
return nil, err
}
if _, err = customCache.GetInformer(context.Background(), &corev1.ConfigMap{}); err != nil {
return nil, err
}
if _, err = customCache.GetInformer(context.Background(), &certmanagerv1.Issuer{}); err != nil {
return nil, err
}
if _, err = customCache.GetInformer(context.Background(), &certmanagerv1.ClusterIssuer{}); err != nil {
return nil, err
}

err = mgr.Add(customCache)
if err != nil {
return nil, err
}

customClient, err := client.New(mgr.GetConfig(), client.Options{
HTTPClient: mgr.GetHTTPClient(),
Scheme: mgr.GetScheme(),
Mapper: mgr.GetRESTMapper(),
Cache: &client.CacheOptions{
Reader: customCache,
},
})
if err != nil {
return nil, err
}

return customClient, nil
}

// SetupWithManager sets up the controller with the Manager.
func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
mapFunc := func(ctx context.Context, obj client.Object) []reconcile.Request {
Expand Down
9 changes: 3 additions & 6 deletions pkg/operator/setup_manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,9 @@ import (
"k8s.io/apimachinery/pkg/runtime"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
clientscheme "k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/rest"
"k8s.io/klog/v2"

ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
ctrllog "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/manager"

Expand Down Expand Up @@ -54,10 +52,9 @@ func NewControllerManager() (*Manager, error) {

mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
Scheme: scheme,
NewClient: func(config *rest.Config, options client.Options) (client.Client, error) {
return client.New(config, options)
},
Logger: ctrl.Log.WithName("operator-manager"),
// Use custom cache builder to configure label selectors for managed resources
NewCache: istiocsr.NewCacheBuilder,
Logger: ctrl.Log.WithName("operator-manager"),
})
if err != nil {
return nil, fmt.Errorf("failed to create manager: %w", err)
Expand Down