Skip to content

Commit

Permalink
Merge pull request #66 from ecordell/must-star
Browse files Browse the repository at this point in the history
Add non-panicing versions of ListerFor, IndexerFor
  • Loading branch information
ecordell authored May 1, 2024
2 parents 0091c10 + 596172c commit 510bcee
Show file tree
Hide file tree
Showing 11 changed files with 201 additions and 35 deletions.
4 changes: 2 additions & 2 deletions adopt/adopt.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,11 @@ const Owned = "owned"

var (
// AlwaysExistsFunc is an ExistsFunc that always returns nil
AlwaysExistsFunc ExistsFunc = func(ctx context.Context, nn types.NamespacedName) error {
AlwaysExistsFunc ExistsFunc = func(_ context.Context, _ types.NamespacedName) error {
return nil
}
// NoopObjectMissingFunc is an ObjectMissing func that does nothing
NoopObjectMissingFunc = func(ctx context.Context, err error) {}
NoopObjectMissingFunc = func(_ context.Context, _ error) {}
)

// AdoptionHandler implements handler.Handler to "adopt" an existing resource
Expand Down
10 changes: 5 additions & 5 deletions adopt/adopt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func TestSecretAdopterHandler(t *testing.T) {
err error
}

secretNotFound := func(name string) error {
secretNotFound := func(_ string) error {
return apierrors.NewNotFound(
corev1.SchemeGroupVersion.WithResource("secrets").GroupResource(),
"test")
Expand Down Expand Up @@ -324,21 +324,21 @@ func TestSecretAdopterHandler(t *testing.T) {
applyCallIndex := 0
s := NewSecretAdoptionHandler(
recorder,
func(ctx context.Context) (*corev1.Secret, error) {
func(_ context.Context) (*corev1.Secret, error) {
return tt.secretInCache, tt.cacheErr
},
func(ctx context.Context, err error) {
func(_ context.Context, err error) {
require.Equal(t, tt.expectObjectMissingErr, err)
},
typed.NewIndexer[*corev1.Secret](indexer),
func(ctx context.Context, secret *applycorev1.SecretApplyConfiguration, opts metav1.ApplyOptions) (result *corev1.Secret, err error) {
func(_ context.Context, secret *applycorev1.SecretApplyConfiguration, _ metav1.ApplyOptions) (result *corev1.Secret, err error) {
defer func() { applyCallIndex++ }()
call := tt.applyCalls[applyCallIndex]
call.called = true
require.Equal(t, call.input, secret, "error on call %d", applyCallIndex)
return call.result, call.err
},
func(ctx context.Context, nn types.NamespacedName) error {
func(_ context.Context, _ types.NamespacedName) error {
return tt.secretExistsErr
},
handler.NewHandlerFromFunc(func(ctx context.Context) {
Expand Down
2 changes: 1 addition & 1 deletion bootstrap/crds.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func waitForDiscovery(ctx context.Context, config *rest.Config, crds []*apiexten
return err
}

return wait.PollUntilContextTimeout(ctx, crdInstallPollInterval, maxCRDInstallTime, true, func(ctx context.Context) (done bool, err error) {
return wait.PollUntilContextTimeout(ctx, crdInstallPollInterval, maxCRDInstallTime, true, func(_ context.Context) (done bool, err error) {
_, serverGVRs, err := discoveryClient.ServerGroupsAndResources()
if err != nil {
return false, nil
Expand Down
10 changes: 5 additions & 5 deletions component/ensure_component_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func TestEnsureServiceHandler(t *testing.T) {
client := clientfake.NewSimpleDynamicClient(scheme, tt.existingServices...)
informerFactory := dynamicinformer.NewDynamicSharedInformerFactory(client, 0)
require.NoError(t, informerFactory.ForResource(serviceGVR).Informer().AddIndexers(map[string]cache.IndexFunc{
ownerIndex: func(obj interface{}) ([]string, error) {
ownerIndex: func(_ interface{}) ([]string, error) {
return []string{types.NamespacedName{Namespace: "test", Name: "owner"}.String()}, nil
},
}))
Expand All @@ -137,23 +137,23 @@ func TestEnsureServiceHandler(t *testing.T) {
NewIndexedComponent(
indexer,
ownerIndex,
func(ctx context.Context) labels.Selector {
func(_ context.Context) labels.Selector {
return labels.SelectorFromSet(map[string]string{
"example.com/component": "the-main-service-component",
})
}),
hash.NewObjectHash(), hashKey),
ctxOwner,
queueOps,
func(ctx context.Context, apply *applycorev1.ServiceApplyConfiguration) (*corev1.Service, error) {
func(_ context.Context, _ *applycorev1.ServiceApplyConfiguration) (*corev1.Service, error) {
applyCalled = true
return nil, nil
},
func(ctx context.Context, nn types.NamespacedName) error {
func(_ context.Context, _ types.NamespacedName) error {
deleteCalled = true
return nil
},
func(ctx context.Context) *applycorev1.ServiceApplyConfiguration {
func(_ context.Context) *applycorev1.ServiceApplyConfiguration {
return applycorev1.Service("test", "test").
WithLabels(map[string]string{
"example.com/component": "the-main-service-component",
Expand Down
2 changes: 1 addition & 1 deletion handler/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func (f Builder) Handler(id Key) Handler {
}

// NoopHandler is a handler that does nothing
var NoopHandler = NewHandler(ContextHandlerFunc(func(ctx context.Context) {}), NextKey)
var NoopHandler = NewHandler(ContextHandlerFunc(func(_ context.Context) {}), NextKey)

// Key is used to identify a given Handler in a set of Handlers
type Key string
Expand Down
7 changes: 4 additions & 3 deletions manager/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ func ExampleNewOwnedResourceController() {

// the controller processes objects on the queue, but doesn't set up any
// informers by default.
controller := NewOwnedResourceController(klogr.New(), "my-controller", gvr, CtxQueue, registry, broadcaster, func(ctx context.Context, gvr schema.GroupVersionResource, namespace, name string) {
// process object
controller := NewOwnedResourceController(klogr.New(), "my-controller", gvr, CtxQueue, registry, broadcaster, func(_ context.Context, gvr schema.GroupVersionResource, namespace, name string) {
fmt.Println("processing", gvr, namespace, name)
})

mgr := NewManager(ctrlmanageropts.RecommendedDebuggingOptions().DebuggingConfiguration, ":", broadcaster, eventSink)
Expand All @@ -54,7 +54,8 @@ func TestControllerQueueDone(t *testing.T) {
broadcaster := record.NewBroadcaster()
eventSink := &typedcorev1.EventSinkImpl{Interface: fake.NewSimpleClientset().CoreV1().Events("")}

controller := NewOwnedResourceController(klogr.New(), "my-controller", gvr, CtxQueue, registry, broadcaster, func(ctx context.Context, gvr schema.GroupVersionResource, namespace, name string) {
controller := NewOwnedResourceController(klogr.New(), "my-controller", gvr, CtxQueue, registry, broadcaster, func(_ context.Context, gvr schema.GroupVersionResource, namespace, name string) {
fmt.Println("processing", gvr, namespace, name)
})

mgr := NewManager(ctrlmanageropts.RecommendedDebuggingOptions().DebuggingConfiguration, ":", broadcaster, eventSink)
Expand Down
6 changes: 3 additions & 3 deletions pause/pause_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func ExampleNewPauseContextHandler() {
queueOperations.Key,
"example.com/paused",
ctxObject,
func(ctx context.Context, patch *MyObject) error {
func(_ context.Context, _ *MyObject) error {
// update status
return nil
},
Expand Down Expand Up @@ -184,7 +184,7 @@ func TestPauseHandler(t *testing.T) {
ctrls := &fake.FakeInterface{}
patchCalled := false

patchStatus := func(ctx context.Context, patch *MyObject) error {
patchStatus := func(_ context.Context, patch *MyObject) error {
patchCalled = true

if tt.patchError != nil {
Expand All @@ -209,7 +209,7 @@ func TestPauseHandler(t *testing.T) {
ctx = ctxMyObject.WithValue(ctx, tt.obj)
var called handler.Key

NewPauseContextHandler(queueOps.Key, PauseLabelKey, ctxMyObject, patchStatus, handler.ContextHandlerFunc(func(ctx context.Context) {
NewPauseContextHandler(queueOps.Key, PauseLabelKey, ctxMyObject, patchStatus, handler.ContextHandlerFunc(func(_ context.Context) {
called = nextKey
})).Handle(ctx)

Expand Down
4 changes: 2 additions & 2 deletions queue/controls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func ExampleNewOperations() {
}, cancel)

// typically called from a handler
handler.NewHandlerFromFunc(func(ctx context.Context) {
handler.NewHandlerFromFunc(func(_ context.Context) {
// do some work
operations.Done()
}, "example").Handle(ctx)
Expand Down Expand Up @@ -60,7 +60,7 @@ func ExampleNewQueueOperationsCtx() {
}, cancel))

// queue controls are passed via context
handler.NewHandlerFromFunc(func(ctx context.Context) {
handler.NewHandlerFromFunc(func(_ context.Context) {
// do some work
CtxQueue.Done()
}, "example").Handle(ctx)
Expand Down
6 changes: 3 additions & 3 deletions static/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ func NewStaticController[K bootstrap.KubeResourceObject](log logr.Logger, name s
func (c *Controller[K]) Start(ctx context.Context, _ int) {
inf := c.fileInformerFactory.ForResource(c.staticClusterResource).Informer()
_, err := inf.AddEventHandler(cache.ResourceEventHandlerFuncs{
AddFunc: func(obj interface{}) { c.handleStaticResource(ctx) },
UpdateFunc: func(_, obj interface{}) { c.handleStaticResource(ctx) },
DeleteFunc: func(obj interface{}) { c.handleStaticResource(ctx) },
AddFunc: func(_ any) { c.handleStaticResource(ctx) },
UpdateFunc: func(_, _ any) { c.handleStaticResource(ctx) },
DeleteFunc: func(_ any) { c.handleStaticResource(ctx) },
})
if err != nil {
panic("failed to add handlers: " + err.Error())
Expand Down
112 changes: 105 additions & 7 deletions typed/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,43 @@ func (r *Registry) NewFilteredDynamicSharedInformerFactory(key FactoryKey, clien
}

// ListerFor returns a typed Lister from a Registry
// Deprecated: Use MustListerForKey instead
func ListerFor[K runtime.Object](r *Registry, key RegistryKey) *Lister[K] {
return NewLister[K](r.ListerFor(key))
return MustListerForKey[K](r, key)
}

// MustListerForKey returns a typed Lister from a Registry, or panics if the key is not found
func MustListerForKey[K runtime.Object](r *Registry, key RegistryKey) *Lister[K] {
return NewLister[K](r.MustListerForKey(key))
}

// ListerForKey returns a typed Lister from a Registry, or an error if the key is not found
func ListerForKey[K runtime.Object](r *Registry, key RegistryKey) (*Lister[K], error) {
lister, err := r.ListerForKey(key)
if err != nil {
return nil, err
}
return NewLister[K](lister), nil
}

// IndexerFor returns a typed Indexer from a Registry
// Deprecated: Use MustIndexerForKey instead
func IndexerFor[K runtime.Object](r *Registry, key RegistryKey) *Indexer[K] {
return NewIndexer[K](r.InformerFor(key).GetIndexer())
return MustIndexerForKey[K](r, key)
}

// MustIndexerForKey returns a typed Indexer from a Registry, or panics if the key is not found
func MustIndexerForKey[K runtime.Object](r *Registry, key RegistryKey) *Indexer[K] {
return NewIndexer[K](r.MustIndexerForKey(key))
}

// IndexerForKey returns a typed Indexer from a Registry, or an error if the key is not found
func IndexerForKey[K runtime.Object](r *Registry, key RegistryKey) (*Indexer[K], error) {
indexer, err := r.IndexerForKey(key)
if err != nil {
return nil, err
}
return NewIndexer[K](indexer), nil
}

// Add adds a factory to the registry under the given FactoryKey
Expand All @@ -124,27 +154,95 @@ func (r *Registry) Remove(key FactoryKey) {
}

// InformerFactoryFor returns GVR-specific InformerFactory from the Registry.
// Deprecated: use MustInformerFactoryForKey instead.
func (r *Registry) InformerFactoryFor(key RegistryKey) informers.GenericInformer {
return r.MustInformerFactoryForKey(key)
}

// MustInformerFactoryForKey returns GVR-specific InformerFactory from the Registry
// or panics if the key is not found.
func (r *Registry) MustInformerFactoryForKey(key RegistryKey) informers.GenericInformer {
informer, err := r.InformerFactoryForKey(key)
if err != nil {
panic(err)
}
return informer
}

// InformerFactoryForKey returns GVR-specific InformerFactory from the Registry
// or returns an error if the key is not found.
func (r *Registry) InformerFactoryForKey(key RegistryKey) (informers.GenericInformer, error) {
r.RLock()
defer r.RUnlock()
factory, ok := r.factories[key.FactoryKey]
if !ok {
panic(fmt.Errorf("InformerFactoryFor called with unknown key %s", key))
return nil, fmt.Errorf("InformerFactoryFor called with unknown key %s", key)
}
return factory.ForResource(key.GroupVersionResource)
return factory.ForResource(key.GroupVersionResource), nil
}

// ListerFor returns the GVR-specific Lister from the Registry
// Deprecated: use MustListerForKey instead.
func (r *Registry) ListerFor(key RegistryKey) cache.GenericLister {
return r.InformerFactoryFor(key).Lister()
return r.MustInformerFactoryForKey(key).Lister()
}

// MustListerForKey returns the GVR-specific Lister from the Registry, or panics
// if the key is not found.
func (r *Registry) MustListerForKey(key RegistryKey) cache.GenericLister {
return r.MustInformerFactoryForKey(key).Lister()
}

// ListerForKey returns the GVR-specific Lister from the Registry, or an error
// if the key is not found.
func (r *Registry) ListerForKey(key RegistryKey) (cache.GenericLister, error) {
factory, err := r.InformerFactoryForKey(key)
if err != nil {
return nil, err
}
return factory.Lister(), nil
}

// InformerFor returns the GVR-specific Informer from the Registry
// Deprecated: use MustInformerForKey instead.
func (r *Registry) InformerFor(key RegistryKey) cache.SharedIndexInformer {
return r.InformerFactoryFor(key).Informer()
return r.MustInformerFactoryForKey(key).Informer()
}

// MustInformerForKey returns the GVR-specific Informer from the Registry, or panics
// if the key is not found.
func (r *Registry) MustInformerForKey(key RegistryKey) cache.SharedIndexInformer {
return r.MustInformerFactoryForKey(key).Informer()
}

// InformerForKey returns the GVR-specific Informer from the Registry, or an error
// if the key is not found.
func (r *Registry) InformerForKey(key RegistryKey) (cache.SharedIndexInformer, error) {
factory, err := r.InformerFactoryForKey(key)
if err != nil {
return nil, err
}
return factory.Informer(), nil
}

// IndexerFor returns the GVR-specific Indexer from the Registry
// Deprecated: use MustIndexerForKey instead.
func (r *Registry) IndexerFor(key RegistryKey) cache.Indexer {
return r.InformerFactoryFor(key).Informer().GetIndexer()
return r.MustInformerForKey(key).GetIndexer()
}

// MustIndexerForKey returns the GVR-specific Indexer from the Registry, or panics
// if the key is not found.
func (r *Registry) MustIndexerForKey(key RegistryKey) cache.Indexer {
return r.MustInformerForKey(key).GetIndexer()
}

// IndexerForKey returns the GVR-specific Indexer from the Registry, or an error
// if the key is not found.
func (r *Registry) IndexerForKey(key RegistryKey) (cache.Indexer, error) {
informer, err := r.InformerForKey(key)
if err != nil {
return nil, err
}
return informer.GetIndexer(), nil
}
Loading

0 comments on commit 510bcee

Please sign in to comment.