From 1df526b3f79a212f575889dc388158f48e9ac204 Mon Sep 17 00:00:00 2001 From: Sergey Kanzhelev Date: Fri, 18 Mar 2022 17:32:28 +0000 Subject: [PATCH] remove TODOs from http package and prober --- pkg/kubelet/prober/prober.go | 38 +++++++++------------------- pkg/kubelet/prober/prober_manager.go | 1 - pkg/kubelet/prober/worker.go | 4 +-- pkg/probe/http/http.go | 10 ++++---- 4 files changed, 18 insertions(+), 35 deletions(-) diff --git a/pkg/kubelet/prober/prober.go b/pkg/kubelet/prober/prober.go index 3fef2b022edd1..2f10f4f4a55c8 100644 --- a/pkg/kubelet/prober/prober.go +++ b/pkg/kubelet/prober/prober.go @@ -49,16 +49,11 @@ const maxProbeRetries = 3 // Prober helps to check the liveness/readiness/startup of a container. type prober struct { - exec execprobe.Prober - // probe types needs different httpprobe instances so they don't - // share a connection pool which can cause collisions to the - // same host:port and transient failures. See #49740. - readinessHTTP httpprobe.Prober - livenessHTTP httpprobe.Prober - startupHTTP httpprobe.Prober - tcp tcpprobe.Prober - grpc grpcprobe.Prober - runner kubecontainer.CommandRunner + exec execprobe.Prober + http httpprobe.Prober + tcp tcpprobe.Prober + grpc grpcprobe.Prober + runner kubecontainer.CommandRunner recorder record.EventRecorder } @@ -71,14 +66,12 @@ func newProber( const followNonLocalRedirects = false return &prober{ - exec: execprobe.New(), - readinessHTTP: httpprobe.New(followNonLocalRedirects), - livenessHTTP: httpprobe.New(followNonLocalRedirects), - startupHTTP: httpprobe.New(followNonLocalRedirects), - tcp: tcpprobe.New(), - grpc: grpcprobe.New(), - runner: runner, - recorder: recorder, + exec: execprobe.New(), + http: httpprobe.New(followNonLocalRedirects), + tcp: tcpprobe.New(), + grpc: grpcprobe.New(), + runner: runner, + recorder: recorder, } } @@ -179,14 +172,7 @@ func (pb *prober) runProbe(probeType probeType, p *v1.Probe, pod *v1.Pod, status url := formatURL(scheme, host, port, path) headers := buildHeader(p.HTTPGet.HTTPHeaders) klog.V(4).InfoS("HTTP-Probe Headers", "headers", headers) - switch probeType { - case liveness: - return pb.livenessHTTP.Probe(url, headers, timeout) - case startup: - return pb.startupHTTP.Probe(url, headers, timeout) - default: - return pb.readinessHTTP.Probe(url, headers, timeout) - } + return pb.http.Probe(url, headers, timeout) } if p.TCPSocket != nil { port, err := extractPort(p.TCPSocket.Port, container) diff --git a/pkg/kubelet/prober/prober_manager.go b/pkg/kubelet/prober/prober_manager.go index 570b323249952..37282748f6523 100644 --- a/pkg/kubelet/prober/prober_manager.go +++ b/pkg/kubelet/prober/prober_manager.go @@ -52,7 +52,6 @@ var ProberResults = metrics.NewCounterVec( // probe (AddPod). The worker periodically probes its assigned container and caches the results. The // manager use the cached probe results to set the appropriate Ready state in the PodStatus when // requested (UpdatePodStatus). Updating probe parameters is not currently supported. -// TODO: Move liveness probing out of the runtime, to here. type Manager interface { // AddPod creates new probe workers for every container probe. This should be called for every // pod created. diff --git a/pkg/kubelet/prober/worker.go b/pkg/kubelet/prober/worker.go index f627a79d036bd..9e472c32edd37 100644 --- a/pkg/kubelet/prober/worker.go +++ b/pkg/kubelet/prober/worker.go @@ -261,9 +261,7 @@ func (w *worker) doProbe() (keepGoing bool) { } } - // TODO: in order for exec probes to correctly handle downward API env, we must be able to reconstruct - // the full container environment here, OR we must make a call to the CRI in order to get those environment - // values from the running container. + // Note, exec probe does NOT have access to pod environment variables or downward API result, err := w.probeManager.prober.probe(w.probeType, w.pod, status, w.container, w.containerID) if err != nil { // Prober error, throw away the result. diff --git a/pkg/probe/http/http.go b/pkg/probe/http/http.go index b6cc57d512d3a..c44f235b16964 100644 --- a/pkg/probe/http/http.go +++ b/pkg/probe/http/http.go @@ -51,9 +51,10 @@ func NewWithTLSConfig(config *tls.Config, followNonLocalRedirects bool) Prober { // We do not want the probe use node's local proxy set. transport := utilnet.SetTransportDefaults( &http.Transport{ - TLSClientConfig: config, - DisableKeepAlives: true, - Proxy: http.ProxyURL(nil), + TLSClientConfig: config, + DisableKeepAlives: true, + Proxy: http.ProxyURL(nil), + DisableCompression: true, // removes Accept-Encoding header }) return httpProber{transport, followNonLocalRedirects} } @@ -68,9 +69,8 @@ type httpProber struct { followNonLocalRedirects bool } -// Probe returns a ProbeRunner capable of running an HTTP check. +// Probe returns a probing result. The only case the err will not be nil is when there is a problem reading the response body. func (pr httpProber) Probe(url *url.URL, headers http.Header, timeout time.Duration) (probe.Result, string, error) { - pr.transport.DisableCompression = true // removes Accept-Encoding header client := &http.Client{ Timeout: timeout, Transport: pr.transport,