Merge pull request #52287 from yujuhong/rm-nsenter

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>..

kubelet: remove the --docker-exec-handler flag

Stop supporting the "nsenter" exec handler. Only the Docker native exec
handler is supported.

The flag was deprecated in Kubernetes 1.6 and is safe to remove
in Kubernetes 1.9 according to the deprecation policy.

**What this PR does / why we need it**:

**Which issue this PR fixes** : fixes #40229

**Special notes for your reviewer**:
N/A

**Release note**:

```release-note
Remove the --docker-exec-handler flag. Only native exec handler is supported.
```

cmd/kubelet/app/options/container_runtime.go

@@ -57,10 +57,6 @@ type ContainerRuntimeOptions struct {
 	PodSandboxImage string
 	// DockerEndpoint is the path to the docker endpoint to communicate with.
 	DockerEndpoint string
-	// DockerExecHandlerName is the handler to use when executing a command
-	// in a container. Valid values are 'native' and 'nsenter'. Defaults to
-	// 'native'.
-	DockerExecHandlerName string
 	// If no pulling progress is made before the deadline imagePullProgressDeadline,
 	// the image pulling will be cancelled. Defaults to 1m0s.
 	// +optional
@@ -107,7 +103,6 @@ func NewContainerRuntimeOptions() *ContainerRuntimeOptions {
 	return &ContainerRuntimeOptions{
 		DockerEndpoint:            dockerEndpoint,
 		DockershimRootDirectory:   "/var/lib/dockershim",
-		DockerExecHandlerName:     "native",
 		DockerDisableSharedPID:    true,
 		PodSandboxImage:           defaultPodSandboxImage,
 		ImagePullProgressDeadline: metav1.Duration{Duration: 1 * time.Minute},
@@ -125,9 +120,6 @@ func (s *ContainerRuntimeOptions) AddFlags(fs *pflag.FlagSet) {
 	fs.BoolVar(&s.DockerDisableSharedPID, "docker-disable-shared-pid", s.DockerDisableSharedPID, "The Container Runtime Interface (CRI) defaults to using a shared PID namespace for containers in a pod when running with Docker 1.13.1 or higher. Setting this flag reverts to the previous behavior of isolated PID namespaces. This ability will be removed in a future Kubernetes release.")
 	fs.StringVar(&s.PodSandboxImage, "pod-infra-container-image", s.PodSandboxImage, "The image whose network/ipc namespaces containers in each pod will use.")
 	fs.StringVar(&s.DockerEndpoint, "docker-endpoint", s.DockerEndpoint, "Use this for the docker endpoint to communicate with")
-	// TODO(#40229): Remove the docker-exec-handler flag.
-	fs.StringVar(&s.DockerExecHandlerName, "docker-exec-handler", s.DockerExecHandlerName, "Handler to use when executing a command in a container. Valid values are 'native' and 'nsenter'. Defaults to 'native'.")
-	fs.MarkDeprecated("docker-exec-handler", "this flag will be removed and only the 'native' handler will be supported in the future.")
 	fs.DurationVar(&s.ImagePullProgressDeadline.Duration, "image-pull-progress-deadline", s.ImagePullProgressDeadline.Duration, "If no pulling progress is made before this deadline, the image pulling will be cancelled.")
 
 	// Network plugin settings. Shared by both docker and rkt.

cmd/kubelet/app/server.go

@@ -855,7 +855,7 @@ func RunDockershim(c *kubeletconfiginternal.KubeletConfiguration, r *options.Con
 	}
 
 	ds, err := dockershim.NewDockerService(dockerClient, r.PodSandboxImage, streamingConfig, &pluginSettings,
-		c.RuntimeCgroups, c.CgroupDriver, r.DockerExecHandlerName, r.DockershimRootDirectory, r.DockerDisableSharedPID)
+		c.RuntimeCgroups, c.CgroupDriver, r.DockershimRootDirectory, r.DockerDisableSharedPID)
 	if err != nil {
 		return err
 	}

pkg/kubelet/container/runtime.go

@@ -127,9 +127,8 @@ type Runtime interface {
 // DirectStreamingRuntime is the interface implemented by runtimes for which the streaming calls
 // (exec/attach/port-forward) should be served directly by the Kubelet.
 type DirectStreamingRuntime interface {
-	// Runs the command in the container of the specified pod using nsenter.
-	// Attaches the processes stdin, stdout, and stderr. Optionally uses a
-	// tty.
+	// Runs the command in the container of the specified pod. Attaches
+	// the processes stdin, stdout, and stderr. Optionally uses a tty.
 	ExecInContainer(containerID ContainerID, cmd []string, stdin io.Reader, stdout, stderr io.WriteCloser, tty bool, resize <-chan remotecommand.TerminalSize, timeout time.Duration) error
 	// Forward the specified port from the specified pod to the stream.
 	PortForward(pod *Pod, port int32, stream io.ReadWriteCloser) error

pkg/kubelet/dockershim/BUILD

@@ -57,7 +57,6 @@ go_library(
         "//pkg/security/apparmor:go_default_library",
         "//pkg/util/hash:go_default_library",
         "//pkg/util/parsers:go_default_library",
-        "//pkg/util/term:go_default_library",
         "//vendor/github.com/blang/semver:go_default_library",
         "//vendor/github.com/docker/docker/api/types:go_default_library",
         "//vendor/github.com/docker/docker/api/types/container:go_default_library",
@@ -70,7 +69,6 @@ go_library(
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/errors:go_default_library",
         "//vendor/k8s.io/client-go/tools/remotecommand:go_default_library",
-        "//vendor/k8s.io/utils/exec:go_default_library",
     ],
 )
 

pkg/kubelet/dockershim/docker_service.go

@@ -147,22 +147,12 @@ var internalLabelKeys []string = []string{containerTypeLabelKey, containerLogPat
 
 // NOTE: Anything passed to DockerService should be eventually handled in another way when we switch to running the shim as a different process.
 func NewDockerService(client libdocker.Interface, podSandboxImage string, streamingConfig *streaming.Config,
-	pluginSettings *NetworkPluginSettings, cgroupsName string, kubeCgroupDriver string, execHandlerName, dockershimRootDir string, disableSharedPID bool) (DockerService, error) {
+	pluginSettings *NetworkPluginSettings, cgroupsName string, kubeCgroupDriver string, dockershimRootDir string, disableSharedPID bool) (DockerService, error) {
 	c := libdocker.NewInstrumentedInterface(client)
 	checkpointHandler, err := NewPersistentCheckpointHandler(dockershimRootDir)
 	if err != nil {
 		return nil, err
 	}
-	var execHandler ExecHandler
-	switch execHandlerName {
-	case "native":
-		execHandler = &NativeExecHandler{}
-	case "nsenter":
-		execHandler = &NsenterExecHandler{}
-	default:
-		glog.Warningf("Unknown Docker exec handler %q; defaulting to native", execHandlerName)
-		execHandler = &NativeExecHandler{}
-	}
 
 	ds := &dockerService{
 		client:          c,
@@ -170,7 +160,7 @@ func NewDockerService(client libdocker.Interface, podSandboxImage string, stream
 		podSandboxImage: podSandboxImage,
 		streamingRuntime: &streamingRuntime{
 			client:      client,
-			execHandler: execHandler,
+			execHandler: &NativeExecHandler{},
 		},
 		containerManager:  cm.NewContainerManager(cgroupsName, client),
 		checkpointHandler: checkpointHandler,

pkg/kubelet/dockershim/exec.go

@@ -19,8 +19,6 @@ package dockershim
 import (
 	"fmt"
 	"io"
-	"os"
-	"os/exec"
 	"time"
 
 	dockertypes "github.com/docker/docker/api/types"
@@ -28,8 +26,6 @@ import (
 
 	"k8s.io/client-go/tools/remotecommand"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
-	"k8s.io/kubernetes/pkg/util/term"
-	utilexec "k8s.io/utils/exec"
 
 	"k8s.io/kubernetes/pkg/kubelet/dockershim/libdocker"
 )
@@ -39,9 +35,6 @@ type ExecHandler interface {
 	ExecInContainer(client libdocker.Interface, container *dockertypes.ContainerJSON, cmd []string, stdin io.Reader, stdout, stderr io.WriteCloser, tty bool, resize <-chan remotecommand.TerminalSize, timeout time.Duration) error
 }
 
-// NsenterExecHandler executes commands in Docker containers using nsenter.
-type NsenterExecHandler struct{}
-
 type dockerExitError struct {
 	Inspect *dockertypes.ContainerExecInspect
 }
@@ -62,75 +55,6 @@ func (d *dockerExitError) ExitStatus() int {
 	return d.Inspect.ExitCode
 }
 
-// TODO should we support nsenter in a container, running with elevated privs and --pid=host?
-func (*NsenterExecHandler) ExecInContainer(client libdocker.Interface, container *dockertypes.ContainerJSON, cmd []string, stdin io.Reader, stdout, stderr io.WriteCloser, tty bool, resize <-chan remotecommand.TerminalSize, timeout time.Duration) error {
-	nsenter, err := exec.LookPath("nsenter")
-	if err != nil {
-		return fmt.Errorf("exec unavailable - unable to locate nsenter")
-	}
-
-	containerPid := container.State.Pid
-
-	// TODO what if the container doesn't have `env`???
-	args := []string{"-t", fmt.Sprintf("%d", containerPid), "-m", "-i", "-u", "-n", "-p", "--", "env", "-i"}
-	args = append(args, fmt.Sprintf("HOSTNAME=%s", container.Config.Hostname))
-	args = append(args, container.Config.Env...)
-	args = append(args, cmd...)
-	command := exec.Command(nsenter, args...)
-	var cmdErr error
-	if tty {
-		p, err := kubecontainer.StartPty(command)
-		if err != nil {
-			return err
-		}
-		defer p.Close()
-
-		// make sure to close the stdout stream
-		defer stdout.Close()
-
-		kubecontainer.HandleResizing(resize, func(size remotecommand.TerminalSize) {
-			term.SetSize(p.Fd(), size)
-		})
-
-		if stdin != nil {
-			go io.Copy(p, stdin)
-		}
-
-		if stdout != nil {
-			go io.Copy(stdout, p)
-		}
-
-		cmdErr = command.Wait()
-	} else {
-		if stdin != nil {
-			// Use an os.Pipe here as it returns true *os.File objects.
-			// This way, if you run 'kubectl exec <pod> -i bash' (no tty) and type 'exit',
-			// the call below to command.Run() can unblock because its Stdin is the read half
-			// of the pipe.
-			r, w, err := os.Pipe()
-			if err != nil {
-				return err
-			}
-			go io.Copy(w, stdin)
-
-			command.Stdin = r
-		}
-		if stdout != nil {
-			command.Stdout = stdout
-		}
-		if stderr != nil {
-			command.Stderr = stderr
-		}
-
-		cmdErr = command.Run()
-	}
-
-	if exitErr, ok := cmdErr.(*exec.ExitError); ok {
-		return &utilexec.ExitErrorWrapper{ExitError: exitErr}
-	}
-	return cmdErr
-}
-
 // NativeExecHandler executes commands in Docker containers using Docker's exec API.
 type NativeExecHandler struct{}
 

pkg/kubelet/kubelet.go

@@ -587,8 +587,8 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
 			// Create and start the CRI shim running as a grpc server.
 			streamingConfig := getStreamingConfig(kubeCfg, kubeDeps)
 			ds, err := dockershim.NewDockerService(kubeDeps.DockerClient, crOptions.PodSandboxImage, streamingConfig,
-				&pluginSettings, kubeCfg.RuntimeCgroups, kubeCfg.CgroupDriver, crOptions.DockerExecHandlerName,
-				crOptions.DockershimRootDirectory, crOptions.DockerDisableSharedPID)
+				&pluginSettings, kubeCfg.RuntimeCgroups, kubeCfg.CgroupDriver, crOptions.DockershimRootDirectory,
+				crOptions.DockerDisableSharedPID)
 			if err != nil {
 				return nil, err
 			}