Update shutdown/ready logic and misc cleanup

Signed-off-by: David Alger <davidmalger@gmail.com>

internal/cmd/envoy/shutdown_manager.go

@@ -34,8 +34,8 @@ const (
 	ShutdownManagerHealthCheckPath = "/healthz"
 	// ShutdownManagerReadyPath is the path used to indicate shutdown readiness.
 	ShutdownManagerReadyPath = "/shutdown/ready"
-	// ShutdownReadyFile is the file used to indicate shutdown readiness.
-	ShutdownReadyFile = "/tmp/shutdown-ready"
+	// ShutdownActiveFile is the file used to indicate shutdown readiness.
+	ShutdownActiveFile = "/tmp/shutdown-active"
 )
 
 // ShutdownManager serves shutdown manager process for Envoy proxies.
@@ -44,7 +44,7 @@ func ShutdownManager(readyTimeout time.Duration) error {
 	handler := http.NewServeMux()
 	handler.HandleFunc(ShutdownManagerHealthCheckPath, func(_ http.ResponseWriter, _ *http.Request) {})
 	handler.HandleFunc(ShutdownManagerReadyPath, func(w http.ResponseWriter, _ *http.Request) {
-		shutdownReadyHandler(w, readyTimeout, ShutdownReadyFile)
+		shutdownReadyHandler(w, readyTimeout, ShutdownActiveFile)
 	})
 
 	// Setup HTTP server
@@ -85,31 +85,44 @@ func ShutdownManager(readyTimeout time.Duration) error {
 }
 
 // shutdownReadyHandler handles the endpoint used by a preStop hook on the Envoy
-// container to block until ready to terminate. After the graceful drain process
-// has completed a file will be written to indicate shutdown readiness.
-func shutdownReadyHandler(w http.ResponseWriter, readyTimeout time.Duration, readyFile string) {
+// container to block until ready to terminate. When the graceful drain process
+// has begun a file will be written to indicate shutdown is in progress. This
+// handler will poll for the file and block until it is removed.
+func shutdownReadyHandler(w http.ResponseWriter, readyTimeout time.Duration, activeFile string) {
 	var startTime = time.Now()
 
 	logger.Info("received shutdown ready request")
 
+	// Since we cannot establish order between preStop hooks, wait a bit giving the exec
+	// preStop hook a chance to touch the signaling file before starting to poll for it.
+	time.Sleep(2 * time.Second)
+
+	// Exit early if active file failed to show up
+	if _, err := os.Stat(activeFile); os.IsNotExist(err) {
+		logger.Info("shutdown active file not found")
+		w.WriteHeader(http.StatusInternalServerError)
+		return
+	}
+
 	// Poll for shutdown readiness
 	for {
+		// Check if ready timeout is exceeded
 		elapsedTime := time.Since(startTime)
 		if elapsedTime > readyTimeout {
 			logger.Info("shutdown readiness timeout exceeded")
 			w.WriteHeader(http.StatusInternalServerError)
 			return
 		}
 
-		_, err := os.Stat(readyFile)
+		_, err := os.Stat(activeFile)
 		switch {
 		case os.IsNotExist(err):
-			time.Sleep(1 * time.Second)
+			logger.Info("shutdown readiness detected")
+			return
 		case err != nil:
 			logger.Error(err, "error checking for shutdown readiness")
 		case err == nil:
-			logger.Info("shutdown readiness detected")
-			return
+			time.Sleep(1 * time.Second)
 		}
 	}
 }
@@ -126,6 +139,12 @@ func Shutdown(drainTimeout time.Duration, minDrainDuration time.Duration, exitAt
 		logger = logging.FileLogger("/proc/1/fd/1", "shutdown-manager", v1alpha1.LogLevelInfo)
 	}
 
+	// Signal to shutdownReadyHandler that drain process is started
+	if _, err := os.Create(ShutdownActiveFile); err != nil {
+		logger.Error(err, "error creating shutdown active file")
+		return err
+	}
+
 	logger.Info(fmt.Sprintf("initiating graceful drain with %.0f second minimum drain period and %.0f second timeout",
 		minDrainDuration.Seconds(), drainTimeout.Seconds()))
 
@@ -167,7 +186,8 @@ func Shutdown(drainTimeout time.Duration, minDrainDuration time.Duration, exitAt
 	}
 
 	// Signal to shutdownReadyHandler that drain process is complete
-	if err := createShutdownReadyFile(); err != nil {
+	if err := os.Remove(ShutdownActiveFile); err != nil {
+		logger.Error(err, "error removing shutdown active file")
 		return err
 	}
 
@@ -212,12 +232,3 @@ func getTotalConnections() (*int, error) {
 		}
 	}
 }
-
-// createShutdownReadyFile creates a file to indicate that the shutdown process is complete
-func createShutdownReadyFile() error {
-	if _, err := os.Create(ShutdownReadyFile); err != nil {
-		logger.Error(err, "error creating shutdown ready file")
-		return err
-	}
-	return nil
-}

internal/cmd/envoy_test.go

@@ -0,0 +1,17 @@
+// Copyright Envoy Gateway Authors
+// SPDX-License-Identifier: Apache-2.0
+// The full text of the Apache license is available in the LICENSE file at
+// the root of the repo.
+
+package cmd
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetEnvoyCommand(t *testing.T) {
+	got := getEnvoyCommand()
+	assert.Equal(t, "envoy", got.Use)
+}

test/config/gatewayclass.yaml

@@ -60,6 +60,5 @@ spec:
             name: ENVOY_GATEWAY_NAMESPACE
             defaultValue: "envoy-gateway-system"
   shutdown:
-    drainTimeout: 5s
-    minDrainDuration: 2s
-    exitAtConnections: 5
+    drainTimeout: 15s
+    minDrainDuration: 5s

tools/make/kube.mk

@@ -118,7 +118,6 @@ run-e2e: prepare-e2e
 	kubectl wait --timeout=5m -n envoy-gateway-system deployment/envoy-ratelimit --for=condition=Available
 	kubectl wait --timeout=5m -n envoy-gateway-system deployment/envoy-gateway --for=condition=Available
 	kubectl apply -f test/config/gatewayclass.yaml
-	kubectl describe node
 	go test -v -tags e2e ./test/e2e --gateway-class=envoy-gateway --debug=true
 
 .PHONY: prepare-e2e
@@ -172,7 +171,6 @@ run-conformance: ## Run Gateway API conformance.
 	@$(LOG_TARGET)
 	kubectl wait --timeout=$(WAIT_TIMEOUT) -n envoy-gateway-system deployment/envoy-gateway --for=condition=Available
 	kubectl apply -f test/config/gatewayclass.yaml
-	kubectl describe node
 	go test -v -tags conformance ./test/conformance --gateway-class=envoy-gateway --debug=true
 
 CONFORMANCE_REPORT_PATH ?=