From d73e40f207d92153849721c8492b5a329783353d Mon Sep 17 00:00:00 2001 From: Uwe Krueger Date: Wed, 4 Aug 2021 21:54:32 +0200 Subject: [PATCH] rename handle to registration Kubernetes-commit: 7054ac16d43a4a55d6e7b69943eb209f6495c4ce --- tools/cache/shared_informer.go | 39 +++++++++++++++-------------- tools/cache/shared_informer_test.go | 32 +++++++++++------------ 2 files changed, 36 insertions(+), 35 deletions(-) diff --git a/tools/cache/shared_informer.go b/tools/cache/shared_informer.go index 9e930b1c..813daafd 100644 --- a/tools/cache/shared_informer.go +++ b/tools/cache/shared_informer.go @@ -138,7 +138,7 @@ type SharedInformer interface { // between different handlers. // It returns a handle for the handler that can be used to remove // the handler again. - AddEventHandler(handler ResourceEventHandler) (*ResourceEventHandlerHandle, error) + AddEventHandler(handler ResourceEventHandler) (*ResourceEventHandlerRegistration, error) // AddEventHandlerWithResyncPeriod adds an event handler to the // shared informer with the requested resync period; zero means // this handler does not care about resyncs. The resync operation @@ -155,8 +155,8 @@ type SharedInformer interface { // be competing load and scheduling noise. // It returns a handle for the handler that can be used to remove // the handler again and an error if the handler cannot be added. - AddEventHandlerWithResyncPeriod(handler ResourceEventHandler, resyncPeriod time.Duration) (*ResourceEventHandlerHandle, error) - // RemoveEventHandlerByHandle removes a formerly added event handler given by + AddEventHandlerWithResyncPeriod(handler ResourceEventHandler, resyncPeriod time.Duration) (*ResourceEventHandlerRegistration, error) + // RemoveEventHandlerByRegistration removes a formerly added event handler given by // its registration handle. // If, for some reason, the same handler has been added multiple // times, only the registration for the given registration handle @@ -166,7 +166,7 @@ type SharedInformer interface { // Calling Remove on an already removed handle returns no error // because the handler is finally (still) removed after calling this // method. - RemoveEventHandlerByHandle(handle *ResourceEventHandlerHandle) error + RemoveEventHandlerByRegistration(handle *ResourceEventHandlerRegistration) error // GetStore returns the informer's local cache as a Store. GetStore() Store // GetController is deprecated, it does nothing useful @@ -218,21 +218,21 @@ type SharedInformer interface { IsStopped() bool } -// ResourceEventHandlerHandle is a handle returned by the +// ResourceEventHandlerRegistration is a handle returned by the // registration methods of SharedInformers for a registered // ResourceEventHandler. It can be used later on to remove // a registration again. // This indirection is required, because the ResourceEventHandler // interface can be implemented by non-go-comparable handlers, which // could not be removed from a list anymore. -type ResourceEventHandlerHandle struct { +type ResourceEventHandlerRegistration struct { listener *processorListener } -// IsActive reports whether this registration is still active +// isActive reports whether this registration is still active // meaning that the handler registered with this handle is // still registered. -func (h *ResourceEventHandlerHandle) IsActive() bool { +func (h *ResourceEventHandlerRegistration) isActive() bool { return h.listener != nil } @@ -531,7 +531,7 @@ func (s *sharedIndexInformer) GetController() Controller { return &dummyController{informer: s} } -func (s *sharedIndexInformer) AddEventHandler(handler ResourceEventHandler) (*ResourceEventHandlerHandle, error) { +func (s *sharedIndexInformer) AddEventHandler(handler ResourceEventHandler) (*ResourceEventHandlerRegistration, error) { return s.AddEventHandlerWithResyncPeriod(handler, s.defaultEventHandlerResyncPeriod) } @@ -552,7 +552,7 @@ func determineResyncPeriod(desired, check time.Duration) time.Duration { const minimumResyncPeriod = 1 * time.Second -func (s *sharedIndexInformer) AddEventHandlerWithResyncPeriod(handler ResourceEventHandler, resyncPeriod time.Duration) (*ResourceEventHandlerHandle, error) { +func (s *sharedIndexInformer) AddEventHandlerWithResyncPeriod(handler ResourceEventHandler, resyncPeriod time.Duration) (*ResourceEventHandlerRegistration, error) { s.startedLock.Lock() defer s.startedLock.Unlock() @@ -582,7 +582,7 @@ func (s *sharedIndexInformer) AddEventHandlerWithResyncPeriod(handler ResourceEv } listener := newProcessListener(handler, resyncPeriod, determineResyncPeriod(resyncPeriod, s.resyncCheckPeriod), s.clock.Now(), initialBufferSize) - handle := &ResourceEventHandlerHandle{listener} + handle := &ResourceEventHandlerRegistration{listener} if !s.started { s.processor.addListener(listener) @@ -658,18 +658,19 @@ func (s *sharedIndexInformer) IsStopped() bool { return s.stopped } -// RemoveEventHandlerByHandle tries to remove a formerly added event handler by its -// handle returned for its registration. -// If a handler has been added multiple times, only the registration for the -// given handle will be removed. -func (s *sharedIndexInformer) RemoveEventHandlerByHandle(handle *ResourceEventHandlerHandle) error { +// RemoveEventHandlerByRegistration tries to remove a formerly added event handler by its +// registration handle returned for its registration. +// If a handler has been added multiple times, only the actual registration for the +// handler will be removed. Other registrations of the same handler will still be +// active until they are explicitly removes, also. +func (s *sharedIndexInformer) RemoveEventHandlerByRegistration(handle *ResourceEventHandlerRegistration) error { + s.startedLock.Lock() + defer s.startedLock.Unlock() + if handle.listener == nil { return nil } - s.startedLock.Lock() - defer s.startedLock.Unlock() - if s.stopped { return fmt.Errorf("handler %v is not removed from shared informer because it has stopped already", handle.listener.handler) } diff --git a/tools/cache/shared_informer_test.go b/tools/cache/shared_informer_test.go index 2fdf56af..6feeb218 100644 --- a/tools/cache/shared_informer_test.go +++ b/tools/cache/shared_informer_test.go @@ -428,14 +428,14 @@ func TestSharedInformerRemoveHandler(t *testing.T) { t.Errorf("informer has %d registered handler, instead of 2", eventHandlerCount(informer)) } - if err := informer.RemoveEventHandlerByHandle(handle2); err != nil { + if err := informer.RemoveEventHandlerByRegistration(handle2); err != nil { t.Errorf("removing of first pointer handler failed: %s", err) } if eventHandlerCount(informer) != 1 { t.Errorf("after removing handler informer has %d registered handler(s), instead of 1", eventHandlerCount(informer)) } - if err := informer.RemoveEventHandlerByHandle(handle1); err != nil { + if err := informer.RemoveEventHandlerByRegistration(handle1); err != nil { t.Errorf("removing of second pointer handler failed: %s", err) } if eventHandlerCount(informer) != 0 { @@ -466,14 +466,14 @@ func TestSharedInformerRemoveNonComparableHandler(t *testing.T) { t.Errorf("informer has %d registered handler(s), instead of 2", eventHandlerCount(informer)) } - if err := informer.RemoveEventHandlerByHandle(handle2); err != nil { + if err := informer.RemoveEventHandlerByRegistration(handle2); err != nil { t.Errorf("removing of pointer handler failed: %s", err) } if eventHandlerCount(informer) != 1 { t.Errorf("after removal informer has %d registered handler(s), instead of 1", eventHandlerCount(informer)) } - if err := informer.RemoveEventHandlerByHandle(handle1); err != nil { + if err := informer.RemoveEventHandlerByRegistration(handle1); err != nil { t.Errorf("removing of non-pointer handler failed: %s", err) } if eventHandlerCount(informer) != 0 { @@ -494,7 +494,7 @@ func TestSharedInformerMultipleRegistration(t *testing.T) { return } - if !reg1.IsActive() { + if !reg1.isActive() { t.Errorf("handle1 is not active after successful registration") return } @@ -505,7 +505,7 @@ func TestSharedInformerMultipleRegistration(t *testing.T) { return } - if !reg2.IsActive() { + if !reg2.isActive() { t.Errorf("handle2 is not active after successful registration") return } @@ -514,15 +514,15 @@ func TestSharedInformerMultipleRegistration(t *testing.T) { t.Errorf("informer has %d registered handler(s), instead of 1", eventHandlerCount(informer)) } - if err := informer.RemoveEventHandlerByHandle(reg1); err != nil { + if err := informer.RemoveEventHandlerByRegistration(reg1); err != nil { t.Errorf("removing of duplicate handler registration failed: %s", err) } - if reg1.IsActive() { + if reg1.isActive() { t.Errorf("handle1 is still active after successful remove") return } - if !reg2.IsActive() { + if !reg2.isActive() { t.Errorf("handle2 is not active after removing handle1") return } @@ -535,11 +535,11 @@ func TestSharedInformerMultipleRegistration(t *testing.T) { } } - if err := informer.RemoveEventHandlerByHandle(reg2); err != nil { + if err := informer.RemoveEventHandlerByRegistration(reg2); err != nil { t.Errorf("removing of second handler registration failed: %s", err) } - if reg2.IsActive() { + if reg2.isActive() { t.Errorf("handle2 is still active after successful remove") return } @@ -561,18 +561,18 @@ func TestRemovingRemovedSharedInformer(t *testing.T) { t.Errorf("informer did not add handler for the first time: %s", err) return } - if err := informer.RemoveEventHandlerByHandle(reg); err != nil { + if err := informer.RemoveEventHandlerByRegistration(reg); err != nil { t.Errorf("removing of handler registration failed: %s", err) return } - if reg.IsActive() { + if reg.isActive() { t.Errorf("handle is still active after successful remove") return } - if err := informer.RemoveEventHandlerByHandle(reg); err != nil { + if err := informer.RemoveEventHandlerByRegistration(reg); err != nil { t.Errorf("removing of already removed registration yields unexpected error: %s", err) } - if reg.IsActive() { + if reg.isActive() { t.Errorf("handle is still active after second remove") return } @@ -679,7 +679,7 @@ func TestRemoveOnStoppedSharedInformer(t *testing.T) { t.Errorf("informer reports not to be stopped although stop channel closed") return } - err = informer.RemoveEventHandlerByHandle(handle) + err = informer.RemoveEventHandlerByRegistration(handle) if err == nil { t.Errorf("informer removes handler on stopped informer") return