From 96f60446d7f51212d8984d1a481d8440946fd26b Mon Sep 17 00:00:00 2001 From: Matous Jobanek Date: Thu, 6 Mar 2025 16:01:42 +0100 Subject: [PATCH 1/2] do not duplicate cached clients --- main.go | 40 +++++++++------------------------------- 1 file changed, 9 insertions(+), 31 deletions(-) diff --git a/main.go b/main.go index f5a055e6..d91a35db 100644 --- a/main.go +++ b/main.go @@ -5,9 +5,10 @@ import ( "fmt" "os" goruntime "runtime" - metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" "time" + metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" + "github.com/codeready-toolchain/member-operator/controllers/idler" membercfgctrl "github.com/codeready-toolchain/member-operator/controllers/memberoperatorconfig" "github.com/codeready-toolchain/member-operator/controllers/memberstatus" @@ -183,6 +184,10 @@ func main() { os.Exit(1) } + // create a new client with a cache that watches (as opposed to the standard client) resources in all namespaces. + // This client should be used only for resources and kinds that are retrieved from other namespaces than the watched one. + // This will help keeping a reasonable memory usage for this operator since the cache won't store all other namespace scoped + // resources (secrets, etc.). allNamespacesCluster, err := runtimecluster.New(ctrl.GetConfigOrDie(), func(options *runtimecluster.Options) { options.Scheme = scheme }) @@ -196,12 +201,6 @@ func main() { os.Exit(1) } - allNamespacesClient, allNamespacesCache, err := newAllNamespacesClient(cfg) - if err != nil { - setupLog.Error(err, "") - os.Exit(1) - } - scalesClient, err := newScalesClient(cfg) if err != nil { setupLog.Error(err, "unable to create scales client") @@ -242,7 +241,7 @@ func main() { } if err := (&idler.Reconciler{ Scheme: mgr.GetScheme(), - AllNamespacesClient: allNamespacesClient, + AllNamespacesClient: allNamespacesCluster.GetClient(), Client: mgr.GetClient(), ScalesClient: scalesClient, DynamicClient: dynamicClient, @@ -257,7 +256,7 @@ func main() { Client: mgr.GetClient(), Scheme: mgr.GetScheme(), GetHostCluster: cluster.GetHostCluster, - AllNamespacesClient: allNamespacesClient, + AllNamespacesClient: allNamespacesCluster.GetClient(), VersionCheckManager: status.VersionCheckManager{GetGithubClientFunc: commonclient.NewGitHubClient}, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "MemberStatus") @@ -265,7 +264,7 @@ func main() { } if err = (nstemplateset.NewReconciler(&nstemplateset.APIClient{ Client: mgr.GetClient(), - AllNamespacesClient: allNamespacesClient, + AllNamespacesClient: allNamespacesCluster.GetClient(), Scheme: mgr.GetScheme(), GetHostCluster: cluster.GetHostCluster, })).SetupWithManager(mgr, allNamespacesCluster, discoveryClient); err != nil { @@ -316,13 +315,6 @@ func main() { os.Exit(1) } - go func() { - if err := allNamespacesCache.Start(stopChannel); err != nil { - setupLog.Error(err, "failed to start all-namespaces cache") - os.Exit(1) - } - }() - setupLog.Info("starting manager") if err := mgr.Start(stopChannel); err != nil { setupLog.Error(err, "problem running manager") @@ -331,20 +323,6 @@ func main() { } -// newAllNamespacesClient creates a new client that watches (as opposed to the standard client) resources in all namespaces. -// This client should be used only for resources and kinds that are retrieved from other namespaces than the watched one. -// This will help keeping a reasonable memory usage for this operator since the cache won't store all other namespace scoped -// resources (secrets, etc.). -func newAllNamespacesClient(config *rest.Config) (client.Client, cache.Cache, error) { - clusterAllNamespaces, err := runtimecluster.New(config, func(clusterOptions *runtimecluster.Options) { - clusterOptions.Scheme = scheme - }) - if err != nil { - return nil, nil, err - } - return clusterAllNamespaces.GetClient(), clusterAllNamespaces.GetCache(), nil -} - func newScalesClient(config *rest.Config) (scale.ScalesGetter, error) { c, err := kubernetes.NewForConfig(config) if err != nil { From 6df08c0744c778603c35dda02daf9e9a3a905410 Mon Sep 17 00:00:00 2001 From: Matous Jobanek Date: Fri, 7 Mar 2025 08:15:07 +0100 Subject: [PATCH 2/2] de-duplicate config initialization --- main.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/main.go b/main.go index d91a35db..86863828 100644 --- a/main.go +++ b/main.go @@ -158,7 +158,7 @@ func main() { } crtConfig.Print() - discoveryClient, err := discovery.NewDiscoveryClientForConfig(ctrl.GetConfigOrDie()) + discoveryClient, err := discovery.NewDiscoveryClientForConfig(cfg) if err != nil { setupLog.Error(err, "failed to create discovery client") os.Exit(1) @@ -166,7 +166,7 @@ func main() { // Webhook server will be created with default values (port 9443) as per doc - https://github.com/kubernetes-sigs/controller-runtime/blob/main/pkg/manager/manager.go#L244-L247 // Cache Options design doc - https://github.com/kubernetes-sigs/controller-runtime/blob/main/designs/cache_options.md - mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ + mgr, err := ctrl.NewManager(cfg, ctrl.Options{ Scheme: scheme, Metrics: metricsserver.Options{ BindAddress: metricsAddr, @@ -188,7 +188,7 @@ func main() { // This client should be used only for resources and kinds that are retrieved from other namespaces than the watched one. // This will help keeping a reasonable memory usage for this operator since the cache won't store all other namespace scoped // resources (secrets, etc.). - allNamespacesCluster, err := runtimecluster.New(ctrl.GetConfigOrDie(), func(options *runtimecluster.Options) { + allNamespacesCluster, err := runtimecluster.New(cfg, func(options *runtimecluster.Options) { options.Scheme = scheme }) if err != nil {