From 6cedba4b54feff744ec5b0b94c89844adac9ee08 Mon Sep 17 00:00:00 2001 From: Dave Protasowski Date: Tue, 22 Nov 2022 12:44:19 -0500 Subject: [PATCH 1/2] fix nil pointer in the drainer when it's reset --- network/handlers/drain.go | 2 +- network/handlers/drain_test.go | 41 ++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/network/handlers/drain.go b/network/handlers/drain.go index 5eee2ea42e..3b2f28a077 100644 --- a/network/handlers/drain.go +++ b/network/handlers/drain.go @@ -196,7 +196,7 @@ func (d *Drainer) resetTimer() { d.Lock() defer d.Unlock() - if d.timer.Stop() { + if d.timer != nil && d.timer.Stop() { d.timer.Reset(d.QuietPeriod) } } diff --git a/network/handlers/drain_test.go b/network/handlers/drain_test.go index 082b8100d0..8e35eb944a 100644 --- a/network/handlers/drain_test.go +++ b/network/handlers/drain_test.go @@ -546,3 +546,44 @@ func TestReset(t *testing.T) { // Calling reset after a drain should succeed d.Reset() } + +// https://github.com/knative/pkg/issues/2642 +func TestResetWithActiveRequests(t *testing.T) { + d := Drainer{ + QuietPeriod: 5 * time.Second, + Inner: http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + return + }), + } + + trafficStopped := make(chan struct{}) + trafficStarted := make(chan struct{}) + drainStarted := make(chan struct{}) + defer close(trafficStopped) + + go func() { + r, _ := http.NewRequest("GET", "knative.dev", nil) + close(trafficStarted) + for { + select { + case <-trafficStopped: + return + default: + w := httptest.NewRecorder() + d.ServeHTTP(w, r) + } + } + }() + + go func() { + <-trafficStarted + close(drainStarted) + d.Drain() + }() + + <-drainStarted + d.Reset() + + // We need requests to be active for a bit + time.Sleep(time.Second) +} From 1f75f966604576575f43a4ebc6c5abf6045ae468 Mon Sep 17 00:00:00 2001 From: dprotaso Date: Tue, 22 Nov 2022 13:20:20 -0500 Subject: [PATCH 2/2] fix linter --- network/handlers/drain_test.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/network/handlers/drain_test.go b/network/handlers/drain_test.go index 8e35eb944a..88e4f07e57 100644 --- a/network/handlers/drain_test.go +++ b/network/handlers/drain_test.go @@ -551,9 +551,7 @@ func TestReset(t *testing.T) { func TestResetWithActiveRequests(t *testing.T) { d := Drainer{ QuietPeriod: 5 * time.Second, - Inner: http.HandlerFunc(func(http.ResponseWriter, *http.Request) { - return - }), + Inner: http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), } trafficStopped := make(chan struct{}) @@ -562,15 +560,16 @@ func TestResetWithActiveRequests(t *testing.T) { defer close(trafficStopped) go func() { - r, _ := http.NewRequest("GET", "knative.dev", nil) + req, _ := http.NewRequest("GET", "knative.dev", nil) + rec := httptest.NewRecorder() + close(trafficStarted) for { select { case <-trafficStopped: return default: - w := httptest.NewRecorder() - d.ServeHTTP(w, r) + d.ServeHTTP(rec, req) } } }()