Skip to content

Commit fd7881d

Browse files
Merge pull request #135278 from aman4433/KUBE-134468
Fix goroutine leak in CRDFinalizer Kubernetes-commit: 9998041e0ffe0dd3f2abab3b9f95505c4402bf14
2 parents 8db5ab6 + 7d598d7 commit fd7881d

3 files changed

Lines changed: 22 additions & 18 deletions

File tree

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,6 @@ require (
122122
gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect
123123
gopkg.in/yaml.v3 v3.0.1 // indirect
124124
k8s.io/gengo/v2 v2.0.0-20250922181213-ec3ebc5fd46b // indirect
125-
k8s.io/kms v0.0.0-20251126210012-3215d77feb60 // indirect
125+
k8s.io/kms v0.0.0-20251203145945-db26c430546c // indirect
126126
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.31.2 // indirect
127127
)

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -312,8 +312,8 @@ k8s.io/gengo/v2 v2.0.0-20250922181213-ec3ebc5fd46b h1:gMplByicHV/TJBizHd9aVEsTYo
312312
k8s.io/gengo/v2 v2.0.0-20250922181213-ec3ebc5fd46b/go.mod h1:CgujABENc3KuTrcsdpGmrrASjtQsWCT7R99mEV4U/fM=
313313
k8s.io/klog/v2 v2.130.1 h1:n9Xl7H1Xvksem4KFG4PYbdQCQxqc/tTUyrgXaOhHSzk=
314314
k8s.io/klog/v2 v2.130.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE=
315-
k8s.io/kms v0.0.0-20251126210012-3215d77feb60 h1:wDMkRbiitSOzCEWWzd/1BneWnGn63Jda0eJImm1744s=
316-
k8s.io/kms v0.0.0-20251126210012-3215d77feb60/go.mod h1:jqaSb8D+h2Z90dz8NrTNdtgsD6Tqy0DR4o4lGNUt0lU=
315+
k8s.io/kms v0.0.0-20251203145945-db26c430546c h1:bPkHveflJECmqO5XaFLgi4MTnuh6HmTUx9CHSEPG058=
316+
k8s.io/kms v0.0.0-20251203145945-db26c430546c/go.mod h1:jqaSb8D+h2Z90dz8NrTNdtgsD6Tqy0DR4o4lGNUt0lU=
317317
k8s.io/kube-openapi v0.0.0-20250910181357-589584f1c912 h1:Y3gxNAuB0OBLImH611+UDZcmKS3g6CthxToOb37KgwE=
318318
k8s.io/kube-openapi v0.0.0-20250910181357-589584f1c912/go.mod h1:kdmbQkyfwUagLfXIad1y2TdrjPFWp2Q89B3qkRwf/pQ=
319319
k8s.io/utils v0.0.0-20251002143259-bc988d571ff4 h1:SjGebBtkBqHFOli+05xYbK8YF1Dzkbzn+gDM4X9T4Ck=

pkg/controller/finalizer/crd_finalizer.go

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ type CRDFinalizer struct {
6464
crdSynced cache.InformerSynced
6565

6666
// To allow injection for testing.
67-
syncFn func(key string) error
67+
syncFn func(ctx context.Context, key string) error
6868

6969
queue workqueue.TypedRateLimitingInterface[string]
7070
}
@@ -109,7 +109,7 @@ func NewCRDFinalizer(
109109
return c
110110
}
111111

112-
func (c *CRDFinalizer) sync(key string) error {
112+
func (c *CRDFinalizer) sync(ctx context.Context, key string) error {
113113
cachedCRD, err := c.crdLister.Get(key)
114114
if apierrors.IsNotFound(err) {
115115
return nil
@@ -132,7 +132,7 @@ func (c *CRDFinalizer) sync(key string) error {
132132
Reason: "InstanceDeletionInProgress",
133133
Message: "CustomResource deletion is in progress",
134134
})
135-
crd, err = c.crdClient.CustomResourceDefinitions().UpdateStatus(context.TODO(), crd, metav1.UpdateOptions{})
135+
crd, err = c.crdClient.CustomResourceDefinitions().UpdateStatus(ctx, crd, metav1.UpdateOptions{})
136136
if apierrors.IsNotFound(err) || apierrors.IsConflict(err) {
137137
// deleted or changed in the meantime, we'll get called again
138138
return nil
@@ -152,10 +152,10 @@ func (c *CRDFinalizer) sync(key string) error {
152152
Message: "instances overlap with built-in resources in storage",
153153
})
154154
} else if apiextensionshelpers.IsCRDConditionTrue(crd, apiextensionsv1.Established) {
155-
cond, deleteErr := c.deleteInstances(crd)
155+
cond, deleteErr := c.deleteInstances(ctx, crd)
156156
apiextensionshelpers.SetCRDCondition(crd, cond)
157157
if deleteErr != nil {
158-
if _, err = c.crdClient.CustomResourceDefinitions().UpdateStatus(context.TODO(), crd, metav1.UpdateOptions{}); err != nil {
158+
if _, err = c.crdClient.CustomResourceDefinitions().UpdateStatus(ctx, crd, metav1.UpdateOptions{}); err != nil {
159159
utilruntime.HandleError(err)
160160
}
161161
return deleteErr
@@ -170,15 +170,15 @@ func (c *CRDFinalizer) sync(key string) error {
170170
}
171171

172172
apiextensionshelpers.CRDRemoveFinalizer(crd, apiextensionsv1.CustomResourceCleanupFinalizer)
173-
_, err = c.crdClient.CustomResourceDefinitions().UpdateStatus(context.TODO(), crd, metav1.UpdateOptions{})
173+
_, err = c.crdClient.CustomResourceDefinitions().UpdateStatus(ctx, crd, metav1.UpdateOptions{})
174174
if apierrors.IsNotFound(err) || apierrors.IsConflict(err) {
175175
// deleted or changed in the meantime, we'll get called again
176176
return nil
177177
}
178178
return err
179179
}
180180

181-
func (c *CRDFinalizer) deleteInstances(crd *apiextensionsv1.CustomResourceDefinition) (apiextensionsv1.CustomResourceDefinitionCondition, error) {
181+
func (c *CRDFinalizer) deleteInstances(ctx context.Context, crd *apiextensionsv1.CustomResourceDefinition) (apiextensionsv1.CustomResourceDefinitionCondition, error) {
182182
// Now we can start deleting items. While it would be ideal to use a REST API client, doing so
183183
// could incorrectly delete a ThirdPartyResource with the same URL as the CustomResource, so we go
184184
// directly to the storage instead. Since we control the storage, we know that delete collection works.
@@ -193,7 +193,6 @@ func (c *CRDFinalizer) deleteInstances(crd *apiextensionsv1.CustomResourceDefini
193193
}, err
194194
}
195195

196-
ctx := genericapirequest.NewContext()
197196
allResources, err := crClient.List(ctx, nil)
198197
if err != nil {
199198
return apiextensionsv1.CustomResourceDefinitionCondition{
@@ -263,37 +262,42 @@ func (c *CRDFinalizer) deleteInstances(crd *apiextensionsv1.CustomResourceDefini
263262
}
264263

265264
func (c *CRDFinalizer) Run(workers int, stopCh <-chan struct{}) {
265+
c.RunWithContext(workers, wait.ContextForChannel(stopCh))
266+
}
267+
268+
//logcheck:context // RunWithContext should be used instead of Run in code which supports contextual logging.
269+
func (c *CRDFinalizer) RunWithContext(workers int, ctx context.Context) {
266270
defer utilruntime.HandleCrash()
267271
defer c.queue.ShutDown()
268272

269273
klog.Info("Starting CRDFinalizer")
270274
defer klog.Info("Shutting down CRDFinalizer")
271275

272-
if !cache.WaitForCacheSync(stopCh, c.crdSynced) {
276+
if !cache.WaitForCacheSync(ctx.Done(), c.crdSynced) {
273277
return
274278
}
275279

276280
for i := 0; i < workers; i++ {
277-
go wait.Until(c.runWorker, time.Second, stopCh)
281+
go wait.UntilWithContext(ctx, c.runWorker, time.Second)
278282
}
279283

280-
<-stopCh
284+
<-ctx.Done()
281285
}
282286

283-
func (c *CRDFinalizer) runWorker() {
284-
for c.processNextWorkItem() {
287+
func (c *CRDFinalizer) runWorker(ctx context.Context) {
288+
for c.processNextWorkItem(ctx) {
285289
}
286290
}
287291

288292
// processNextWorkItem deals with one key off the queue. It returns false when it's time to quit.
289-
func (c *CRDFinalizer) processNextWorkItem() bool {
293+
func (c *CRDFinalizer) processNextWorkItem(ctx context.Context) bool {
290294
key, quit := c.queue.Get()
291295
if quit {
292296
return false
293297
}
294298
defer c.queue.Done(key)
295299

296-
err := c.syncFn(key)
300+
err := c.syncFn(ctx, key)
297301
if err == nil {
298302
c.queue.Forget(key)
299303
return true

0 commit comments

Comments
 (0)