Merge pull request #691 from abhi/socat

Getting rid of nsenter and socat

Author: Random-Liu

.travis.yml

@@ -21,8 +21,6 @@ install:
     - sudo apt-get install btrfs-tools
     - sudo apt-get install libseccomp2/trusty-backports
     - sudo apt-get install libseccomp-dev/trusty-backports
-    - sudo apt-get install socat
-    - docker run --rm -v /usr/local/bin:/target jpetazzo/nsenter
 
 before_script:
     - export PATH=$HOME/gopath/bin:$PATH

README.md

@@ -67,12 +67,9 @@ specifications as appropriate.
 (Fedora, CentOS, RHEL). On releases of Ubuntu <=Trusty and Debian <=jessie a
 backport version of `libseccomp-dev` is required. See [travis.yml](.travis.yml) for an example on trusty.
 * **btrfs development library.** Required by containerd btrfs support. `btrfs-tools`(Ubuntu, Debian) / `btrfs-progs-devel`(Fedora, CentOS, RHEL)
-2. Install other dependencies:
-* **`nsenter`**: Required by portforward.
-* **`socat`**: Required by portforward.
-3. Install and setup a go 1.10 development environment.
-4. Make a local clone of this repository.
-5. Install binary dependencies by running the following command from your cloned `cri/` project directory:
+2. Install and setup a go 1.10 development environment.
+3. Make a local clone of this repository.
+4. Install binary dependencies by running the following command from your cloned `cri/` project directory:
 ```bash
 # Note: install.deps installs the above mentioned runc, containerd, and CNI
 # binary dependencies. install.deps is only provided for general use and ease of

contrib/ansible/tasks/bootstrap_centos.yaml

@@ -9,5 +9,4 @@
     - btrfs-progs
     - libseccomp
     - util-linux 
-    - socat
     - libselinux-python

contrib/ansible/tasks/bootstrap_ubuntu.yaml

@@ -9,6 +9,5 @@
       - apt-transport-https
       - btrfs-tools
       - libseccomp2
-      - socat
       - util-linux
 # TODO: Limited support for trusty for nsenter. Need to handle/verify 

pkg/server/sandbox_portforward.go

@@ -17,24 +17,22 @@ limitations under the License.
 package server
 
 import (
-	"bytes"
 	"fmt"
 	"io"
-	"os/exec"
-	"strings"
+	"net"
+	"sync"
 
+	"github.com/containernetworking/plugins/pkg/ns"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/net/context"
 	runtime "k8s.io/kubernetes/pkg/kubelet/apis/cri/runtime/v1alpha2"
 
-	ctrdutil "github.com/containerd/cri/pkg/containerd/util"
 	sandboxstore "github.com/containerd/cri/pkg/store/sandbox"
 )
 
 // PortForward prepares a streaming endpoint to forward ports from a PodSandbox, and returns the address.
 func (c *criService) PortForward(ctx context.Context, r *runtime.PortForwardRequest) (retRes *runtime.PortForwardResponse, retErr error) {
-	// TODO(random-liu): Run a socat container inside the sandbox to do portforward.
 	sandbox, err := c.sandboxStore.Get(r.GetPodSandboxId())
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to find sandbox %q", r.GetPodSandboxId())
@@ -46,69 +44,48 @@ func (c *criService) PortForward(ctx context.Context, r *runtime.PortForwardRequ
 	return c.streamServer.GetPortForward(r)
 }
 
-// portForward requires `nsenter` and `socat` on the node, it uses `nsenter` to enter the
-// sandbox namespace, and run `socat` inside the namespace to forward stream for a specific
-// port. The `socat` command keeps running until it exits or client disconnect.
+// portForward requires it uses netns to enter the sandbox namespace,
+// and forward stream for a specific port.
 func (c *criService) portForward(id string, port int32, stream io.ReadWriteCloser) error {
 	s, err := c.sandboxStore.Get(id)
 	if err != nil {
 		return errors.Wrapf(err, "failed to find sandbox %q in store", id)
 	}
-	t, err := s.Container.Task(ctrdutil.NamespacedContext(), nil)
-	if err != nil {
-		return errors.Wrap(err, "failed to get sandbox container task")
-	}
-	pid := t.Pid()
-
-	socat, err := exec.LookPath("socat")
-	if err != nil {
-		return errors.Wrap(err, "failed to find socat")
-	}
-
-	// Check following links for meaning of the options:
-	// * socat: https://linux.die.net/man/1/socat
-	// * nsenter: http://man7.org/linux/man-pages/man1/nsenter.1.html
-	args := []string{"-t", fmt.Sprintf("%d", pid), "-n", socat,
-		"-", fmt.Sprintf("TCP4:localhost:%d", port)}
-
-	nsenter, err := exec.LookPath("nsenter")
-	if err != nil {
-		return errors.Wrap(err, "failed to find nsenter")
+	if s.NetNS == nil || s.NetNS.Closed() {
+		return errors.Errorf("network namespace for sandbox %q is closed", id)
 	}
 
-	logrus.Infof("Executing port forwarding command: %s %s", nsenter, strings.Join(args, " "))
-
-	cmd := exec.Command(nsenter, args...)
-	cmd.Stdout = stream
-
-	stderr := new(bytes.Buffer)
-	cmd.Stderr = stderr
-
-	// If we use Stdin, command.Run() won't return until the goroutine that's copying
-	// from stream finishes. Unfortunately, if you have a client like telnet connected
-	// via port forwarding, as long as the user's telnet client is connected to the user's
-	// local listener that port forwarding sets up, the telnet session never exits. This
-	// means that even if socat has finished running, command.Run() won't ever return
-	// (because the client still has the connection and stream open).
-	//
-	// The work around is to use StdinPipe(), as Wait() (called by Run()) closes the pipe
-	// when the command (socat) exits.
-	in, err := cmd.StdinPipe()
-	if err != nil {
-		return errors.Wrap(err, "failed to create stdin pipe")
-	}
-	go func() {
-		if _, err := io.Copy(in, stream); err != nil {
-			logrus.WithError(err).Errorf("Failed to copy port forward input for %q port %d", id, port)
+	err = s.NetNS.GetNs().Do(func(_ ns.NetNS) error {
+		var wg sync.WaitGroup
+		client, err := net.Dial("tcp4", fmt.Sprintf("localhost:%d", port))
+		if err != nil {
+			return errors.Wrapf(err, "failed to dial %q", port)
 		}
-		in.Close()
-		logrus.Debugf("Finish copy port forward input for %q port %d", id, port)
-	}()
-
-	if err := cmd.Run(); err != nil {
-		return errors.Errorf("nsenter command returns error: %v, stderr: %q", err, stderr.String())
+		wg.Add(1)
+		go func() {
+			defer client.Close()
+			if _, err := io.Copy(client, stream); err != nil {
+				logrus.WithError(err).Errorf("Failed to copy port forward input for %q port %d", id, port)
+			}
+			logrus.Infof("Finish copy port forward input for %q port %d", id, port)
+			wg.Done()
+		}()
+		wg.Add(1)
+		go func() {
+			defer stream.Close()
+			if _, err := io.Copy(stream, client); err != nil {
+				logrus.WithError(err).Errorf("Failed to copy port forward output for %q port %d", id, port)
+			}
+			logrus.Infof("Finish copy port forward output for %q port %d", id, port)
+			wg.Done()
+		}()
+		wg.Wait()
+
+		return nil
+	})
+	if err != nil {
+		return errors.Wrapf(err, "failed to execute portforward in network namespace %s", s.NetNS.GetPath())
 	}
-
 	logrus.Infof("Finish port forwarding for %q port %d", id, port)
 
 	return nil

pkg/store/sandbox/netns.go

@@ -117,3 +117,10 @@ func (n *NetNS) GetPath() string {
 	defer n.Unlock()
 	return n.ns.Path()
 }
+
+// GetNs returns the network namespace handle
+func (n *NetNS) GetNs() cnins.NetNS {
+	n.Lock()
+	defer n.Unlock()
+	return n.ns
+}