Separate shutting down v. not ready in queue proxy.

Fixes: knative#8151

* Makes health_state return a different error code (currently 409) when it's shutting down.
* Makes shutting down fail fast (like preferPodForScaleDown).
* Unit tests to validate behavior.

cmd/queue/main.go

@@ -281,13 +281,17 @@ func probeQueueHealthPath(timeoutSeconds int, env probeConfig) error {
 		defer res.Body.Close()
 		success := health.IsHTTPProbeReady(res)
 
-		// The check for preferForScaledown() fails readiness faster
-		// in the presence of the label
+		// Both preferPodForScaledown and IsHTTPProbeShuttingDon can fail readiness faster.
+		// The check for preferPodForScaledown() fails readiness faster in the presence of the label,
+		// while shutting down has a different response code than not ready.
 		if preferScaleDown, err := preferPodForScaledown(env.DownwardAPILabelsPath); err != nil {
 			fmt.Fprintln(os.Stderr, err)
 		} else if !success && preferScaleDown {
 			return false, errors.New("failing probe deliberately for pod scaledown")
 		}
+		if health.IsHTTPProbeShuttingDown(res) {
+			return false, errors.New("failing probe deliberately for shutdown")
+		}
 		return success, nil
 	}, ctx.Done())
 

cmd/queue/main_test.go

@@ -212,6 +212,34 @@ func TestProbeQueueNotReady(t *testing.T) {
 	}
 }
 
+func TestProbeQueueShuttingDownFailsFast(t *testing.T) {
+	ts := newProbeTestServer(func(w http.ResponseWriter) {
+		w.WriteHeader(http.StatusConflict)
+	})
+
+	defer ts.Close()
+
+	u, err := url.Parse(ts.URL)
+	if err != nil {
+		t.Fatalf("%s is not a valid URL: %v", ts.URL, err)
+	}
+
+	port, err := strconv.Atoi(u.Port())
+	if err != nil {
+		t.Fatalf("Failed to convert port(%s) to int: %v", u.Port(), err)
+	}
+
+	start := time.Now()
+	if err = probeQueueHealthPath(1, probeConfig{QueueServingPort: port}); err == nil {
+		t.Error("probeQueueHealthPath did not fail")
+	}
+
+	// if fails due to timeout and not cancelation, then it took too long
+	if time.Since(start) >= 1*time.Second {
+		t.Error("took too long to fail")
+	}
+}
+
 func TestProbeQueueReady(t *testing.T) {
 	queueProbed := ptr.Int32(0)
 	ts := newProbeTestServer(func(w http.ResponseWriter) {

pkg/queue/health/health_state.go

@@ -24,8 +24,11 @@ import (
 
 const (
 	// Return `queue` as body for 200 responses to indicate the response is from queue-proxy.
-	aliveBody    = "queue"
+	aliveBody = "queue"
+	// Return `queue not ready` as body for 503 response for queue not yet ready.
 	notAliveBody = "queue not ready"
+	// Return `shutting down` as body for 503 response if the queue got a shutdown request.
+	shuttingDownBody = "shutting down"
 )
 
 // State holds state about the current healthiness of the component.
@@ -101,11 +104,16 @@ func (h *State) HandleHealthProbe(prober func() bool, isAggressive bool, w http.
 		io.WriteString(w, notAliveBody)
 	}
 
+	sendShuttingDown := func() {
+		w.WriteHeader(http.StatusConflict)
+		io.WriteString(w, shuttingDownBody)
+	}
+
 	switch {
 	case !isAggressive && h.IsAlive():
 		sendAlive()
 	case h.IsShuttingDown():
-		sendNotAlive()
+		sendShuttingDown()
 	case prober != nil && !prober():
 		sendNotAlive()
 	default:

pkg/queue/health/health_state_test.go

@@ -97,8 +97,8 @@ func TestHealthStateHealthHandler(t *testing.T) {
 		name:         "shuttingDown: true, K-Probe",
 		state:        &State{shuttingDown: true},
 		isAggressive: false,
-		wantStatus:   http.StatusServiceUnavailable,
-		wantBody:     notAliveBody,
+		wantStatus:   http.StatusConflict,
+		wantBody:     shuttingDownBody,
 	}, {
 		name:         "no prober, shuttingDown: false",
 		state:        &State{},
@@ -110,8 +110,8 @@ func TestHealthStateHealthHandler(t *testing.T) {
 		state:        &State{shuttingDown: true},
 		prober:       func() bool { return true },
 		isAggressive: true,
-		wantStatus:   http.StatusServiceUnavailable,
-		wantBody:     notAliveBody,
+		wantStatus:   http.StatusConflict,
+		wantBody:     shuttingDownBody,
 	}, {
 		name:         "prober: true, shuttingDown: false",
 		state:        &State{},

pkg/queue/health/probe.go

@@ -103,3 +103,13 @@ func IsHTTPProbeReady(res *http.Response) bool {
 	// response status code between 200-399 indicates success
 	return res.StatusCode >= 200 && res.StatusCode < 400
 }
+
+// IsHTTPProbeShuttingDown checks whether the Response indicates the prober is shutting down.
+func IsHTTPProbeShuttingDown(res *http.Response) bool {
+	if res == nil {
+		return false
+	}
+
+	// status 409 indicates the probe returned a shutdown scenario.
+	return res.StatusCode == http.StatusConflict
+}

pkg/queue/health/probe_test.go

@@ -166,6 +166,41 @@ func TestHTTPProbeResponseErrorFailure(t *testing.T) {
 	}
 }
 
+func TestIsHTTPProbeShuttingDown(t *testing.T) {
+	tests := []struct {
+		name       string
+		statusCode int
+		wantResult bool
+	}{{
+		name:       "statusCode: 409",
+		statusCode: 409,
+		wantResult: true,
+	}, {
+		name:       "statusCode: 503",
+		statusCode: 503,
+		wantResult: false,
+	}, {
+		name:       "statusCode: 200",
+		statusCode: 200,
+		wantResult: false,
+	}, {
+		name:       "statusCode: 301",
+		statusCode: 301,
+		wantResult: false,
+	}}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			response := http.Response{StatusCode: test.statusCode}
+			result := IsHTTPProbeShuttingDown(&response)
+			if result != test.wantResult {
+				t.Errorf("IsHTTPProbeShuttingDown returned unexpected result: got %v want %v",
+					result, test.wantResult)
+			}
+		})
+	}
+}
+
 func newHTTPGetAction(t *testing.T, serverURL string) *corev1.HTTPGetAction {
 	urlParsed, err := url.Parse(serverURL)
 	if err != nil {