Merge pull request #122578 from carlory/lock-default-ConsistentHTTPGetHandlers

lock GA feature-gate ConsistentHTTPGetHandlers to default

pkg/features/kube_features.go

@@ -954,7 +954,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 
 	ContainerCheckpoint: {Default: false, PreRelease: featuregate.Alpha},
 
-	ConsistentHTTPGetHandlers: {Default: true, PreRelease: featuregate.GA},
+	ConsistentHTTPGetHandlers: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.31
 
 	CronJobsScheduledAnnotation: {Default: true, PreRelease: featuregate.Beta},
 

pkg/kubelet/kubelet.go

@@ -501,13 +501,11 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
 	//
 	// This client must not be modified to include credentials, because it is
 	// critical that credentials not leak from the client to arbitrary hosts.
-	insecureContainerLifecycleHTTPClient := &http.Client{}
-	if utilfeature.DefaultFeatureGate.Enabled(features.ConsistentHTTPGetHandlers) {
-		insecureTLSTransport := &http.Transport{
+	insecureContainerLifecycleHTTPClient := &http.Client{
+		Transport: &http.Transport{
 			TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
-		}
-		insecureContainerLifecycleHTTPClient.Transport = insecureTLSTransport
-		insecureContainerLifecycleHTTPClient.CheckRedirect = httpprobe.RedirectChecker(false)
+		},
+		CheckRedirect: httpprobe.RedirectChecker(false),
 	}
 
 	tracer := kubeDeps.TracerProvider.Tracer(instrumentationScope)

pkg/kubelet/kuberuntime/kuberuntime_container_test.go

@@ -435,17 +435,6 @@ func testLifeCycleHook(t *testing.T, testPod *v1.Pod, testContainer *v1.Containe
 
 	// Configured and working HTTP hook
 	t.Run("PreStop-HTTPGet", func(t *testing.T) {
-		t.Run("inconsistent", func(t *testing.T) {
-			ctx := context.Background()
-			defer func() { fakeHTTP.req = nil }()
-			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ConsistentHTTPGetHandlers, false)()
-			httpLifeCycle.PreStop.HTTPGet.Port = intstr.IntOrString{}
-			testContainer.Lifecycle = httpLifeCycle
-			_ = m.killContainer(ctx, testPod, cID, "foo", "testKill", "", &gracePeriod, nil)
-			if fakeHTTP.req == nil || !strings.Contains(fakeHTTP.req.URL.String(), httpLifeCycle.PreStop.HTTPGet.Host) {
-				t.Errorf("HTTP Prestop hook was not invoked")
-			}
-		})
 		t.Run("consistent", func(t *testing.T) {
 			ctx := context.Background()
 			defer func() { fakeHTTP.req = nil }()

pkg/kubelet/lifecycle/handlers.go

@@ -21,7 +21,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"net"
 	"net/http"
 	"net/url"
 	"strconv"
@@ -156,56 +155,32 @@ func (hr *handlerRunner) runHTTPHandler(ctx context.Context, pod *v1.Pod, contai
 		podIP = host
 	}
 
-	if utilfeature.DefaultFeatureGate.Enabled(features.ConsistentHTTPGetHandlers) {
-		req, err := httpprobe.NewRequestForHTTPGetAction(handler.HTTPGet, container, podIP, "lifecycle")
-		if err != nil {
-			return err
-		}
-		resp, err := hr.httpDoer.Do(req)
-		discardHTTPRespBody(resp)
-
-		if isHTTPResponseError(err) {
-			klog.V(1).ErrorS(err, "HTTPS request to lifecycle hook got HTTP response, retrying with HTTP.", "pod", klog.KObj(pod), "host", req.URL.Host)
-
-			req := req.Clone(context.Background())
-			req.URL.Scheme = "http"
-			req.Header.Del("Authorization")
-			resp, httpErr := hr.httpDoer.Do(req)
-
-			// clear err since the fallback succeeded
-			if httpErr == nil {
-				metrics.LifecycleHandlerHTTPFallbacks.Inc()
-				if eventRecorder != nil {
-					// report the fallback with an event
-					eventRecorder.Event(pod, v1.EventTypeWarning, "LifecycleHTTPFallback", fmt.Sprintf("request to HTTPS lifecycle hook %s got HTTP response, retry with HTTP succeeded", req.URL.Host))
-				}
-				err = nil
-			}
-			discardHTTPRespBody(resp)
-		}
-		return err
-	}
-
-	// Deprecated code path.
-	var port int
-	if handler.HTTPGet.Port.Type == intstr.String && len(handler.HTTPGet.Port.StrVal) == 0 {
-		port = 80
-	} else {
-		var err error
-		port, err = resolvePort(handler.HTTPGet.Port, container)
-		if err != nil {
-			return err
-		}
-	}
-
-	url := fmt.Sprintf("http://%s/%s", net.JoinHostPort(host, strconv.Itoa(port)), handler.HTTPGet.Path)
-	req, err := http.NewRequest(http.MethodGet, url, nil)
+	req, err := httpprobe.NewRequestForHTTPGetAction(handler.HTTPGet, container, podIP, "lifecycle")
 	if err != nil {
 		return err
 	}
 	resp, err := hr.httpDoer.Do(req)
-
 	discardHTTPRespBody(resp)
+
+	if isHTTPResponseError(err) {
+		klog.V(1).ErrorS(err, "HTTPS request to lifecycle hook got HTTP response, retrying with HTTP.", "pod", klog.KObj(pod), "host", req.URL.Host)
+
+		req := req.Clone(context.Background())
+		req.URL.Scheme = "http"
+		req.Header.Del("Authorization")
+		resp, httpErr := hr.httpDoer.Do(req)
+
+		// clear err since the fallback succeeded
+		if httpErr == nil {
+			metrics.LifecycleHandlerHTTPFallbacks.Inc()
+			if eventRecorder != nil {
+				// report the fallback with an event
+				eventRecorder.Event(pod, v1.EventTypeWarning, "LifecycleHTTPFallback", fmt.Sprintf("request to HTTPS lifecycle hook %s got HTTP response, retry with HTTP succeeded", req.URL.Host))
+			}
+			err = nil
+		}
+		discardHTTPRespBody(resp)
+	}
 	return err
 }
 

pkg/kubelet/lifecycle/handlers_test.go

@@ -276,50 +276,23 @@ func TestRunHandlerHttps(t *testing.T) {
 			t.Errorf("unexpected url: %s", fakeHTTPDoer.url)
 		}
 	})
-
-	t.Run("inconsistent", func(t *testing.T) {
-		defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ConsistentHTTPGetHandlers, false)()
-		container.Lifecycle.PostStart.HTTPGet.Port = intstr.FromString("70")
-		pod.Spec.Containers = []v1.Container{container}
-		_, err := handlerRunner.Run(ctx, containerID, &pod, &container, container.Lifecycle.PostStart)
-
-		if err != nil {
-			t.Errorf("unexpected error: %v", err)
-		}
-		if fakeHTTPDoer.url != "http://foo:70/bar" {
-			t.Errorf("unexpected url: %q", fakeHTTPDoer.url)
-		}
-	})
 }
 
 func TestRunHandlerHTTPPort(t *testing.T) {
 	tests := []struct {
-		Name               string
-		FeatureGateEnabled bool
-		Port               intstr.IntOrString
-		ExpectError        bool
-		Expected           string
+		Name        string
+		Port        intstr.IntOrString
+		ExpectError bool
+		Expected    string
 	}{
 		{
-			Name:               "consistent/with port",
-			FeatureGateEnabled: true,
-			Port:               intstr.FromString("70"),
-			Expected:           "https://foo:70/bar",
+			Name:     "consistent/with port",
+			Port:     intstr.FromString("70"),
+			Expected: "https://foo:70/bar",
 		}, {
-			Name:               "consistent/without port",
-			FeatureGateEnabled: true,
-			Port:               intstr.FromString(""),
-			ExpectError:        true,
-		}, {
-			Name:               "inconsistent/with port",
-			FeatureGateEnabled: false,
-			Port:               intstr.FromString("70"),
-			Expected:           "http://foo:70/bar",
-		}, {
-			Name:               "inconsistent/without port",
-			Port:               intstr.FromString(""),
-			FeatureGateEnabled: false,
-			Expected:           "http://foo:80/bar",
+			Name:        "consistent/without port",
+			Port:        intstr.FromString(""),
+			ExpectError: true,
 		},
 	}
 
@@ -349,7 +322,6 @@ func TestRunHandlerHTTPPort(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.Name, func(t *testing.T) {
 			ctx := context.Background()
-			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ConsistentHTTPGetHandlers, tt.FeatureGateEnabled)()
 			fakeHTTPDoer := fakeHTTP{}
 			handlerRunner := NewHandlerRunner(&fakeHTTPDoer, &fakeContainerCommandRunner{}, fakePodStatusProvider, nil)
 
@@ -644,14 +616,8 @@ func TestRunHTTPHandler(t *testing.T) {
 			}
 
 			t.Run("consistent", func(t *testing.T) {
-				defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ConsistentHTTPGetHandlers, true)()
 				verify(t, tt.Expected.NewHeader, tt.Expected.NewURL)
 			})
-
-			t.Run("inconsistent", func(t *testing.T) {
-				defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ConsistentHTTPGetHandlers, false)()
-				verify(t, tt.Expected.OldHeader, tt.Expected.OldURL)
-			})
 		})
 	}
 }
@@ -760,7 +726,6 @@ func TestRunHandlerHttpFailure(t *testing.T) {
 
 func TestRunHandlerHttpsFailureFallback(t *testing.T) {
 	ctx := context.Background()
-	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ConsistentHTTPGetHandlers, true)()
 
 	// Since prometheus' gatherer is global, other tests may have updated metrics already, so
 	// we need to reset them prior running this test.

pkg/kubelet/metrics/metrics.go

@@ -903,9 +903,7 @@ func Register(collectors ...metrics.StableCollector) {
 			legacyregistry.MustRegister(GracefulShutdownEndTime)
 		}
 
-		if utilfeature.DefaultFeatureGate.Enabled(features.ConsistentHTTPGetHandlers) {
-			legacyregistry.MustRegister(LifecycleHandlerHTTPFallbacks)
-		}
+		legacyregistry.MustRegister(LifecycleHandlerHTTPFallbacks)
 	})
 }