From 87e7d77e156835c0f88d87b194ac32827e9bd76b Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Fri, 15 May 2020 22:31:45 -0400 Subject: [PATCH 1/3] kube-aggregator: Fix goroutine leak --- .../pkg/controllers/status/available_controller.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller.go b/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller.go index 12b056d70d8..61205ad7d73 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller.go @@ -321,6 +321,17 @@ func (c *AvailableConditionController) sync(key string) error { // we had trouble with slow dial and DNS responses causing us to wait too long. // we added this as insurance case <-time.After(6 * time.Second): + defer func() { + // errCh needs to have a reader since the above goroutine doing the work + // needs to send to it. This is defered to ensure it runs + // even if the post timeout work itself panics. + go func() { + res := <-errch + if res != nil { + fmt.Error("%v", res) + } + }() + }() results <- fmt.Errorf("timed out waiting for %v", discoveryURL) return } From 0d0af48444a21579658c9325b4972f4abf7ac138 Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Sat, 16 May 2020 20:22:12 -0400 Subject: [PATCH 2/3] Fix changes --- .../controllers/status/available_controller.go | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller.go b/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller.go index 61205ad7d73..918a01c18e7 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller.go @@ -321,17 +321,13 @@ func (c *AvailableConditionController) sync(key string) error { // we had trouble with slow dial and DNS responses causing us to wait too long. // we added this as insurance case <-time.After(6 * time.Second): - defer func() { - // errCh needs to have a reader since the above goroutine doing the work - // needs to send to it. This is defered to ensure it runs - // even if the post timeout work itself panics. - go func() { - res := <-errch - if res != nil { - fmt.Error("%v", res) - } - }() - }() + // errCh needs to have a reader since the above goroutine doing the work + // needs to send to it. + go func() { + if res := <-errch; res != nil { + fmt.Errorf("timed out response from %v: %v", discoveryURL, res) + } + }() results <- fmt.Errorf("timed out waiting for %v", discoveryURL) return } From c5f73de4fd6a566621e586986352383d3a84bc9a Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Mon, 18 May 2020 09:29:19 -0400 Subject: [PATCH 3/3] Converted errch to a buffered channel to avoid goroutine leak Signed-off-by: Gaurav Singh --- .../pkg/controllers/status/available_controller.go | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller.go b/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller.go index 918a01c18e7..182891a37f9 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/controllers/status/available_controller.go @@ -287,7 +287,7 @@ func (c *AvailableConditionController) sync(key string) error { } discoveryURL.Path = "/apis/" + apiService.Spec.Group + "/" + apiService.Spec.Version - errCh := make(chan error) + errCh := make(chan error, 1) go func() { // be sure to check a URL that the aggregated API server is required to serve newReq, err := http.NewRequest("GET", discoveryURL.String(), nil) @@ -321,13 +321,6 @@ func (c *AvailableConditionController) sync(key string) error { // we had trouble with slow dial and DNS responses causing us to wait too long. // we added this as insurance case <-time.After(6 * time.Second): - // errCh needs to have a reader since the above goroutine doing the work - // needs to send to it. - go func() { - if res := <-errch; res != nil { - fmt.Errorf("timed out response from %v: %v", discoveryURL, res) - } - }() results <- fmt.Errorf("timed out waiting for %v", discoveryURL) return }