From 6ead3c79f12df824aa01acf493e75f848c3ac627 Mon Sep 17 00:00:00 2001 From: Qixiang Wan Date: Tue, 16 Dec 2025 10:51:59 +0800 Subject: [PATCH] Refactor: replace APIReader with cache configuration for memory optimization The previous implementation (58c7dbe) used APIReader to bypass the cache for infrequently accessed resources. This commit replaces that approach with controller-runtime's built-in cache configuration options. For non-watched resources use DisableFor to prevent lazy informer creation when Client.Get/List is called. Without this, a single Get/List call would create a cluster-wide informer that caches ALL objects of that type. For watched resources (DependencyUpdateCheck, Event, PipelineRun), use namespace-scoped caching to limit informers to the mintmaker namespace only. Also apply TransformStripManagedFields to reduce cached object size. --- cmd/manager/main.go | 57 ++++++++++++++++--- .../dependencyupdatecheck_controller.go | 32 +++++------ internal/controller/event_controller.go | 24 +++----- internal/controller/pipelinerun_controller.go | 12 +--- internal/controller/suite_test.go | 28 ++++++++- 5 files changed, 101 insertions(+), 52 deletions(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index e7afefce..097d2c8c 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -26,10 +26,13 @@ import ( _ "k8s.io/client-go/plugin/pkg/client/auth" tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/cache" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/controller-runtime/pkg/metrics" @@ -41,6 +44,7 @@ import ( mmv1alpha1 "github.com/konflux-ci/mintmaker/api/v1alpha1" "github.com/konflux-ci/mintmaker/internal/controller" + . "github.com/konflux-ci/mintmaker/internal/pkg/constant" mintmakermetrics "github.com/konflux-ci/mintmaker/internal/pkg/metrics" // +kubebuilder:scaffold:imports ) @@ -130,7 +134,48 @@ func main() { } mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ - Scheme: scheme, + Scheme: scheme, + // Configure client to bypass cache for resources that are NOT watched + // and accessed infrequently. This prevents informer creation and reduces + // memory consumption. + Client: client.Options{ + Cache: &client.CacheOptions{ + DisableFor: []client.Object{ + // Core resources accessed infrequently during reconciliation + // These are NOT watched, so disabling cache prevents informer creation + &corev1.Secret{}, + &corev1.ServiceAccount{}, + &corev1.ConfigMap{}, + &corev1.Pod{}, + &appstudiov1alpha1.Component{}, + }, + }, + }, + // Configure cache for WATCHED resources with namespace-scoping and transforms. + // These resources need informers for the watch to work, but we only care about + // the resources in mintmaker namespace. Strip managed fields to reduce memory. + Cache: cache.Options{ + ByObject: map[client.Object]cache.ByObject{ + &mmv1alpha1.DependencyUpdateCheck{}: { + Namespaces: map[string]cache.Config{ + MintMakerNamespaceName: {}, + }, + Transform: cache.TransformStripManagedFields(), + }, + &corev1.Event{}: { + Namespaces: map[string]cache.Config{ + MintMakerNamespaceName: {}, + }, + Transform: cache.TransformStripManagedFields(), + }, + &tektonv1.PipelineRun{}: { + Namespaces: map[string]cache.Config{ + MintMakerNamespaceName: {}, + }, + Transform: cache.TransformStripManagedFields(), + }, + }, + }, Metrics: metricsServerOptions, WebhookServer: webhookServer, HealthProbeBindAddress: probeAddr, @@ -173,9 +218,8 @@ func main() { } if err = (&controller.DependencyUpdateCheckReconciler{ - Client: mgr.GetClient(), - APIReader: mgr.GetAPIReader(), - Scheme: mgr.GetScheme(), + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "DependencyUpdateCheck") os.Exit(1) @@ -190,9 +234,8 @@ func main() { } if err = (&controller.EventReconciler{ - Client: mgr.GetClient(), - APIReader: mgr.GetAPIReader(), - Scheme: mgr.GetScheme(), + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Event") os.Exit(1) diff --git a/internal/controller/dependencyupdatecheck_controller.go b/internal/controller/dependencyupdatecheck_controller.go index 81336999..a5e95f4d 100644 --- a/internal/controller/dependencyupdatecheck_controller.go +++ b/internal/controller/dependencyupdatecheck_controller.go @@ -28,7 +28,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" 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/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/event" @@ -52,16 +51,14 @@ const InternalSecretLabelName = "appstudio.redhat.com/internal" // DependencyUpdateCheckReconciler reconciles a DependencyUpdateCheck object type DependencyUpdateCheckReconciler struct { - Client client.Client - APIReader client.Reader - Scheme *runtime.Scheme + Client client.Client + Scheme *runtime.Scheme } -func NewDependencyUpdateCheckReconciler(client client.Client, apiReader client.Reader, scheme *runtime.Scheme, eventRecorder record.EventRecorder) *DependencyUpdateCheckReconciler { +func NewDependencyUpdateCheckReconciler(client client.Client, scheme *runtime.Scheme, eventRecorder record.EventRecorder) *DependencyUpdateCheckReconciler { return &DependencyUpdateCheckReconciler{ - Client: client, - APIReader: apiReader, - Scheme: scheme, + Client: client, + Scheme: scheme, } } @@ -96,7 +93,7 @@ func (r *DependencyUpdateCheckReconciler) getMergedDockerConfigJson(ctx context. componentNamespace := comp.GetNamespace() serviceAccountName := "build-pipeline-" + comp.GetName() serviceAccount := &corev1.ServiceAccount{} - if err := r.APIReader.Get(ctx, types.NamespacedName{Namespace: componentNamespace, Name: serviceAccountName}, serviceAccount); err != nil { + if err := r.Client.Get(ctx, types.NamespacedName{Namespace: componentNamespace, Name: serviceAccountName}, serviceAccount); err != nil { if errors.IsNotFound(err) { log.Info("service account not found in component namespace", "service-account", serviceAccountName) return nil, nil @@ -108,7 +105,7 @@ func (r *DependencyUpdateCheckReconciler) getMergedDockerConfigJson(ctx context. mergedAuths := make(map[string]interface{}) for _, secretRef := range serviceAccount.Secrets { var secret corev1.Secret - if err := r.APIReader.Get(ctx, types.NamespacedName{Namespace: componentNamespace, Name: secretRef.Name}, &secret); err != nil { + if err := r.Client.Get(ctx, types.NamespacedName{Namespace: componentNamespace, Name: secretRef.Name}, &secret); err != nil { if errors.IsNotFound(err) { log.Info("secret not found in component namespace", "secret", secretRef.Name) continue @@ -406,7 +403,7 @@ func (r *DependencyUpdateCheckReconciler) Reconcile(ctx context.Context, req ctr ctx = ctrllog.IntoContext(ctx, log) dependencyupdatecheck := &mmv1alpha1.DependencyUpdateCheck{} - err := r.APIReader.Get(ctx, req.NamespacedName, dependencyupdatecheck) + err := r.Client.Get(ctx, req.NamespacedName, dependencyupdatecheck) if err != nil { if errors.IsNotFound(err) { return ctrl.Result{}, nil @@ -447,7 +444,7 @@ func (r *DependencyUpdateCheckReconciler) Reconcile(ctx context.Context, req ctr } } else { allComponents := &appstudiov1alpha1.ComponentList{} - if err := r.APIReader.List(ctx, allComponents, &client.ListOptions{}); err != nil { + if err := r.Client.List(ctx, allComponents, &client.ListOptions{}); err != nil { log.Error(err, "failed to list Components") return ctrl.Result{}, err } @@ -520,15 +517,12 @@ func (r *DependencyUpdateCheckReconciler) Reconcile(ctx context.Context, req ctr // SetupWithManager sets up the controller with the Manager. func (r *DependencyUpdateCheckReconciler) SetupWithManager(mgr ctrl.Manager) error { - // we are monitoring the creation of DependencyUpdateCheck + // We only react to Create events for DependencyUpdateCheck in mintmaker namespace. + // Namespace filtering is handled by the manager's cache configuration. return ctrl.NewControllerManagedBy(mgr). - For(&mmv1alpha1.DependencyUpdateCheck{}, builder.WithPredicates(predicate.NewPredicateFuncs(func(object client.Object) bool { - return object.GetNamespace() == MintMakerNamespaceName - }))). + For(&mmv1alpha1.DependencyUpdateCheck{}). WithEventFilter(predicate.Funcs{ - CreateFunc: func(e event.CreateEvent) bool { - return e.Object.GetNamespace() == MintMakerNamespaceName - }, + CreateFunc: func(e event.CreateEvent) bool { return true }, DeleteFunc: func(e event.DeleteEvent) bool { return false }, UpdateFunc: func(e event.UpdateEvent) bool { return false }, GenericFunc: func(e event.GenericEvent) bool { return false }, diff --git a/internal/controller/event_controller.go b/internal/controller/event_controller.go index 92bae7a9..a52626b6 100644 --- a/internal/controller/event_controller.go +++ b/internal/controller/event_controller.go @@ -24,7 +24,6 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" 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" ctrllog "sigs.k8s.io/controller-runtime/pkg/log" @@ -33,14 +32,12 @@ import ( appstudiov1alpha1 "github.com/konflux-ci/application-api/api/v1alpha1" component "github.com/konflux-ci/mintmaker/internal/pkg/component" - . "github.com/konflux-ci/mintmaker/internal/pkg/constant" ) // EventReconciler reconciles a Event object type EventReconciler struct { client.Client - APIReader client.Reader - Scheme *runtime.Scheme + Scheme *runtime.Scheme } // markEventAsProcessed adds an annotation to the event indicating it has been processed @@ -96,7 +93,7 @@ func (r *EventReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl } var plr tektonv1.PipelineRun - if err := r.APIReader.Get(ctx, client.ObjectKey{Namespace: pod.Namespace, Name: plrName}, &plr); err != nil { + if err := r.Client.Get(ctx, client.ObjectKey{Namespace: pod.Namespace, Name: plrName}, &plr); err != nil { if apierrors.IsNotFound(err) { // The PipelineRun is gone, we can't update it. return @@ -120,7 +117,7 @@ func (r *EventReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl } }() - if err := r.APIReader.Get(ctx, req.NamespacedName, &evt); err != nil { + if err := r.Client.Get(ctx, req.NamespacedName, &evt); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } @@ -146,7 +143,7 @@ func (r *EventReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl podNamespace := evt.InvolvedObject.Namespace // Get the actual corresponding Pod object for this event - if err := r.APIReader.Get(ctx, client.ObjectKey{Namespace: podNamespace, Name: podName}, &pod); err != nil { + if err := r.Client.Get(ctx, client.ObjectKey{Namespace: podNamespace, Name: podName}, &pod); err != nil { if apierrors.IsNotFound(err) { // Pod has gone, we can't proceed return ctrl.Result{}, nil @@ -177,7 +174,7 @@ func (r *EventReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl // Get the secret var secret corev1.Secret - if err := r.APIReader.Get(ctx, client.ObjectKey{Namespace: podNamespace, Name: secretName}, &secret); err != nil { + if err := r.Client.Get(ctx, client.ObjectKey{Namespace: podNamespace, Name: secretName}, &secret); err != nil { if apierrors.IsNotFound(err) { // Secret doesn't exist, in theory this should not happen unless someone // deleted the secret by manual, anyway we will ignore this @@ -198,7 +195,7 @@ func (r *EventReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl // Get the component var comp appstudiov1alpha1.Component - if err := r.APIReader.Get(ctx, client.ObjectKey{Namespace: componentNamespace, Name: componentName}, &comp); err != nil { + if err := r.Client.Get(ctx, client.ObjectKey{Namespace: componentNamespace, Name: componentName}, &comp); err != nil { if apierrors.IsNotFound(err) { // Component has gone, we can't proceed return ctrl.Result{}, nil @@ -242,15 +239,12 @@ func (r *EventReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl // SetupWithManager sets up the controller with the Manager. func (r *EventReconciler) SetupWithManager(mgr ctrl.Manager) error { + // We only react to Create events for Event in mintmaker namespace. + // Namespace filtering is handled by the manager's cache configuration. return ctrl.NewControllerManagedBy(mgr). - For(&corev1.Event{}, builder.WithPredicates(predicate.NewPredicateFuncs(func(object client.Object) bool { - return object.GetNamespace() == MintMakerNamespaceName - }))). + For(&corev1.Event{}). WithEventFilter(predicate.Funcs{ CreateFunc: func(e event.CreateEvent) bool { - if e.Object.GetNamespace() != MintMakerNamespaceName { - return false - } evt, ok := e.Object.(*corev1.Event) if !ok { return false diff --git a/internal/controller/pipelinerun_controller.go b/internal/controller/pipelinerun_controller.go index 5f222cd8..75de300d 100644 --- a/internal/controller/pipelinerun_controller.go +++ b/internal/controller/pipelinerun_controller.go @@ -22,14 +22,11 @@ import ( "k8s.io/apimachinery/pkg/runtime" "knative.dev/pkg/apis" 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/predicate" tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" - - . "github.com/konflux-ci/mintmaker/internal/pkg/constant" ) var ( @@ -52,10 +49,10 @@ func (r *PipelineRunReconciler) Reconcile(ctx context.Context, req ctrl.Request) // SetupWithManager sets up the controller with the Manager. func (r *PipelineRunReconciler) SetupWithManager(mgr ctrl.Manager) error { + // We only react to Update events for PipelineRun in mintmaker namespace. + // Namespace filtering is handled by the manager's cache configuration. return ctrl.NewControllerManagedBy(mgr). - For(&tektonv1.PipelineRun{}, builder.WithPredicates(predicate.NewPredicateFuncs(func(object client.Object) bool { - return object.GetNamespace() == MintMakerNamespaceName - }))). + For(&tektonv1.PipelineRun{}). WithEventFilter(predicate.Funcs{ CreateFunc: func(e event.CreateEvent) bool { return false @@ -64,9 +61,6 @@ func (r *PipelineRunReconciler) SetupWithManager(mgr ctrl.Manager) error { return false }, UpdateFunc: func(e event.UpdateEvent) bool { - if e.ObjectNew.GetNamespace() != MintMakerNamespaceName { - return false - } if oldPipelineRun, ok := e.ObjectOld.(*tektonv1.PipelineRun); ok { if newPipelineRun, ok := e.ObjectNew.(*tektonv1.PipelineRun); ok { if !oldPipelineRun.IsDone() && newPipelineRun.IsDone() { diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index ab68a4b0..b9702d4d 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -26,8 +26,10 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" + corev1 "k8s.io/api/core/v1" "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -35,6 +37,7 @@ import ( //+kubebuilder:scaffold:imports mmv1alpha1 "github.com/konflux-ci/mintmaker/api/v1alpha1" + . "github.com/konflux-ci/mintmaker/internal/pkg/constant" ) const ( @@ -98,16 +101,37 @@ var _ = BeforeSuite(func() { k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{ Scheme: scheme.Scheme, + // Configure namespace-scoped cache to match production behavior. + // This ensures tests validate the actual filtering mechanism. + Cache: cache.Options{ + ByObject: map[client.Object]cache.ByObject{ + &mmv1alpha1.DependencyUpdateCheck{}: { + Namespaces: map[string]cache.Config{ + MintMakerNamespaceName: {}, + }, + }, + &corev1.Event{}: { + Namespaces: map[string]cache.Config{ + MintMakerNamespaceName: {}, + }, + }, + &tektonv1.PipelineRun{}: { + Namespaces: map[string]cache.Config{ + MintMakerNamespaceName: {}, + }, + }, + }, + }, }) Expect(err).ToNot(HaveOccurred()) - err = (NewDependencyUpdateCheckReconciler(k8sManager.GetClient(), k8sManager.GetAPIReader(), k8sManager.GetScheme(), k8sManager.GetEventRecorderFor("DependencyUpdateCheckController"))).SetupWithManager(k8sManager) + err = (NewDependencyUpdateCheckReconciler(k8sManager.GetClient(), k8sManager.GetScheme(), k8sManager.GetEventRecorderFor("DependencyUpdateCheckController"))).SetupWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) err = (&PipelineRunReconciler{Client: k8sManager.GetClient(), Scheme: k8sManager.GetScheme()}).SetupWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) - err = (&EventReconciler{Client: k8sManager.GetClient(), APIReader: k8sManager.GetAPIReader(), Scheme: k8sManager.GetScheme()}).SetupWithManager(k8sManager) + err = (&EventReconciler{Client: k8sManager.GetClient(), Scheme: k8sManager.GetScheme()}).SetupWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) go func() {