Skip to content

Commit 6f565c1

Browse files
authored
Fix panic when SharedHandler.Register is never called (#164)
* add failing test * Fix panic when SharedHandler.Register is never called
1 parent e12da3d commit 6f565c1

File tree

2 files changed

+51
-0
lines changed

2 files changed

+51
-0
lines changed

pkg/controller/controller_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"testing"
66
"time"
77

8+
"github.com/stretchr/testify/assert"
89
"go.uber.org/mock/gomock"
910
corev1 "k8s.io/api/core/v1"
1011
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -18,6 +19,8 @@ import (
1819
//go:generate mockgen --build_flags=--mod=mod -package controller -destination ./mock_shared_index_informer_test.go k8s.io/client-go/tools/cache SharedIndexInformer
1920

2021
func TestController_multiple_finalizers_race(t *testing.T) {
22+
t.Parallel()
23+
2124
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
2225
defer cancel()
2326

@@ -94,3 +97,45 @@ func TestController_multiple_finalizers_race(t *testing.T) {
9497
t.Errorf("unexpected number of handler executions, got %d, want %d", got, want)
9598
}
9699
}
100+
101+
func TestController_no_registered_handlers(t *testing.T) {
102+
t.Parallel()
103+
104+
queue := workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter())
105+
defer queue.ShutDown()
106+
store := cache.NewStore(cache.MetaNamespaceKeyFunc)
107+
108+
ctrl := gomock.NewController(t)
109+
informer := NewMockSharedIndexInformer(ctrl)
110+
informer.EXPECT().GetStore().Return(store).AnyTimes()
111+
handler := &SharedHandler{controllerGVR: corev1.SchemeGroupVersion.WithResource("ConfigMap").String()}
112+
c := &controller{
113+
informer: informer,
114+
workqueue: queue,
115+
handler: handler,
116+
}
117+
118+
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
119+
defer cancel()
120+
assert.NotPanics(t, func() {
121+
go c.runWorker()
122+
123+
cm := &corev1.ConfigMap{
124+
ObjectMeta: metav1.ObjectMeta{
125+
Namespace: "test-ns",
126+
Name: "test-cm",
127+
Finalizers: []string{"test-finalizer-1", "test-finalizer-2"},
128+
DeletionTimestamp: ptr.To(metav1.Now()),
129+
UID: uuid.NewUUID(),
130+
},
131+
}
132+
key := "test-ns/test-cm"
133+
134+
if err := store.Add(cm); err != nil {
135+
t.Fatal(err)
136+
}
137+
queue.Add(key)
138+
139+
<-ctx.Done()
140+
})
141+
}

pkg/controller/sharedhandler.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,12 @@ func (h *SharedHandler) observeDeletedObjectAfterFinalize(obj runtime.Object) {
143143

144144
// deletedInPreviousExecution returns whether an object has been deleted in earlier executions of the controller.
145145
func (h *SharedHandler) deletedInPreviousExecution(obj runtime.Object) bool {
146+
// Corner-case: Register was never called, so recentDeletions was not initialized
147+
// Currently, since there is no constructor for SharedHandler, the only place where we can initialize this cache is the Register hook
148+
if h.recentDeletions == nil {
149+
return false
150+
}
151+
146152
meta, err := meta.Accessor(obj)
147153
if err != nil {
148154
return false

0 commit comments

Comments
 (0)