Skip to content

Commit c20ea14

Browse files
maxsmythesbueringer
maxsmythe
and
sbueringer
authored
✨ Allow non-blocking retrieval of informers (#2371)
* Allow non-blocking retrieval of informers Signed-off-by: Max Smythe <[email protected]> Re-organize functional arguments Signed-off-by: Max Smythe <[email protected]> Add unit tests Signed-off-by: Max Smythe <[email protected]> Add deferred cancel call to test Signed-off-by: Max Smythe <[email protected]> Run gofmt Signed-off-by: Max Smythe <[email protected]> * Update pkg/cache/internal/informers.go Co-authored-by: Stefan Büringer <[email protected]> * Update pkg/cache/internal/informers.go Co-authored-by: Stefan Büringer <[email protected]> * Alias functional options * Use private option for newInformer override * Fix lint errors --------- Signed-off-by: Max Smythe <[email protected]> Co-authored-by: Stefan Büringer <[email protected]>
1 parent 304027b commit c20ea14

File tree

9 files changed

+163
-27
lines changed

9 files changed

+163
-27
lines changed

‎pkg/builder/controller_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -576,15 +576,15 @@ type nonTypedOnlyCache struct {
576576
cache.Cache
577577
}
578578

579-
func (c *nonTypedOnlyCache) GetInformer(ctx context.Context, obj client.Object) (cache.Informer, error) {
579+
func (c *nonTypedOnlyCache) GetInformer(ctx context.Context, obj client.Object, opts ...cache.InformerGetOption) (cache.Informer, error) {
580580
switch obj.(type) {
581581
case (*metav1.PartialObjectMetadata):
582-
return c.Cache.GetInformer(ctx, obj)
582+
return c.Cache.GetInformer(ctx, obj, opts...)
583583
default:
584584
return nil, fmt.Errorf("did not want to provide an informer for normal type %T", obj)
585585
}
586586
}
587-
func (c *nonTypedOnlyCache) GetInformerForKind(ctx context.Context, gvk schema.GroupVersionKind) (cache.Informer, error) {
587+
func (c *nonTypedOnlyCache) GetInformerForKind(ctx context.Context, gvk schema.GroupVersionKind, opts ...cache.InformerGetOption) (cache.Informer, error) {
588588
return nil, fmt.Errorf("don't try to sidestep the restriction on informer types by calling GetInformerForKind")
589589
}
590590

‎pkg/cache/cache.go

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,20 @@ var (
4444
defaultSyncPeriod = 10 * time.Hour
4545
)
4646

47+
// InformerGetOptions defines the behavior of how informers are retrieved.
48+
type InformerGetOptions internal.GetOptions
49+
50+
// InformerGetOption defines an option that alters the behavior of how informers are retrieved.
51+
type InformerGetOption func(*InformerGetOptions)
52+
53+
// BlockUntilSynced determines whether a get request for an informer should block
54+
// until the informer's cache has synced.
55+
func BlockUntilSynced(shouldBlock bool) InformerGetOption {
56+
return func(opts *InformerGetOptions) {
57+
opts.BlockUntilSynced = &shouldBlock
58+
}
59+
}
60+
4761
// Cache knows how to load Kubernetes objects, fetch informers to request
4862
// to receive events for Kubernetes objects (at a low-level),
4963
// and add indices to fields on the objects stored in the cache.
@@ -61,11 +75,11 @@ type Cache interface {
6175
type Informers interface {
6276
// GetInformer fetches or constructs an informer for the given object that corresponds to a single
6377
// API kind and resource.
64-
GetInformer(ctx context.Context, obj client.Object) (Informer, error)
78+
GetInformer(ctx context.Context, obj client.Object, opts ...InformerGetOption) (Informer, error)
6579

6680
// GetInformerForKind is similar to GetInformer, except that it takes a group-version-kind, instead
6781
// of the underlying object.
68-
GetInformerForKind(ctx context.Context, gvk schema.GroupVersionKind) (Informer, error)
82+
GetInformerForKind(ctx context.Context, gvk schema.GroupVersionKind, opts ...InformerGetOption) (Informer, error)
6983

7084
// Start runs all the informers known to this cache until the context is closed.
7185
// It blocks.
@@ -187,6 +201,9 @@ type Options struct {
187201
// ByObject restricts the cache's ListWatch to the desired fields per GVK at the specified object.
188202
// object, this will fall through to Default* settings.
189203
ByObject map[client.Object]ByObject
204+
205+
// newInformer allows overriding of NewSharedIndexInformer for testing.
206+
newInformer *func(toolscache.ListerWatcher, runtime.Object, time.Duration, toolscache.Indexers) toolscache.SharedIndexInformer
190207
}
191208

192209
// ByObject offers more fine-grained control over the cache's ListWatch by object.
@@ -337,6 +354,7 @@ func newCache(restConfig *rest.Config, opts Options) newCacheFunc {
337354
},
338355
Transform: config.Transform,
339356
UnsafeDisableDeepCopy: pointer.BoolDeref(config.UnsafeDisableDeepCopy, false),
357+
NewInformer: opts.newInformer,
340358
}),
341359
readerFailOnMissingInformer: opts.ReaderFailOnMissingInformer,
342360
}

‎pkg/cache/cache_test.go

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"sort"
2525
"strconv"
2626
"strings"
27+
"time"
2728

2829
. "github.com/onsi/ginkgo/v2"
2930
. "github.com/onsi/gomega"
@@ -43,6 +44,7 @@ import (
4344

4445
"sigs.k8s.io/controller-runtime/pkg/cache"
4546
"sigs.k8s.io/controller-runtime/pkg/client"
47+
"sigs.k8s.io/controller-runtime/pkg/controller/controllertest"
4648
)
4749

4850
const testNodeOne = "test-node-1"
@@ -117,6 +119,7 @@ func deletePod(pod client.Object) {
117119

118120
var _ = Describe("Informer Cache", func() {
119121
CacheTest(cache.New, cache.Options{})
122+
NonBlockingGetTest(cache.New, cache.Options{})
120123
})
121124

122125
var _ = Describe("Informer Cache with ReaderFailOnMissingInformer", func() {
@@ -131,12 +134,22 @@ var _ = Describe("Multi-Namespace Informer Cache", func() {
131134
"default": {},
132135
},
133136
})
137+
NonBlockingGetTest(cache.New, cache.Options{
138+
DefaultNamespaces: map[string]cache.Config{
139+
testNamespaceOne: {},
140+
testNamespaceTwo: {},
141+
"default": {},
142+
},
143+
})
134144
})
135145

136146
var _ = Describe("Informer Cache without global DeepCopy", func() {
137147
CacheTest(cache.New, cache.Options{
138148
DefaultUnsafeDisableDeepCopy: pointer.Bool(true),
139149
})
150+
NonBlockingGetTest(cache.New, cache.Options{
151+
DefaultUnsafeDisableDeepCopy: pointer.Bool(true),
152+
})
140153
})
141154

142155
var _ = Describe("Cache with transformers", func() {
@@ -440,7 +453,6 @@ func CacheTestReaderFailOnMissingInformer(createCacheFunc func(config *rest.Conf
440453
BeforeEach(func() {
441454
informerCacheCtx, informerCacheCancel = context.WithCancel(context.Background())
442455
Expect(cfg).NotTo(BeNil())
443-
444456
By("creating the informer cache")
445457
var err error
446458
informerCache, err = createCacheFunc(cfg, opts)
@@ -507,6 +519,83 @@ func CacheTestReaderFailOnMissingInformer(createCacheFunc func(config *rest.Conf
507519
})
508520
}
509521

522+
func NonBlockingGetTest(createCacheFunc func(config *rest.Config, opts cache.Options) (cache.Cache, error), opts cache.Options) {
523+
Describe("non-blocking get test", func() {
524+
var (
525+
informerCache cache.Cache
526+
informerCacheCtx context.Context
527+
informerCacheCancel context.CancelFunc
528+
)
529+
BeforeEach(func() {
530+
informerCacheCtx, informerCacheCancel = context.WithCancel(context.Background())
531+
Expect(cfg).NotTo(BeNil())
532+
533+
By("creating expected namespaces")
534+
cl, err := client.New(cfg, client.Options{})
535+
Expect(err).NotTo(HaveOccurred())
536+
err = ensureNode(testNodeOne, cl)
537+
Expect(err).NotTo(HaveOccurred())
538+
err = ensureNamespace(testNamespaceOne, cl)
539+
Expect(err).NotTo(HaveOccurred())
540+
err = ensureNamespace(testNamespaceTwo, cl)
541+
Expect(err).NotTo(HaveOccurred())
542+
err = ensureNamespace(testNamespaceThree, cl)
543+
Expect(err).NotTo(HaveOccurred())
544+
545+
By("creating the informer cache")
546+
v := reflect.ValueOf(&opts).Elem()
547+
newInformerField := v.FieldByName("newInformer")
548+
newFakeInformer := func(_ kcache.ListerWatcher, _ runtime.Object, _ time.Duration, _ kcache.Indexers) kcache.SharedIndexInformer {
549+
return &controllertest.FakeInformer{Synced: false}
550+
}
551+
reflect.NewAt(newInformerField.Type(), newInformerField.Addr().UnsafePointer()).
552+
Elem().
553+
Set(reflect.ValueOf(&newFakeInformer))
554+
informerCache, err = createCacheFunc(cfg, opts)
555+
Expect(err).NotTo(HaveOccurred())
556+
By("running the cache and waiting for it to sync")
557+
// pass as an arg so that we don't race between close and re-assign
558+
go func(ctx context.Context) {
559+
defer GinkgoRecover()
560+
Expect(informerCache.Start(ctx)).To(Succeed())
561+
}(informerCacheCtx)
562+
Expect(informerCache.WaitForCacheSync(informerCacheCtx)).To(BeTrue())
563+
})
564+
565+
AfterEach(func() {
566+
By("cleaning up created pods")
567+
informerCacheCancel()
568+
})
569+
570+
Describe("as an Informer", func() {
571+
It("should be able to get informer for the object without blocking", func() {
572+
By("getting a shared index informer for a pod")
573+
pod := &corev1.Pod{
574+
ObjectMeta: metav1.ObjectMeta{
575+
Name: "informer-obj",
576+
Namespace: "default",
577+
},
578+
Spec: corev1.PodSpec{
579+
Containers: []corev1.Container{
580+
{
581+
Name: "nginx",
582+
Image: "nginx",
583+
},
584+
},
585+
},
586+
}
587+
588+
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
589+
defer cancel()
590+
sii, err := informerCache.GetInformer(ctx, pod, cache.BlockUntilSynced(false))
591+
Expect(err).NotTo(HaveOccurred())
592+
Expect(sii).NotTo(BeNil())
593+
Expect(sii.HasSynced()).To(BeFalse())
594+
})
595+
})
596+
})
597+
}
598+
510599
func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (cache.Cache, error), opts cache.Options) {
511600
Describe("Cache test", func() {
512601
var (

‎pkg/cache/delegating_by_gvk_cache.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,16 +52,16 @@ func (dbt *delegatingByGVKCache) List(ctx context.Context, list client.ObjectLis
5252
return cache.List(ctx, list, opts...)
5353
}
5454

55-
func (dbt *delegatingByGVKCache) GetInformer(ctx context.Context, obj client.Object) (Informer, error) {
55+
func (dbt *delegatingByGVKCache) GetInformer(ctx context.Context, obj client.Object, opts ...InformerGetOption) (Informer, error) {
5656
cache, err := dbt.cacheForObject(obj)
5757
if err != nil {
5858
return nil, err
5959
}
60-
return cache.GetInformer(ctx, obj)
60+
return cache.GetInformer(ctx, obj, opts...)
6161
}
6262

63-
func (dbt *delegatingByGVKCache) GetInformerForKind(ctx context.Context, gvk schema.GroupVersionKind) (Informer, error) {
64-
return dbt.cacheForGVK(gvk).GetInformerForKind(ctx, gvk)
63+
func (dbt *delegatingByGVKCache) GetInformerForKind(ctx context.Context, gvk schema.GroupVersionKind, opts ...InformerGetOption) (Informer, error) {
64+
return dbt.cacheForGVK(gvk).GetInformerForKind(ctx, gvk, opts...)
6565
}
6666

6767
func (dbt *delegatingByGVKCache) Start(ctx context.Context) error {

‎pkg/cache/informer_cache.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -141,29 +141,37 @@ func (ic *informerCache) objectTypeForListObject(list client.ObjectList) (*schem
141141
return &gvk, cacheTypeObj, nil
142142
}
143143

144+
func applyGetOptions(opts ...InformerGetOption) *internal.GetOptions {
145+
cfg := &InformerGetOptions{}
146+
for _, opt := range opts {
147+
opt(cfg)
148+
}
149+
return (*internal.GetOptions)(cfg)
150+
}
151+
144152
// GetInformerForKind returns the informer for the GroupVersionKind. If no informer exists, one will be started.
145-
func (ic *informerCache) GetInformerForKind(ctx context.Context, gvk schema.GroupVersionKind) (Informer, error) {
153+
func (ic *informerCache) GetInformerForKind(ctx context.Context, gvk schema.GroupVersionKind, opts ...InformerGetOption) (Informer, error) {
146154
// Map the gvk to an object
147155
obj, err := ic.scheme.New(gvk)
148156
if err != nil {
149157
return nil, err
150158
}
151159

152-
_, i, err := ic.Informers.Get(ctx, gvk, obj)
160+
_, i, err := ic.Informers.Get(ctx, gvk, obj, applyGetOptions(opts...))
153161
if err != nil {
154162
return nil, err
155163
}
156164
return i.Informer, nil
157165
}
158166

159167
// GetInformer returns the informer for the obj. If no informer exists, one will be started.
160-
func (ic *informerCache) GetInformer(ctx context.Context, obj client.Object) (Informer, error) {
168+
func (ic *informerCache) GetInformer(ctx context.Context, obj client.Object, opts ...InformerGetOption) (Informer, error) {
161169
gvk, err := apiutil.GVKForObject(obj, ic.scheme)
162170
if err != nil {
163171
return nil, err
164172
}
165173

166-
_, i, err := ic.Informers.Get(ctx, gvk, obj)
174+
_, i, err := ic.Informers.Get(ctx, gvk, obj, applyGetOptions(opts...))
167175
if err != nil {
168176
return nil, err
169177
}
@@ -179,7 +187,7 @@ func (ic *informerCache) getInformerForKind(ctx context.Context, gvk schema.Grou
179187
return started, cache, nil
180188
}
181189

182-
return ic.Informers.Get(ctx, gvk, obj)
190+
return ic.Informers.Get(ctx, gvk, obj, &internal.GetOptions{})
183191
}
184192

185193
// NeedLeaderElection implements the LeaderElectionRunnable interface

‎pkg/cache/informertest/fake_cache.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ type FakeInformers struct {
4040
}
4141

4242
// GetInformerForKind implements Informers.
43-
func (c *FakeInformers) GetInformerForKind(ctx context.Context, gvk schema.GroupVersionKind) (cache.Informer, error) {
43+
func (c *FakeInformers) GetInformerForKind(ctx context.Context, gvk schema.GroupVersionKind, opts ...cache.InformerGetOption) (cache.Informer, error) {
4444
if c.Scheme == nil {
4545
c.Scheme = scheme.Scheme
4646
}
@@ -61,7 +61,7 @@ func (c *FakeInformers) FakeInformerForKind(ctx context.Context, gvk schema.Grou
6161
}
6262

6363
// GetInformer implements Informers.
64-
func (c *FakeInformers) GetInformer(ctx context.Context, obj client.Object) (cache.Informer, error) {
64+
func (c *FakeInformers) GetInformer(ctx context.Context, obj client.Object, opts ...cache.InformerGetOption) (cache.Informer, error) {
6565
if c.Scheme == nil {
6666
c.Scheme = scheme.Scheme
6767
}

‎pkg/cache/internal/informers.go

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,18 @@ type InformersOpts struct {
4545
Mapper meta.RESTMapper
4646
ResyncPeriod time.Duration
4747
Namespace string
48+
NewInformer *func(cache.ListerWatcher, runtime.Object, time.Duration, cache.Indexers) cache.SharedIndexInformer
4849
Selector Selector
4950
Transform cache.TransformFunc
5051
UnsafeDisableDeepCopy bool
5152
}
5253

5354
// NewInformers creates a new InformersMap that can create informers under the hood.
5455
func NewInformers(config *rest.Config, options *InformersOpts) *Informers {
56+
newInformer := cache.NewSharedIndexInformer
57+
if options.NewInformer != nil {
58+
newInformer = *options.NewInformer
59+
}
5560
return &Informers{
5661
config: config,
5762
httpClient: options.HTTPClient,
@@ -70,6 +75,7 @@ func NewInformers(config *rest.Config, options *InformersOpts) *Informers {
7075
selector: options.Selector,
7176
transform: options.Transform,
7277
unsafeDisableDeepCopy: options.UnsafeDisableDeepCopy,
78+
newInformer: newInformer,
7379
}
7480
}
7581

@@ -88,6 +94,13 @@ type tracker struct {
8894
Metadata map[schema.GroupVersionKind]*Cache
8995
}
9096

97+
// GetOptions provides configuration to customize the behavior when
98+
// getting an informer.
99+
type GetOptions struct {
100+
// BlockUntilSynced controls if the informer retrieval will block until the informer is synced. Defaults to `true`.
101+
BlockUntilSynced *bool
102+
}
103+
91104
// Informers create and caches Informers for (runtime.Object, schema.GroupVersionKind) pairs.
92105
// It uses a standard parameter codec constructed based on the given generated Scheme.
93106
type Informers struct {
@@ -143,6 +156,9 @@ type Informers struct {
143156
selector Selector
144157
transform cache.TransformFunc
145158
unsafeDisableDeepCopy bool
159+
160+
// NewInformer allows overriding of the shared index informer constructor for testing.
161+
newInformer func(cache.ListerWatcher, runtime.Object, time.Duration, cache.Indexers) cache.SharedIndexInformer
146162
}
147163

148164
// Start calls Run on each of the informers and sets started to true. Blocks on the context.
@@ -240,7 +256,7 @@ func (ip *Informers) Peek(gvk schema.GroupVersionKind, obj runtime.Object) (res
240256

241257
// Get will create a new Informer and add it to the map of specificInformersMap if none exists. Returns
242258
// the Informer from the map.
243-
func (ip *Informers) Get(ctx context.Context, gvk schema.GroupVersionKind, obj runtime.Object) (bool, *Cache, error) {
259+
func (ip *Informers) Get(ctx context.Context, gvk schema.GroupVersionKind, obj runtime.Object, opts *GetOptions) (bool, *Cache, error) {
244260
// Return the informer if it is found
245261
i, started, ok := ip.Peek(gvk, obj)
246262
if !ok {
@@ -250,7 +266,12 @@ func (ip *Informers) Get(ctx context.Context, gvk schema.GroupVersionKind, obj r
250266
}
251267
}
252268

253-
if started && !i.Informer.HasSynced() {
269+
shouldBlock := true
270+
if opts.BlockUntilSynced != nil {
271+
shouldBlock = *opts.BlockUntilSynced
272+
}
273+
274+
if shouldBlock && started && !i.Informer.HasSynced() {
254275
// Wait for it to sync before returning the Informer so that folks don't read from a stale cache.
255276
if !cache.WaitForCacheSync(ctx.Done(), i.Informer.HasSynced) {
256277
return started, nil, apierrors.NewTimeoutError(fmt.Sprintf("failed waiting for %T Informer to sync", obj), 0)
@@ -288,7 +309,7 @@ func (ip *Informers) addInformerToMap(gvk schema.GroupVersionKind, obj runtime.O
288309
if err != nil {
289310
return nil, false, err
290311
}
291-
sharedIndexInformer := cache.NewSharedIndexInformer(&cache.ListWatch{
312+
sharedIndexInformer := ip.newInformer(&cache.ListWatch{
292313
ListFunc: func(opts metav1.ListOptions) (runtime.Object, error) {
293314
ip.selector.ApplyToList(&opts)
294315
return listWatcher.ListFunc(opts)

0 commit comments

Comments
 (0)