From 9677616eaf3c03d57838b8d2a9a5d9dc263af678 Mon Sep 17 00:00:00 2001 From: Lantao Liu Date: Fri, 1 Jun 2018 17:24:19 -0700 Subject: [PATCH] Address comments in #64006. Signed-off-by: Lantao Liu --- pkg/kubelet/config/flags.go | 2 +- pkg/kubelet/dockershim/docker_service.go | 2 +- pkg/kubelet/server/server.go | 16 ++++++++++------ 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/pkg/kubelet/config/flags.go b/pkg/kubelet/config/flags.go index 377d64e4574..7710580794f 100644 --- a/pkg/kubelet/config/flags.go +++ b/pkg/kubelet/config/flags.go @@ -86,7 +86,7 @@ func (s *ContainerRuntimeOptions) AddFlags(fs *pflag.FlagSet) { // General settings. fs.StringVar(&s.ContainerRuntime, "container-runtime", s.ContainerRuntime, "The container runtime to use. Possible values: 'docker', 'remote', 'rkt (deprecated)'.") fs.StringVar(&s.RuntimeCgroups, "runtime-cgroups", s.RuntimeCgroups, "Optional absolute name of cgroups to create and run the runtime in.") - fs.BoolVar(&s.RedirectContainerStreaming, "redirect-container-streaming", s.RedirectContainerStreaming, "Enables container streaming redirect. If false, kubelet will proxy container streaming data between apiserver and container runtime; if true, kubelet will return an http redirect to apiserver, and apiserver will access container runtime directly. The proxy approach is more secure, but introduces some overhead. The redirect approach is more performant, but less secure because the connection between apiserver and container runtime is not authenticated.") + fs.BoolVar(&s.RedirectContainerStreaming, "redirect-container-streaming", s.RedirectContainerStreaming, "Enables container streaming redirect. If false, kubelet will proxy container streaming data between apiserver and container runtime; if true, kubelet will return an http redirect to apiserver, and apiserver will access container runtime directly. The proxy approach is more secure, but introduces some overhead. The redirect approach is more performant, but less secure because the connection between apiserver and container runtime may not be authenticated.") // Docker-specific settings. fs.BoolVar(&s.ExperimentalDockershim, "experimental-dockershim", s.ExperimentalDockershim, "Enable dockershim only mode. In this mode, kubelet will only start dockershim without any other functionalities. This flag only serves test purpose, please do not use it unless you are conscious of what you are doing. [default=false]") diff --git a/pkg/kubelet/dockershim/docker_service.go b/pkg/kubelet/dockershim/docker_service.go index 2a7cce57f0d..1cba483d17f 100644 --- a/pkg/kubelet/dockershim/docker_service.go +++ b/pkg/kubelet/dockershim/docker_service.go @@ -411,7 +411,7 @@ func (ds *dockerService) Start(stopCh <-chan struct{}) error { }() go func() { if err := ds.streamingServer.Start(true); err != nil && err != http.ErrServerClosed { - glog.Fatalf("Failed to start streaming server: %v", err) + glog.Fatalf("Streaming server stopped unexpectedly: %v", err) } }() } diff --git a/pkg/kubelet/server/server.go b/pkg/kubelet/server/server.go index 2e5bbde211f..b96bf6b1448 100644 --- a/pkg/kubelet/server/server.go +++ b/pkg/kubelet/server/server.go @@ -631,6 +631,13 @@ func (r *responder) Error(w http.ResponseWriter, req *http.Request, err error) { http.Error(w, err.Error(), http.StatusInternalServerError) } +// proxyStream proxies stream to url. +func proxyStream(w http.ResponseWriter, r *http.Request, url *url.URL) { + // TODO(random-liu): Set MaxBytesPerSec to throttle the stream. + handler := proxy.NewUpgradeAwareHandler(url, nil /*transport*/, false /*wrapTransport*/, true /*upgradeRequired*/, &responder{}) + handler.ServeHTTP(w, r) +} + // getAttach handles requests to attach to a container. func (s *Server) getAttach(request *restful.Request, response *restful.Response) { params := getExecRequestParams(request) @@ -657,8 +664,7 @@ func (s *Server) getAttach(request *restful.Request, response *restful.Response) http.Redirect(response.ResponseWriter, request.Request, url.String(), http.StatusFound) return } - handler := proxy.NewUpgradeAwareHandler(url, nil /*transport*/, false /*wrapTransport*/, false /*upgradeRequired*/, &responder{}) - handler.ServeHTTP(response.ResponseWriter, request.Request) + proxyStream(response.ResponseWriter, request.Request, url) } // getExec handles requests to run a command inside a container. @@ -686,8 +692,7 @@ func (s *Server) getExec(request *restful.Request, response *restful.Response) { http.Redirect(response.ResponseWriter, request.Request, url.String(), http.StatusFound) return } - handler := proxy.NewUpgradeAwareHandler(url, nil /*transport*/, false /*wrapTransport*/, false /*upgradeRequired*/, &responder{}) - handler.ServeHTTP(response.ResponseWriter, request.Request) + proxyStream(response.ResponseWriter, request.Request, url) } // getRun handles requests to run a command inside a container. @@ -753,8 +758,7 @@ func (s *Server) getPortForward(request *restful.Request, response *restful.Resp http.Redirect(response.ResponseWriter, request.Request, url.String(), http.StatusFound) return } - handler := proxy.NewUpgradeAwareHandler(url, nil /*transport*/, false /*wrapTransport*/, false /*upgradeRequired*/, &responder{}) - handler.ServeHTTP(response.ResponseWriter, request.Request) + proxyStream(response.ResponseWriter, request.Request, url) } // ServeHTTP responds to HTTP requests on the Kubelet.