[WIP] fix: http probe downgrades from http2 to http1 when it encounters an error

pkg/queue/health/probe.go

@@ -160,20 +160,24 @@ func http2UpgradeProbe(config HTTPProbeConfigOptions) (int, error) {
 
 // HTTPProbe checks that HTTP connection can be established to the address.
 func HTTPProbe(config HTTPProbeConfigOptions) error {
-	if config.MaxProtoMajor == 0 {
+	configMaxProto := config.MaxProtoMajor
+	if configMaxProto == 0 {
 		// If we don't know if the connection supports HTTP2, we will try it.
-		// Once we get a non-error response, we won't try again.
 		// If maxProto is 0, container is not ready, so we don't know whether http2 is supported.
 		// If maxProto is 1, we know we're ready, but we also can't upgrade, so just return.
 		// If maxProto is 2, we know we can upgrade to http2
+		// If we get an error response, we will assume that the connection is HTTP1, as we can not guarantee if the service can handle the OPTIONS request
 		maxProto, err := http2UpgradeProbe(config)
-		if err != nil {
-			return fmt.Errorf("failed to run HTTP2 upgrade probe with error: %w", err)
-		}
 		config.MaxProtoMajor = maxProto
 		if config.MaxProtoMajor == 1 {
 			return nil
 		}
+
+		if err != nil {
+			// we default to HTTP1 because the options request did not work
+			// however, we do not know if the container is ready yet, so we still need to probe
+			config.MaxProtoMajor = 1
+		}
 	}
 	httpClient := &http.Client{
 		Transport: autoDowngradingTransport(config),
@@ -196,6 +200,10 @@ func HTTPProbe(config HTTPProbeConfigOptions) error {
 
 	res, err := httpClient.Do(req)
 	if err != nil {
+		if configMaxProto == 0 {
+			// we guessed that http1 was supported, but it also failed. Let's try the options request again next probe
+			config.MaxProtoMajor = 0
+		}
 		return err
 	}
 
@@ -207,6 +215,10 @@ func HTTPProbe(config HTTPProbeConfigOptions) error {
 	}()
 
 	if !isHTTPProbeReady(res) {
+		if configMaxProto == 0 {
+			// we guessed that http1 was supported, but it also failed. Let's try the options request again next probe
+			config.MaxProtoMajor = 0
+		}
 		return fmt.Errorf("HTTP probe did not respond Ready, got status code: %d", res.StatusCode)
 	}
 

pkg/queue/health/probe_test.go

@@ -201,6 +201,48 @@ func TestHTTPProbeAutoHTTP2(t *testing.T) {
 	}
 }
 
+func TestHTTPProbeAutoHTTP2FailedUpgrade(t *testing.T) {
+	h2cHeaders := map[string]string{
+		"Connection": "Upgrade, HTTP2-Settings",
+		"Upgrade":    "h2c",
+	}
+	expectedPath := "/health"
+	var callCount atomic.Int32
+
+	server := newH2cTestServer(t, func(w http.ResponseWriter, r *http.Request) {
+		count := callCount.Inc()
+		if r.Method == http.MethodOptions {
+			// make sure that the correct headers are present on the options request
+			for key, value := range h2cHeaders {
+				if r.Header.Get(key) != value {
+					t.Errorf("Key %v = %v was supposed to be present in the request", key, value)
+				}
+			}
+			w.WriteHeader(http.StatusInternalServerError)
+		} else {
+			w.WriteHeader(http.StatusOK)
+		}
+		if count > 2 {
+			t.Error("Expected probe to make two requests only")
+		}
+	})
+
+	action := newHTTPGetAction(t, server.URL)
+	action.Path = expectedPath
+
+	config := HTTPProbeConfigOptions{
+		Timeout:       time.Second,
+		HTTPGetAction: action,
+		MaxProtoMajor: 0,
+	}
+	if err := HTTPProbe(config); err != nil {
+		t.Error("Expected probe to succeed but it failed with", err)
+	}
+	if count := callCount.Load(); count != 2 {
+		t.Errorf("Unexpected call count %d", count)
+	}
+}
+
 func TestHTTPSchemeProbeSuccess(t *testing.T) {
 	server := newTestServer(t, func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusOK)