Skip to content

Commit d189419

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes-sigs#1351 from estroz/cherry-pick/1332
🐛 pkg/client: optionally allow caching of unstructured objects
2 parents 1d5261b + 0b59b8f commit d189419

File tree

2 files changed

+130
-69
lines changed

2 files changed

+130
-69
lines changed

‎pkg/client/client_test.go‎

Lines changed: 108 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@ import (
2222
"sync/atomic"
2323
"time"
2424

25-
"k8s.io/apimachinery/pkg/types"
26-
2725
. "github.com/onsi/ginkgo"
2826
. "github.com/onsi/gomega"
2927
appsv1 "k8s.io/api/apps/v1"
@@ -33,6 +31,7 @@ import (
3331
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
3432
"k8s.io/apimachinery/pkg/runtime"
3533
"k8s.io/apimachinery/pkg/runtime/schema"
34+
"k8s.io/apimachinery/pkg/types"
3635
kscheme "k8s.io/client-go/kubernetes/scheme"
3736

3837
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -3106,48 +3105,79 @@ var _ = Describe("DelegatingClient", func() {
31063105
Expect(1).To(Equal(cachedReader.Called))
31073106
})
31083107

3109-
It("should call client reader when unstructured object", func() {
3110-
cachedReader := &fakeReader{}
3111-
cl, err := client.New(cfg, client.Options{})
3112-
Expect(err).NotTo(HaveOccurred())
3113-
dReader, err := client.NewDelegatingClient(client.NewDelegatingClientInput{
3114-
CacheReader: cachedReader,
3115-
Client: cl,
3116-
})
3117-
Expect(err).NotTo(HaveOccurred())
3118-
dep := &appsv1.Deployment{
3119-
ObjectMeta: metav1.ObjectMeta{
3120-
Name: "deployment1",
3121-
Labels: map[string]string{"app": "frontend"},
3122-
},
3123-
Spec: appsv1.DeploymentSpec{
3124-
Selector: &metav1.LabelSelector{
3125-
MatchLabels: map[string]string{"app": "frontend"},
3108+
When("getting unstructured objects", func() {
3109+
var dep *appsv1.Deployment
3110+
3111+
BeforeEach(func() {
3112+
dep = &appsv1.Deployment{
3113+
ObjectMeta: metav1.ObjectMeta{
3114+
Name: "deployment1",
3115+
Labels: map[string]string{"app": "frontend"},
31263116
},
3127-
Template: corev1.PodTemplateSpec{
3128-
ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"app": "frontend"}},
3129-
Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "x", Image: "x"}}},
3117+
Spec: appsv1.DeploymentSpec{
3118+
Selector: &metav1.LabelSelector{
3119+
MatchLabels: map[string]string{"app": "frontend"},
3120+
},
3121+
Template: corev1.PodTemplateSpec{
3122+
ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"app": "frontend"}},
3123+
Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "x", Image: "x"}}},
3124+
},
31303125
},
3131-
},
3132-
}
3133-
dep, err = clientset.AppsV1().Deployments("default").Create(context.Background(), dep, metav1.CreateOptions{})
3134-
Expect(err).NotTo(HaveOccurred())
3126+
}
3127+
var err error
3128+
dep, err = clientset.AppsV1().Deployments("default").Create(context.Background(), dep, metav1.CreateOptions{})
3129+
Expect(err).NotTo(HaveOccurred())
3130+
})
3131+
AfterEach(func() {
3132+
Expect(clientset.AppsV1().Deployments("default").Delete(
3133+
context.Background(),
3134+
dep.Name,
3135+
metav1.DeleteOptions{},
3136+
)).To(Succeed())
3137+
})
3138+
It("should call client reader when not cached", func() {
3139+
cachedReader := &fakeReader{}
3140+
cl, err := client.New(cfg, client.Options{})
3141+
Expect(err).NotTo(HaveOccurred())
3142+
dReader, err := client.NewDelegatingClient(client.NewDelegatingClientInput{
3143+
CacheReader: cachedReader,
3144+
Client: cl,
3145+
})
3146+
Expect(err).NotTo(HaveOccurred())
3147+
3148+
actual := &unstructured.Unstructured{}
3149+
actual.SetGroupVersionKind(schema.GroupVersionKind{
3150+
Group: "apps",
3151+
Kind: "Deployment",
3152+
Version: "v1",
3153+
})
3154+
actual.SetName(dep.Name)
3155+
key := client.ObjectKey{Namespace: dep.Namespace, Name: dep.Name}
3156+
Expect(dReader.Get(context.TODO(), key, actual)).To(Succeed())
3157+
Expect(0).To(Equal(cachedReader.Called))
3158+
})
3159+
It("should call cache reader when cached", func() {
3160+
cachedReader := &fakeReader{}
3161+
cl, err := client.New(cfg, client.Options{})
3162+
Expect(err).NotTo(HaveOccurred())
3163+
dReader, err := client.NewDelegatingClient(client.NewDelegatingClientInput{
3164+
CacheReader: cachedReader,
3165+
Client: cl,
3166+
CacheUnstructured: true,
3167+
})
3168+
Expect(err).NotTo(HaveOccurred())
31353169

3136-
actual := &unstructured.Unstructured{}
3137-
actual.SetGroupVersionKind(schema.GroupVersionKind{
3138-
Group: "apps",
3139-
Kind: "Deployment",
3140-
Version: "v1",
3141-
})
3142-
actual.SetName(dep.Name)
3143-
key := client.ObjectKey{Namespace: dep.Namespace, Name: dep.Name}
3144-
Expect(dReader.Get(context.TODO(), key, actual)).To(Succeed())
3145-
Expect(0).To(Equal(cachedReader.Called))
3146-
Expect(clientset.AppsV1().Deployments("default").Delete(
3147-
context.Background(),
3148-
dep.Name,
3149-
metav1.DeleteOptions{},
3150-
)).To(Succeed())
3170+
actual := &unstructured.Unstructured{}
3171+
actual.SetGroupVersionKind(schema.GroupVersionKind{
3172+
Group: "apps",
3173+
Kind: "Deployment",
3174+
Version: "v1",
3175+
})
3176+
actual.SetName(dep.Name)
3177+
key := client.ObjectKey{Namespace: dep.Namespace, Name: dep.Name}
3178+
Expect(dReader.Get(context.TODO(), key, actual)).To(Succeed())
3179+
Expect(1).To(Equal(cachedReader.Called))
3180+
})
31513181
})
31523182
})
31533183
Describe("List", func() {
@@ -3165,24 +3195,46 @@ var _ = Describe("DelegatingClient", func() {
31653195
Expect(1).To(Equal(cachedReader.Called))
31663196
})
31673197

3168-
It("should call client reader when unstructured object", func() {
3169-
cachedReader := &fakeReader{}
3170-
cl, err := client.New(cfg, client.Options{})
3171-
Expect(err).NotTo(HaveOccurred())
3172-
dReader, err := client.NewDelegatingClient(client.NewDelegatingClientInput{
3173-
CacheReader: cachedReader,
3174-
Client: cl,
3198+
When("listing unstructured objects", func() {
3199+
It("should call client reader when not cached", func() {
3200+
cachedReader := &fakeReader{}
3201+
cl, err := client.New(cfg, client.Options{})
3202+
Expect(err).NotTo(HaveOccurred())
3203+
dReader, err := client.NewDelegatingClient(client.NewDelegatingClientInput{
3204+
CacheReader: cachedReader,
3205+
Client: cl,
3206+
})
3207+
Expect(err).NotTo(HaveOccurred())
3208+
3209+
actual := &unstructured.UnstructuredList{}
3210+
actual.SetGroupVersionKind(schema.GroupVersionKind{
3211+
Group: "apps",
3212+
Kind: "DeploymentList",
3213+
Version: "v1",
3214+
})
3215+
Expect(dReader.List(context.Background(), actual)).To(Succeed())
3216+
Expect(0).To(Equal(cachedReader.Called))
31753217
})
3176-
Expect(err).NotTo(HaveOccurred())
3218+
It("should call cache reader when cached", func() {
3219+
cachedReader := &fakeReader{}
3220+
cl, err := client.New(cfg, client.Options{})
3221+
Expect(err).NotTo(HaveOccurred())
3222+
dReader, err := client.NewDelegatingClient(client.NewDelegatingClientInput{
3223+
CacheReader: cachedReader,
3224+
Client: cl,
3225+
CacheUnstructured: true,
3226+
})
3227+
Expect(err).NotTo(HaveOccurred())
31773228

3178-
actual := &unstructured.UnstructuredList{}
3179-
actual.SetGroupVersionKind(schema.GroupVersionKind{
3180-
Group: "apps",
3181-
Kind: "DeploymentList",
3182-
Version: "v1",
3229+
actual := &unstructured.UnstructuredList{}
3230+
actual.SetGroupVersionKind(schema.GroupVersionKind{
3231+
Group: "apps",
3232+
Kind: "DeploymentList",
3233+
Version: "v1",
3234+
})
3235+
Expect(dReader.List(context.Background(), actual)).To(Succeed())
3236+
Expect(1).To(Equal(cachedReader.Called))
31833237
})
3184-
Expect(dReader.List(context.Background(), actual)).To(Succeed())
3185-
Expect(0).To(Equal(cachedReader.Called))
31863238
})
31873239
})
31883240
})

‎pkg/client/split.go‎

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,16 @@ import (
2424
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2525
"k8s.io/apimachinery/pkg/runtime"
2626
"k8s.io/apimachinery/pkg/runtime/schema"
27+
2728
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
2829
)
2930

3031
// NewDelegatingClientInput encapsulates the input parameters to create a new delegating client.
3132
type NewDelegatingClientInput struct {
32-
CacheReader Reader
33-
Client Client
34-
UncachedObjects []Object
33+
CacheReader Reader
34+
Client Client
35+
UncachedObjects []Object
36+
CacheUnstructured bool
3537
}
3638

3739
// NewDelegatingClient creates a new delegating client.
@@ -53,10 +55,11 @@ func NewDelegatingClient(in NewDelegatingClientInput) (Client, error) {
5355
scheme: in.Client.Scheme(),
5456
mapper: in.Client.RESTMapper(),
5557
Reader: &delegatingReader{
56-
CacheReader: in.CacheReader,
57-
ClientReader: in.Client,
58-
scheme: in.Client.Scheme(),
59-
uncachedGVKs: uncachedGVKs,
58+
CacheReader: in.CacheReader,
59+
ClientReader: in.Client,
60+
scheme: in.Client.Scheme(),
61+
uncachedGVKs: uncachedGVKs,
62+
cacheUnstructured: in.CacheUnstructured,
6063
},
6164
Writer: in.Client,
6265
StatusClient: in.Client,
@@ -91,8 +94,9 @@ type delegatingReader struct {
9194
CacheReader Reader
9295
ClientReader Reader
9396

94-
uncachedGVKs map[schema.GroupVersionKind]struct{}
95-
scheme *runtime.Scheme
97+
uncachedGVKs map[schema.GroupVersionKind]struct{}
98+
scheme *runtime.Scheme
99+
cacheUnstructured bool
96100
}
97101

98102
func (d *delegatingReader) shouldBypassCache(obj runtime.Object) (bool, error) {
@@ -105,10 +109,15 @@ func (d *delegatingReader) shouldBypassCache(obj runtime.Object) (bool, error) {
105109
if meta.IsListType(obj) {
106110
gvk.Kind = strings.TrimSuffix(gvk.Kind, "List")
107111
}
108-
_, isUncached := d.uncachedGVKs[gvk]
109-
_, isUnstructured := obj.(*unstructured.Unstructured)
110-
_, isUnstructuredList := obj.(*unstructured.UnstructuredList)
111-
return isUncached || isUnstructured || isUnstructuredList, nil
112+
if _, isUncached := d.uncachedGVKs[gvk]; isUncached {
113+
return true, nil
114+
}
115+
if !d.cacheUnstructured {
116+
_, isUnstructured := obj.(*unstructured.Unstructured)
117+
_, isUnstructuredList := obj.(*unstructured.UnstructuredList)
118+
return isUnstructured || isUnstructuredList, nil
119+
}
120+
return false, nil
112121
}
113122

114123
// Get retrieves an obj for a given object key from the Kubernetes Cluster.

0 commit comments

Comments
 (0)