Skip to content

Commit 806abef

Browse files
committed
Minor improvements to godoc, code style in cache pkg
1 parent b1d6919 commit 806abef

File tree

8 files changed

+111
-163
lines changed

8 files changed

+111
-163
lines changed

pkg/cache/cache.go

+26-27
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,10 @@ var (
4747
// to receive events for Kubernetes objects (at a low-level),
4848
// and add indices to fields on the objects stored in the cache.
4949
type Cache interface {
50-
// Cache acts as a client to objects stored in the cache.
50+
// Reader acts as a client to objects stored in the cache.
5151
client.Reader
5252

53-
// Cache loads informers and adds field indices.
53+
// Informers loads informers and adds field indices.
5454
Informers
5555
}
5656

@@ -70,39 +70,43 @@ type Informers interface {
7070
// It blocks.
7171
Start(ctx context.Context) error
7272

73-
// WaitForCacheSync waits for all the caches to sync. Returns false if it could not sync a cache.
73+
// WaitForCacheSync waits for all the caches to sync. Returns false if it could not sync a cache.
7474
WaitForCacheSync(ctx context.Context) bool
7575

76-
// Informers knows how to add indices to the caches (informers) that it manages.
76+
// FieldIndexer adds indices to the managed informers.
7777
client.FieldIndexer
7878
}
7979

80-
// Informer - informer allows you interact with the underlying informer.
80+
// Informer allows you to interact with the underlying informer.
8181
type Informer interface {
8282
// AddEventHandler adds an event handler to the shared informer using the shared informer's resync
83-
// period. Events to a single handler are delivered sequentially, but there is no coordination
83+
// period. Events to a single handler are delivered sequentially, but there is no coordination
8484
// between different handlers.
8585
// It returns a registration handle for the handler that can be used to remove
86-
// the handler again.
86+
// the handler again and an error if the handler cannot be added.
8787
AddEventHandler(handler toolscache.ResourceEventHandler) (toolscache.ResourceEventHandlerRegistration, error)
88+
8889
// AddEventHandlerWithResyncPeriod adds an event handler to the shared informer using the
89-
// specified resync period. Events to a single handler are delivered sequentially, but there is
90+
// specified resync period. Events to a single handler are delivered sequentially, but there is
9091
// no coordination between different handlers.
9192
// It returns a registration handle for the handler that can be used to remove
9293
// the handler again and an error if the handler cannot be added.
9394
AddEventHandlerWithResyncPeriod(handler toolscache.ResourceEventHandler, resyncPeriod time.Duration) (toolscache.ResourceEventHandlerRegistration, error)
94-
// RemoveEventHandler removes a formerly added event handler given by
95+
96+
// RemoveEventHandler removes a previously added event handler given by
9597
// its registration handle.
96-
// This function is guaranteed to be idempotent, and thread-safe.
98+
// This function is guaranteed to be idempotent and thread-safe.
9799
RemoveEventHandler(handle toolscache.ResourceEventHandlerRegistration) error
98-
// AddIndexers adds more indexers to this store. If you call this after you already have data
100+
101+
// AddIndexers adds indexers to this store. If this is called after there is already data
99102
// in the store, the results are undefined.
100103
AddIndexers(indexers toolscache.Indexers) error
104+
101105
// HasSynced return true if the informers underlying store has synced.
102106
HasSynced() bool
103107
}
104108

105-
// Options are the optional arguments for creating a new InformersMap object.
109+
// Options are the optional arguments for creating a new Cache object.
106110
type Options struct {
107111
// HTTPClient is the http client to use for the REST client
108112
HTTPClient *http.Client
@@ -141,31 +145,31 @@ type Options struct {
141145
SyncPeriod *time.Duration
142146

143147
// Namespaces restricts the cache's ListWatch to the desired namespaces
144-
// Default watches all namespaces
148+
// Per default ListWatch watches all namespaces
145149
Namespaces []string
146150

147-
// DefaultLabelSelector will be used as a label selectors for all object types
151+
// DefaultLabelSelector will be used as a label selector for all object types
148152
// unless they have a more specific selector set in ByObject.
149153
DefaultLabelSelector labels.Selector
150154

151-
// DefaultFieldSelector will be used as a field selectors for all object types
155+
// DefaultFieldSelector will be used as a field selector for all object types
152156
// unless they have a more specific selector set in ByObject.
153157
DefaultFieldSelector fields.Selector
154158

155159
// DefaultTransform will be used as transform for all object types
156160
// unless they have a more specific transform set in ByObject.
157161
DefaultTransform toolscache.TransformFunc
158162

159-
// ByObject restricts the cache's ListWatch to the desired fields per GVK at the specified object.
160-
ByObject map[client.Object]ByObject
161-
162163
// UnsafeDisableDeepCopy indicates not to deep copy objects during get or
163164
// list objects for EVERY object.
164165
// Be very careful with this, when enabled you must DeepCopy any object before mutating it,
165166
// otherwise you will mutate the object in the cache.
166167
//
167168
// This is a global setting for all objects, and can be overridden by the ByObject setting.
168169
UnsafeDisableDeepCopy *bool
170+
171+
// ByObject restricts the cache's ListWatch to the desired fields per GVK at the specified object.
172+
ByObject map[client.Object]ByObject
169173
}
170174

171175
// ByObject offers more fine-grained control over the cache's ListWatch by object.
@@ -176,9 +180,8 @@ type ByObject struct {
176180
// Field represents a field selector for the object.
177181
Field fields.Selector
178182

179-
// Transform is a map from objects to transformer functions which
180-
// get applied when objects of the transformation are about to be committed
181-
// to cache.
183+
// Transform is a transformer function for the object which gets applied
184+
// when objects of the transformation are about to be committed to the cache.
182185
//
183186
// This function is called both for new objects to enter the cache,
184187
// and for updated objects.
@@ -236,15 +239,12 @@ func New(config *rest.Config, opts Options) (Cache, error) {
236239
}
237240

238241
func defaultOpts(config *rest.Config, opts Options) (Options, error) {
239-
logger := log.WithName("setup")
240-
241242
// Use the rest HTTP client for the provided config if unset
242243
if opts.HTTPClient == nil {
243244
var err error
244245
opts.HTTPClient, err = rest.HTTPClientFor(config)
245246
if err != nil {
246-
logger.Error(err, "Failed to get HTTP client")
247-
return opts, fmt.Errorf("could not create HTTP client from config: %w", err)
247+
return Options{}, fmt.Errorf("could not create HTTP client from config: %w", err)
248248
}
249249
}
250250

@@ -258,8 +258,7 @@ func defaultOpts(config *rest.Config, opts Options) (Options, error) {
258258
var err error
259259
opts.Mapper, err = apiutil.NewDiscoveryRESTMapper(config, opts.HTTPClient)
260260
if err != nil {
261-
logger.Error(err, "Failed to get API Group-Resources")
262-
return opts, fmt.Errorf("could not create RESTMapper from config: %w", err)
261+
return Options{}, fmt.Errorf("could not create RESTMapper from config: %w", err)
263262
}
264263
}
265264

pkg/cache/informer_cache.go

+11-10
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"k8s.io/apimachinery/pkg/runtime"
2828
"k8s.io/apimachinery/pkg/runtime/schema"
2929
"k8s.io/client-go/tools/cache"
30+
3031
"sigs.k8s.io/controller-runtime/pkg/cache/internal"
3132
"sigs.k8s.io/controller-runtime/pkg/client"
3233
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
@@ -67,7 +68,7 @@ func (ic *informerCache) Get(ctx context.Context, key client.ObjectKey, out clie
6768
if !started {
6869
return &ErrCacheNotStarted{}
6970
}
70-
return cache.Reader.Get(ctx, key, out)
71+
return cache.Reader.Get(ctx, key, out, opts...)
7172
}
7273

7374
// List implements Reader.
@@ -135,7 +136,7 @@ func (ic *informerCache) GetInformerForKind(ctx context.Context, gvk schema.Grou
135136
if err != nil {
136137
return nil, err
137138
}
138-
return i.Informer, err
139+
return i.Informer, nil
139140
}
140141

141142
// GetInformer returns the informer for the obj.
@@ -149,7 +150,7 @@ func (ic *informerCache) GetInformer(ctx context.Context, obj client.Object) (In
149150
if err != nil {
150151
return nil, err
151152
}
152-
return i.Informer, err
153+
return i.Informer, nil
153154
}
154155

155156
// NeedLeaderElection implements the LeaderElectionRunnable interface
@@ -158,11 +159,11 @@ func (ic *informerCache) NeedLeaderElection() bool {
158159
return false
159160
}
160161

161-
// IndexField adds an indexer to the underlying cache, using extraction function to get
162-
// value(s) from the given field. This index can then be used by passing a field selector
162+
// IndexField adds an indexer to the underlying informer, using extractValue function to get
163+
// value(s) from the given field. This index can then be used by passing a field selector
163164
// to List. For one-to-one compatibility with "normal" field selectors, only return one value.
164-
// The values may be anything. They will automatically be prefixed with the namespace of the
165-
// given object, if present. The objects passed are guaranteed to be objects of the correct type.
165+
// The values may be anything. They will automatically be prefixed with the namespace of the
166+
// given object, if present. The objects passed are guaranteed to be objects of the correct type.
166167
func (ic *informerCache) IndexField(ctx context.Context, obj client.Object, field string, extractValue client.IndexerFunc) error {
167168
informer, err := ic.GetInformer(ctx, obj)
168169
if err != nil {
@@ -171,7 +172,7 @@ func (ic *informerCache) IndexField(ctx context.Context, obj client.Object, fiel
171172
return indexByField(informer, field, extractValue)
172173
}
173174

174-
func indexByField(indexer Informer, field string, extractor client.IndexerFunc) error {
175+
func indexByField(informer Informer, field string, extractValue client.IndexerFunc) error {
175176
indexFunc := func(objRaw interface{}) ([]string, error) {
176177
// TODO(directxman12): check if this is the correct type?
177178
obj, isObj := objRaw.(client.Object)
@@ -184,7 +185,7 @@ func indexByField(indexer Informer, field string, extractor client.IndexerFunc)
184185
}
185186
ns := meta.GetNamespace()
186187

187-
rawVals := extractor(obj)
188+
rawVals := extractValue(obj)
188189
var vals []string
189190
if ns == "" {
190191
// if we're not doubling the keys for the namespaced case, just create a new slice with same length
@@ -207,5 +208,5 @@ func indexByField(indexer Informer, field string, extractor client.IndexerFunc)
207208
return vals, nil
208209
}
209210

210-
return indexer.AddIndexers(cache.Indexers{internal.FieldIndexName(field): indexFunc})
211+
return informer.AddIndexers(cache.Indexers{internal.FieldIndexName(field): indexFunc})
211212
}

pkg/cache/informertest/fake_cache.go

+4-18
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"k8s.io/apimachinery/pkg/runtime/schema"
2424
"k8s.io/client-go/kubernetes/scheme"
2525
toolscache "k8s.io/client-go/tools/cache"
26+
2627
"sigs.k8s.io/controller-runtime/pkg/cache"
2728
"sigs.k8s.io/controller-runtime/pkg/client"
2829
"sigs.k8s.io/controller-runtime/pkg/controller/controllertest"
@@ -52,14 +53,7 @@ func (c *FakeInformers) GetInformerForKind(ctx context.Context, gvk schema.Group
5253

5354
// FakeInformerForKind implements Informers.
5455
func (c *FakeInformers) FakeInformerForKind(ctx context.Context, gvk schema.GroupVersionKind) (*controllertest.FakeInformer, error) {
55-
if c.Scheme == nil {
56-
c.Scheme = scheme.Scheme
57-
}
58-
obj, err := c.Scheme.New(gvk)
59-
if err != nil {
60-
return nil, err
61-
}
62-
i, err := c.informerFor(gvk, obj)
56+
i, err := c.GetInformerForKind(ctx, gvk)
6357
if err != nil {
6458
return nil, err
6559
}
@@ -88,16 +82,8 @@ func (c *FakeInformers) WaitForCacheSync(ctx context.Context) bool {
8882
}
8983

9084
// FakeInformerFor implements Informers.
91-
func (c *FakeInformers) FakeInformerFor(obj runtime.Object) (*controllertest.FakeInformer, error) {
92-
if c.Scheme == nil {
93-
c.Scheme = scheme.Scheme
94-
}
95-
gvks, _, err := c.Scheme.ObjectKinds(obj)
96-
if err != nil {
97-
return nil, err
98-
}
99-
gvk := gvks[0]
100-
i, err := c.informerFor(gvk, obj)
85+
func (c *FakeInformers) FakeInformerFor(ctx context.Context, obj client.Object) (*controllertest.FakeInformer, error) {
86+
i, err := c.GetInformer(ctx, obj)
10187
if err != nil {
10288
return nil, err
10389
}

pkg/cache/internal/cache_reader.go

+7-7
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ type CacheReader struct {
5353
}
5454

5555
// Get checks the indexer for the object and writes a copy of it if found.
56-
func (c *CacheReader) Get(_ context.Context, key client.ObjectKey, out client.Object, opts ...client.GetOption) error {
56+
func (c *CacheReader) Get(_ context.Context, key client.ObjectKey, out client.Object, _ ...client.GetOption) error {
5757
if c.scopeName == apimeta.RESTScopeNameRoot {
5858
key.Namespace = ""
5959
}
@@ -67,9 +67,9 @@ func (c *CacheReader) Get(_ context.Context, key client.ObjectKey, out client.Ob
6767

6868
// Not found, return an error
6969
if !exists {
70-
// Resource gets transformed into Kind in the error anyway, so this is fine
7170
return apierrors.NewNotFound(schema.GroupResource{
72-
Group: c.groupVersionKind.Group,
71+
Group: c.groupVersionKind.Group,
72+
// Resource gets set as Kind in the error so this is fine
7373
Resource: c.groupVersionKind.Kind,
7474
}, key.Name)
7575
}
@@ -119,8 +119,8 @@ func (c *CacheReader) List(_ context.Context, out client.ObjectList, opts ...cli
119119
if !requiresExact {
120120
return fmt.Errorf("non-exact field matches are not supported by the cache")
121121
}
122-
// list all objects by the field selector. If this is namespaced and we have one, ask for the
123-
// namespaced index key. Otherwise, ask for the non-namespaced variant by using the fake "all namespaces"
122+
// list all objects by the field selector. If this is namespaced and we have one, ask for the
123+
// namespaced index key. Otherwise, ask for the non-namespaced variant by using the fake "all namespaces"
124124
// namespace.
125125
objs, err = c.indexer.ByIndex(FieldIndexName(field), KeyToNamespacedKey(listOpts.Namespace, val))
126126
case listOpts.Namespace != "":
@@ -175,7 +175,7 @@ func (c *CacheReader) List(_ context.Context, out client.ObjectList, opts ...cli
175175
}
176176

177177
// objectKeyToStorageKey converts an object key to store key.
178-
// It's akin to MetaNamespaceKeyFunc. It's separate from
178+
// It's akin to MetaNamespaceKeyFunc. It's separate from
179179
// String to allow keeping the key format easily in sync with
180180
// MetaNamespaceKeyFunc.
181181
func objectKeyToStoreKey(k client.ObjectKey) string {
@@ -191,7 +191,7 @@ func FieldIndexName(field string) string {
191191
return "field:" + field
192192
}
193193

194-
// noNamespaceNamespace is used as the "namespace" when we want to list across all namespaces.
194+
// allNamespacesNamespace is used as the "namespace" when we want to list across all namespaces.
195195
const allNamespacesNamespace = "__all_namespaces"
196196

197197
// KeyToNamespacedKey prefixes the given index key with a namespace

pkg/cache/internal/informers.go

+6-4
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import (
3535
"k8s.io/client-go/metadata"
3636
"k8s.io/client-go/rest"
3737
"k8s.io/client-go/tools/cache"
38+
3839
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
3940
)
4041

@@ -48,7 +49,7 @@ type InformersOpts struct {
4849
ByGVK map[schema.GroupVersionKind]InformersOptsByGVK
4950
}
5051

51-
// InformersOptsByGVK configured additional by group version kind (or object)
52+
// InformersOptsByGVK configures additionally by group version kind (or object)
5253
// in an InformerMap.
5354
type InformersOptsByGVK struct {
5455
Selector Selector
@@ -186,7 +187,7 @@ func (ip *Informers) getDisableDeepCopy(gvk schema.GroupVersionKind) bool {
186187
return false
187188
}
188189

189-
// Start calls Run on each of the informers and sets started to true. Blocks on the context.
190+
// Start calls Run on each of the informers and sets started to true. Blocks on the context.
190191
// It doesn't return start because it can't return an error, and it's not a runnable directly.
191192
func (ip *Informers) Start(ctx context.Context) error {
192193
func() {
@@ -278,7 +279,7 @@ func (ip *Informers) get(gvk schema.GroupVersionKind, obj runtime.Object) (res *
278279
return i, ip.started, ok
279280
}
280281

281-
// Get will create a new Informer and add it to the map of specificInformersMap if none exists. Returns
282+
// Get will create a new Informer and add it to the map of specificInformersMap if none exists. Returns
282283
// the Informer from the map.
283284
func (ip *Informers) Get(ctx context.Context, gvk schema.GroupVersionKind, obj runtime.Object) (bool, *Cache, error) {
284285
// Return the informer if it is found
@@ -311,11 +312,12 @@ func (ip *Informers) informersByType(obj runtime.Object) map[schema.GroupVersion
311312
}
312313
}
313314

315+
// addInformerToMap either returns an existing informer or creates a new informer, adds it to the map and returns it.
314316
func (ip *Informers) addInformerToMap(gvk schema.GroupVersionKind, obj runtime.Object) (*Cache, bool, error) {
315317
ip.mu.Lock()
316318
defer ip.mu.Unlock()
317319

318-
// Check the cache to see if we already have an Informer. If we do, return the Informer.
320+
// Check the cache to see if we already have an Informer. If we do, return the Informer.
319321
// This is for the case where 2 routines tried to get the informer when it wasn't in the map
320322
// so neither returned early, but the first one created it.
321323
if i, ok := ip.informersByType(obj)[gvk]; ok {

0 commit comments

Comments
 (0)