From 1ac789696f38282d236e0318221fb95753cbddbb Mon Sep 17 00:00:00 2001 From: Vince Prignano Date: Tue, 15 Dec 2020 09:23:44 -0800 Subject: [PATCH] Use uncached client and partial metadata for secret and configmaps These changes allow the current stable Cluster API release to reduce its memory footprint by a large margin. Currently, we have multiple controllers that are either watching (like ClusterResourceSet) or querying (get/list) corev1.Secret and corev1.ConfigMap resources. When these kinds go through the case, all of the objects in the cluster end up being cached, not just the ones we're interested in. In production environments, there might be a large number of ConfigMap or Secret resources that we end up caching and watching for little gain. Signed-off-by: Vince Prignano --- bootstrap/kubeadm/main.go | 8 ++++- controllers/remote/cluster_cache.go | 36 +++++++------------ controllers/remote/cluster_cache_fake.go | 6 ++-- .../remote/cluster_cache_reconciler_test.go | 4 --- controlplane/kubeadm/main.go | 8 ++++- .../clusterresourceset_controller.go | 6 ++-- exp/addons/controllers/suite_test.go | 1 + go.mod | 2 +- go.sum | 4 +-- main.go | 24 ++++++++----- test/framework/convenience.go | 2 ++ test/infrastructure/docker/go.mod | 2 +- test/infrastructure/docker/go.sum | 4 +-- 13 files changed, 58 insertions(+), 49 deletions(-) diff --git a/bootstrap/kubeadm/main.go b/bootstrap/kubeadm/main.go index b36755aa7bd3..be9af4726953 100644 --- a/bootstrap/kubeadm/main.go +++ b/bootstrap/kubeadm/main.go @@ -26,6 +26,7 @@ import ( "time" "github.com/spf13/pflag" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/klog" @@ -37,6 +38,7 @@ import ( expv1 "sigs.k8s.io/cluster-api/exp/api/v1alpha4" "sigs.k8s.io/cluster-api/feature" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" // +kubebuilder:scaffold:imports ) @@ -132,7 +134,11 @@ func main() { RetryPeriod: &leaderElectionRetryPeriod, Namespace: watchNamespace, SyncPeriod: &syncPeriod, - Port: webhookPort, + ClientDisableCacheFor: []client.Object{ + &corev1.ConfigMap{}, + &corev1.Secret{}, + }, + Port: webhookPort, }) if err != nil { setupLog.Error(err, "unable to start manager") diff --git a/controllers/remote/cluster_cache.go b/controllers/remote/cluster_cache.go index e45b94fec8a7..aa978e29e047 100644 --- a/controllers/remote/cluster_cache.go +++ b/controllers/remote/cluster_cache.go @@ -23,6 +23,7 @@ import ( "github.com/go-logr/logr" "github.com/pkg/errors" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" @@ -78,28 +79,14 @@ func (t *ClusterCacheTracker) GetClient(ctx context.Context, cluster client.Obje return nil, err } - return accessor.delegatingClient, nil + return accessor.client, nil } -// GetLiveClient returns a live client (talks to the api-server directly) for the given cluster. -func (t *ClusterCacheTracker) GetLiveClient(ctx context.Context, cluster client.ObjectKey) (client.Client, error) { - t.lock.Lock() - defer t.lock.Unlock() - - accessor, err := t.getClusterAccessorLH(ctx, cluster) - if err != nil { - return nil, err - } - - return accessor.liveClient, nil -} - -// clusterAccessor represents the combination of a delegating client, live client (direct to api-server), cache, and watches for a remote cluster. +// clusterAccessor represents the combination of a delegating client, cache, and watches for a remote cluster. type clusterAccessor struct { - cache *stoppableCache - delegatingClient client.Client - liveClient client.Client - watches sets.String + cache *stoppableCache + client client.Client + watches sets.String } // clusterAccessorExists returns true if a clusterAccessor exists for cluster. @@ -180,16 +167,19 @@ func (t *ClusterCacheTracker) newClusterAccessor(ctx context.Context, cluster cl delegatingClient, err := client.NewDelegatingClient(client.NewDelegatingClientInput{ CacheReader: cache, Client: c, + UncachedObjects: []client.Object{ + &corev1.ConfigMap{}, + &corev1.Secret{}, + }, }) if err != nil { return nil, err } return &clusterAccessor{ - cache: cache, - delegatingClient: delegatingClient, - liveClient: c, - watches: sets.NewString(), + cache: cache, + client: delegatingClient, + watches: sets.NewString(), }, nil } diff --git a/controllers/remote/cluster_cache_fake.go b/controllers/remote/cluster_cache_fake.go index f72fc618a09a..43aff34f8066 100644 --- a/controllers/remote/cluster_cache_fake.go +++ b/controllers/remote/cluster_cache_fake.go @@ -42,9 +42,9 @@ func NewTestClusterCacheTracker(log logr.Logger, cl client.Client, scheme *runti testCacheTracker.clusterAccessors[objKey] = &clusterAccessor{ - cache: nil, - delegatingClient: delegatingClient, - watches: sets.NewString(watchObjects...), + cache: nil, + client: delegatingClient, + watches: sets.NewString(watchObjects...), } return testCacheTracker } diff --git a/controllers/remote/cluster_cache_reconciler_test.go b/controllers/remote/cluster_cache_reconciler_test.go index 13c578c9d8cc..b57f3f799c66 100644 --- a/controllers/remote/cluster_cache_reconciler_test.go +++ b/controllers/remote/cluster_cache_reconciler_test.go @@ -73,10 +73,6 @@ var _ = Describe("ClusterCache Reconciler suite", func() { By("Creating a clusterAccessor for the cluster") _, err := cct.GetClient(ctx, testClusterKey) Expect(err).To(BeNil()) - - By("Retrieving a live client for the cluster") - _, err = cct.GetLiveClient(ctx, testClusterKey) - Expect(err).To(BeNil()) } BeforeEach(func() { diff --git a/controlplane/kubeadm/main.go b/controlplane/kubeadm/main.go index 7022ae5ae2ee..cc5f5380bbff 100644 --- a/controlplane/kubeadm/main.go +++ b/controlplane/kubeadm/main.go @@ -26,6 +26,7 @@ import ( "time" "github.com/spf13/pflag" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/klog" @@ -36,6 +37,7 @@ import ( kcpv1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1alpha4" kubeadmcontrolplanecontrollers "sigs.k8s.io/cluster-api/controlplane/kubeadm/controllers" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" // +kubebuilder:scaffold:imports ) @@ -126,7 +128,11 @@ func main() { RetryPeriod: &leaderElectionRetryPeriod, Namespace: watchNamespace, SyncPeriod: &syncPeriod, - Port: webhookPort, + ClientDisableCacheFor: []client.Object{ + &corev1.ConfigMap{}, + &corev1.Secret{}, + }, + Port: webhookPort, }) if err != nil { setupLog.Error(err, "unable to start manager") diff --git a/exp/addons/controllers/clusterresourceset_controller.go b/exp/addons/controllers/clusterresourceset_controller.go index 3f680e2c517c..9498a5391107 100644 --- a/exp/addons/controllers/clusterresourceset_controller.go +++ b/exp/addons/controllers/clusterresourceset_controller.go @@ -66,7 +66,7 @@ type ClusterResourceSetReconciler struct { } func (r *ClusterResourceSetReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error { - _, err := ctrl.NewControllerManagedBy(mgr). + err := ctrl.NewControllerManagedBy(mgr). For(&addonsv1.ClusterResourceSet{}). Watches( &source.Kind{Type: &clusterv1.Cluster{}}, @@ -75,6 +75,7 @@ func (r *ClusterResourceSetReconciler) SetupWithManager(ctx context.Context, mgr Watches( &source.Kind{Type: &corev1.ConfigMap{}}, handler.EnqueueRequestsFromMapFunc(r.resourceToClusterResourceSet), + builder.OnlyMetadata, builder.WithPredicates( resourcepredicates.ResourceCreate(ctrl.LoggerFrom(ctx)), ), @@ -82,13 +83,14 @@ func (r *ClusterResourceSetReconciler) SetupWithManager(ctx context.Context, mgr Watches( &source.Kind{Type: &corev1.Secret{}}, handler.EnqueueRequestsFromMapFunc(r.resourceToClusterResourceSet), + builder.OnlyMetadata, builder.WithPredicates( resourcepredicates.AddonsSecretCreate(ctrl.LoggerFrom(ctx)), ), ). WithOptions(options). WithEventFilter(predicates.ResourceNotPaused(ctrl.LoggerFrom(ctx))). - Build(r) + Complete(r) if err != nil { return errors.Wrap(err, "failed setting up with a controller manager") diff --git a/exp/addons/controllers/suite_test.go b/exp/addons/controllers/suite_test.go index 4281b79153f7..dff6da332219 100644 --- a/exp/addons/controllers/suite_test.go +++ b/exp/addons/controllers/suite_test.go @@ -62,6 +62,7 @@ var _ = BeforeSuite(func(done Done) { By("starting the manager") go func() { + defer GinkgoRecover() Expect(testEnv.StartManager(ctx)).To(Succeed()) }() diff --git a/go.mod b/go.mod index e53119c878a0..103ff95141f8 100644 --- a/go.mod +++ b/go.mod @@ -39,7 +39,7 @@ require ( k8s.io/klog v1.0.0 k8s.io/kubectl v0.19.2 k8s.io/utils v0.0.0-20200912215256-4140de9c8800 - sigs.k8s.io/controller-runtime v0.7.0-alpha.8 + sigs.k8s.io/controller-runtime v0.7.1-0.20201215171748-096b2e07c091 sigs.k8s.io/kind v0.9.0 sigs.k8s.io/yaml v1.2.0 ) diff --git a/go.sum b/go.sum index 1d6c9aa4a6b1..ad2617333687 100644 --- a/go.sum +++ b/go.sum @@ -819,8 +819,8 @@ k8s.io/utils v0.0.0-20200912215256-4140de9c8800 h1:9ZNvfPvVIEsp/T1ez4GQuzCcCTEQW k8s.io/utils v0.0.0-20200912215256-4140de9c8800/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.9/go.mod h1:dzAXnQbTRyDlZPJX2SUPEqvnB+j7AJjtlox7PEwigU0= -sigs.k8s.io/controller-runtime v0.7.0-alpha.8 h1:l8I2KO3xLuNaT0yPP6mtWLw5NuMG3dY2YiaQXEGApXg= -sigs.k8s.io/controller-runtime v0.7.0-alpha.8/go.mod h1:pJ3YBrJiAqMAZKi6UVGuE98ZrroV1p+pIhoHsMm9wdU= +sigs.k8s.io/controller-runtime v0.7.1-0.20201215171748-096b2e07c091 h1:tqrTDj7mJmM6TdpoM1rN2PzBRH9yzCReqKGMy4sp+f0= +sigs.k8s.io/controller-runtime v0.7.1-0.20201215171748-096b2e07c091/go.mod h1:pJ3YBrJiAqMAZKi6UVGuE98ZrroV1p+pIhoHsMm9wdU= sigs.k8s.io/kind v0.9.0 h1:SoDlXq6pEc7dGagHULNRCCBYrLH6xOi7lqXTRXeAlg4= sigs.k8s.io/kind v0.9.0/go.mod h1:cxKQWwmbtRDzQ+RNKnR6gZG6fjbeTtItp5cGf+ww+1Y= sigs.k8s.io/kustomize v2.0.3+incompatible/go.mod h1:MkjgH3RdOWrievjo6c9T245dYlB5QeXV4WCbnt/PEpU= diff --git a/main.go b/main.go index 8108514a5382..1730cb7a6eaa 100644 --- a/main.go +++ b/main.go @@ -25,6 +25,7 @@ import ( "time" "github.com/spf13/pflag" + corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" @@ -40,6 +41,7 @@ import ( expcontrollers "sigs.k8s.io/cluster-api/exp/controllers" "sigs.k8s.io/cluster-api/feature" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/healthz" // +kubebuilder:scaffold:imports @@ -153,15 +155,19 @@ func main() { } mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ - Scheme: scheme, - MetricsBindAddress: metricsBindAddr, - LeaderElection: enableLeaderElection, - LeaderElectionID: "controller-leader-election-capi", - LeaseDuration: &leaderElectionLeaseDuration, - RenewDeadline: &leaderElectionRenewDeadline, - RetryPeriod: &leaderElectionRetryPeriod, - Namespace: watchNamespace, - SyncPeriod: &syncPeriod, + Scheme: scheme, + MetricsBindAddress: metricsBindAddr, + LeaderElection: enableLeaderElection, + LeaderElectionID: "controller-leader-election-capi", + LeaseDuration: &leaderElectionLeaseDuration, + RenewDeadline: &leaderElectionRenewDeadline, + RetryPeriod: &leaderElectionRetryPeriod, + Namespace: watchNamespace, + SyncPeriod: &syncPeriod, + ClientDisableCacheFor: []client.Object{ + &corev1.ConfigMap{}, + &corev1.Secret{}, + }, Port: webhookPort, HealthProbeBindAddress: healthAddr, }) diff --git a/test/framework/convenience.go b/test/framework/convenience.go index 10efcb6a1c11..3ede32e2aae2 100644 --- a/test/framework/convenience.go +++ b/test/framework/convenience.go @@ -28,6 +28,7 @@ import ( clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1alpha4" controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1alpha4" + addonsv1 "sigs.k8s.io/cluster-api/exp/addons/api/v1alpha4" expv1 "sigs.k8s.io/cluster-api/exp/api/v1alpha4" ) @@ -50,6 +51,7 @@ func TryAddDefaultSchemes(scheme *runtime.Scheme) { // Add the experiments CAPI scheme. _ = expv1.AddToScheme(scheme) + _ = addonsv1.AddToScheme(scheme) // Add the kubeadm bootstrapper scheme. _ = bootstrapv1.AddToScheme(scheme) diff --git a/test/infrastructure/docker/go.mod b/test/infrastructure/docker/go.mod index e94ef489cc9e..b1bec9231560 100644 --- a/test/infrastructure/docker/go.mod +++ b/test/infrastructure/docker/go.mod @@ -13,7 +13,7 @@ require ( k8s.io/klog v1.0.0 k8s.io/utils v0.0.0-20200912215256-4140de9c8800 sigs.k8s.io/cluster-api v0.3.3 - sigs.k8s.io/controller-runtime v0.7.0-alpha.8 + sigs.k8s.io/controller-runtime v0.7.1-0.20201215171748-096b2e07c091 sigs.k8s.io/kind v0.9.0 sigs.k8s.io/yaml v1.2.0 ) diff --git a/test/infrastructure/docker/go.sum b/test/infrastructure/docker/go.sum index 3d0c16d0ea58..fe6d76eca5cd 100644 --- a/test/infrastructure/docker/go.sum +++ b/test/infrastructure/docker/go.sum @@ -757,8 +757,8 @@ k8s.io/utils v0.0.0-20200912215256-4140de9c8800 h1:9ZNvfPvVIEsp/T1ez4GQuzCcCTEQW k8s.io/utils v0.0.0-20200912215256-4140de9c8800/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.9/go.mod h1:dzAXnQbTRyDlZPJX2SUPEqvnB+j7AJjtlox7PEwigU0= -sigs.k8s.io/controller-runtime v0.7.0-alpha.8 h1:l8I2KO3xLuNaT0yPP6mtWLw5NuMG3dY2YiaQXEGApXg= -sigs.k8s.io/controller-runtime v0.7.0-alpha.8/go.mod h1:pJ3YBrJiAqMAZKi6UVGuE98ZrroV1p+pIhoHsMm9wdU= +sigs.k8s.io/controller-runtime v0.7.1-0.20201215171748-096b2e07c091 h1:tqrTDj7mJmM6TdpoM1rN2PzBRH9yzCReqKGMy4sp+f0= +sigs.k8s.io/controller-runtime v0.7.1-0.20201215171748-096b2e07c091/go.mod h1:pJ3YBrJiAqMAZKi6UVGuE98ZrroV1p+pIhoHsMm9wdU= sigs.k8s.io/kind v0.9.0 h1:SoDlXq6pEc7dGagHULNRCCBYrLH6xOi7lqXTRXeAlg4= sigs.k8s.io/kind v0.9.0/go.mod h1:cxKQWwmbtRDzQ+RNKnR6gZG6fjbeTtItp5cGf+ww+1Y= sigs.k8s.io/kustomize v2.0.3+incompatible/go.mod h1:MkjgH3RdOWrievjo6c9T245dYlB5QeXV4WCbnt/PEpU=