From eb0f1d4aa81794a38206965be25d118af5e18afc Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Wed, 13 Jan 2021 11:05:27 -0500 Subject: [PATCH] pkg/client: optionally allow caching of unstructured objects --- pkg/client/client_test.go | 164 +++++++++++++++++++++++++------------- pkg/client/split.go | 35 +++++--- 2 files changed, 130 insertions(+), 69 deletions(-) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 3c262221ed..2dc152b164 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -22,8 +22,6 @@ import ( "sync/atomic" "time" - "k8s.io/apimachinery/pkg/types" - . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" @@ -33,6 +31,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" kscheme "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client" @@ -3106,48 +3105,79 @@ var _ = Describe("DelegatingClient", func() { Expect(1).To(Equal(cachedReader.Called)) }) - It("should call client reader when unstructured object", func() { - cachedReader := &fakeReader{} - cl, err := client.New(cfg, client.Options{}) - Expect(err).NotTo(HaveOccurred()) - dReader, err := client.NewDelegatingClient(client.NewDelegatingClientInput{ - CacheReader: cachedReader, - Client: cl, - }) - Expect(err).NotTo(HaveOccurred()) - dep := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "deployment1", - Labels: map[string]string{"app": "frontend"}, - }, - Spec: appsv1.DeploymentSpec{ - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"app": "frontend"}, + When("getting unstructured objects", func() { + var dep *appsv1.Deployment + + BeforeEach(func() { + dep = &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "deployment1", + Labels: map[string]string{"app": "frontend"}, }, - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"app": "frontend"}}, - Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "x", Image: "x"}}}, + Spec: appsv1.DeploymentSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "frontend"}, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"app": "frontend"}}, + Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "x", Image: "x"}}}, + }, }, - }, - } - dep, err = clientset.AppsV1().Deployments("default").Create(context.Background(), dep, metav1.CreateOptions{}) - Expect(err).NotTo(HaveOccurred()) + } + var err error + dep, err = clientset.AppsV1().Deployments("default").Create(context.Background(), dep, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + }) + AfterEach(func() { + Expect(clientset.AppsV1().Deployments("default").Delete( + context.Background(), + dep.Name, + metav1.DeleteOptions{}, + )).To(Succeed()) + }) + It("should call client reader when not cached", func() { + cachedReader := &fakeReader{} + cl, err := client.New(cfg, client.Options{}) + Expect(err).NotTo(HaveOccurred()) + dReader, err := client.NewDelegatingClient(client.NewDelegatingClientInput{ + CacheReader: cachedReader, + Client: cl, + }) + Expect(err).NotTo(HaveOccurred()) + + actual := &unstructured.Unstructured{} + actual.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "apps", + Kind: "Deployment", + Version: "v1", + }) + actual.SetName(dep.Name) + key := client.ObjectKey{Namespace: dep.Namespace, Name: dep.Name} + Expect(dReader.Get(context.TODO(), key, actual)).To(Succeed()) + Expect(0).To(Equal(cachedReader.Called)) + }) + It("should call cache reader when cached", func() { + cachedReader := &fakeReader{} + cl, err := client.New(cfg, client.Options{}) + Expect(err).NotTo(HaveOccurred()) + dReader, err := client.NewDelegatingClient(client.NewDelegatingClientInput{ + CacheReader: cachedReader, + Client: cl, + CacheUnstructured: true, + }) + Expect(err).NotTo(HaveOccurred()) - actual := &unstructured.Unstructured{} - actual.SetGroupVersionKind(schema.GroupVersionKind{ - Group: "apps", - Kind: "Deployment", - Version: "v1", - }) - actual.SetName(dep.Name) - key := client.ObjectKey{Namespace: dep.Namespace, Name: dep.Name} - Expect(dReader.Get(context.TODO(), key, actual)).To(Succeed()) - Expect(0).To(Equal(cachedReader.Called)) - Expect(clientset.AppsV1().Deployments("default").Delete( - context.Background(), - dep.Name, - metav1.DeleteOptions{}, - )).To(Succeed()) + actual := &unstructured.Unstructured{} + actual.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "apps", + Kind: "Deployment", + Version: "v1", + }) + actual.SetName(dep.Name) + key := client.ObjectKey{Namespace: dep.Namespace, Name: dep.Name} + Expect(dReader.Get(context.TODO(), key, actual)).To(Succeed()) + Expect(1).To(Equal(cachedReader.Called)) + }) }) }) Describe("List", func() { @@ -3165,24 +3195,46 @@ var _ = Describe("DelegatingClient", func() { Expect(1).To(Equal(cachedReader.Called)) }) - It("should call client reader when unstructured object", func() { - cachedReader := &fakeReader{} - cl, err := client.New(cfg, client.Options{}) - Expect(err).NotTo(HaveOccurred()) - dReader, err := client.NewDelegatingClient(client.NewDelegatingClientInput{ - CacheReader: cachedReader, - Client: cl, + When("listing unstructured objects", func() { + It("should call client reader when not cached", func() { + cachedReader := &fakeReader{} + cl, err := client.New(cfg, client.Options{}) + Expect(err).NotTo(HaveOccurred()) + dReader, err := client.NewDelegatingClient(client.NewDelegatingClientInput{ + CacheReader: cachedReader, + Client: cl, + }) + Expect(err).NotTo(HaveOccurred()) + + actual := &unstructured.UnstructuredList{} + actual.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "apps", + Kind: "DeploymentList", + Version: "v1", + }) + Expect(dReader.List(context.Background(), actual)).To(Succeed()) + Expect(0).To(Equal(cachedReader.Called)) }) - Expect(err).NotTo(HaveOccurred()) + It("should call cache reader when cached", func() { + cachedReader := &fakeReader{} + cl, err := client.New(cfg, client.Options{}) + Expect(err).NotTo(HaveOccurred()) + dReader, err := client.NewDelegatingClient(client.NewDelegatingClientInput{ + CacheReader: cachedReader, + Client: cl, + CacheUnstructured: true, + }) + Expect(err).NotTo(HaveOccurred()) - actual := &unstructured.UnstructuredList{} - actual.SetGroupVersionKind(schema.GroupVersionKind{ - Group: "apps", - Kind: "DeploymentList", - Version: "v1", + actual := &unstructured.UnstructuredList{} + actual.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "apps", + Kind: "DeploymentList", + Version: "v1", + }) + Expect(dReader.List(context.Background(), actual)).To(Succeed()) + Expect(1).To(Equal(cachedReader.Called)) }) - Expect(dReader.List(context.Background(), actual)).To(Succeed()) - Expect(0).To(Equal(cachedReader.Called)) }) }) }) diff --git a/pkg/client/split.go b/pkg/client/split.go index d6c3d5f7da..bf4b861f39 100644 --- a/pkg/client/split.go +++ b/pkg/client/split.go @@ -24,14 +24,16 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" ) // NewDelegatingClientInput encapsulates the input parameters to create a new delegating client. type NewDelegatingClientInput struct { - CacheReader Reader - Client Client - UncachedObjects []Object + CacheReader Reader + Client Client + UncachedObjects []Object + CacheUnstructured bool } // NewDelegatingClient creates a new delegating client. @@ -53,10 +55,11 @@ func NewDelegatingClient(in NewDelegatingClientInput) (Client, error) { scheme: in.Client.Scheme(), mapper: in.Client.RESTMapper(), Reader: &delegatingReader{ - CacheReader: in.CacheReader, - ClientReader: in.Client, - scheme: in.Client.Scheme(), - uncachedGVKs: uncachedGVKs, + CacheReader: in.CacheReader, + ClientReader: in.Client, + scheme: in.Client.Scheme(), + uncachedGVKs: uncachedGVKs, + cacheUnstructured: in.CacheUnstructured, }, Writer: in.Client, StatusClient: in.Client, @@ -91,8 +94,9 @@ type delegatingReader struct { CacheReader Reader ClientReader Reader - uncachedGVKs map[schema.GroupVersionKind]struct{} - scheme *runtime.Scheme + uncachedGVKs map[schema.GroupVersionKind]struct{} + scheme *runtime.Scheme + cacheUnstructured bool } func (d *delegatingReader) shouldBypassCache(obj runtime.Object) (bool, error) { @@ -105,10 +109,15 @@ func (d *delegatingReader) shouldBypassCache(obj runtime.Object) (bool, error) { if meta.IsListType(obj) { gvk.Kind = strings.TrimSuffix(gvk.Kind, "List") } - _, isUncached := d.uncachedGVKs[gvk] - _, isUnstructured := obj.(*unstructured.Unstructured) - _, isUnstructuredList := obj.(*unstructured.UnstructuredList) - return isUncached || isUnstructured || isUnstructuredList, nil + if _, isUncached := d.uncachedGVKs[gvk]; isUncached { + return true, nil + } + if !d.cacheUnstructured { + _, isUnstructured := obj.(*unstructured.Unstructured) + _, isUnstructuredList := obj.(*unstructured.UnstructuredList) + return isUnstructured || isUnstructuredList, nil + } + return false, nil } // Get retrieves an obj for a given object key from the Kubernetes Cluster.