From aeef99a76e343eed9e479ff765021175b0b03e15 Mon Sep 17 00:00:00 2001 From: abhi Date: Thu, 22 Mar 2018 10:54:47 -0700 Subject: [PATCH 1/4] Using netns to perform socat This commit removes the usage of nsenter and uses netns to perform socat operation. Signed-off-by: abhi --- pkg/server/sandbox_portforward.go | 89 ++++++++++++------------------- pkg/store/sandbox/netns.go | 7 +++ 2 files changed, 42 insertions(+), 54 deletions(-) diff --git a/pkg/server/sandbox_portforward.go b/pkg/server/sandbox_portforward.go index 64df5065c..da11b73be 100644 --- a/pkg/server/sandbox_portforward.go +++ b/pkg/server/sandbox_portforward.go @@ -23,12 +23,12 @@ import ( "os/exec" "strings" + "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" ) @@ -46,69 +46,50 @@ 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") + if s.NetNS == nil { + return errors.Errorf("failed to find network namespace fo sandbox %q in store", id) } - // 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") - } - - 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.Wrap(err, "failed to dial") } - in.Close() - logrus.Debugf("Finish copy port forward input for %q port %d", id, port) - }() + defer client.Close() + defer stream.Close() - if err := cmd.Run(); err != nil { - return errors.Errorf("nsenter command returns error: %v, stderr: %q", err, stderr.String()) + wg.Add(1) + go func() { + if _, err := io.Copy(client, stream); err != nil { + logrus.WithError(err).Errorf("Failed to copy port forward input from %q port %d", id, port) + } + logrus.Infof("Finish copy port forward input for %q port %d: %v", id, port) + wg.Done() + }() + wg.Add(1) + go func() { + 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: %v", 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 diff --git a/pkg/store/sandbox/netns.go b/pkg/store/sandbox/netns.go index 8ec4c1de5..5d56d9222 100644 --- a/pkg/store/sandbox/netns.go +++ b/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 +} From 02b952ec1754f64b9d21e3f1f606b2ca7aed1303 Mon Sep 17 00:00:00 2001 From: abhi Date: Fri, 23 Mar 2018 22:36:39 -0700 Subject: [PATCH 2/4] Getting rid of socat Signed-off-by: abhi --- pkg/server/sandbox_portforward.go | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/pkg/server/sandbox_portforward.go b/pkg/server/sandbox_portforward.go index da11b73be..b8435b121 100644 --- a/pkg/server/sandbox_portforward.go +++ b/pkg/server/sandbox_portforward.go @@ -17,11 +17,10 @@ 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" @@ -34,7 +33,6 @@ import ( // 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()) @@ -53,34 +51,32 @@ func (c *criService) portForward(id string, port int32, stream io.ReadWriteClose if err != nil { return errors.Wrapf(err, "failed to find sandbox %q in store", id) } - if s.NetNS == nil { - return errors.Errorf("failed to find network namespace fo sandbox %q in store", id) + if s.NetNS == nil || s.NetNS.Closed() { + return errors.Errorf("network namespace for sandbox %q is closed", id) } 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.Wrap(err, "failed to dial") + return errors.Wrapf(err, "failed to dial %q", port) } - defer client.Close() - defer stream.Close() - 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 from %q port %d", id, port) + 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: %v", 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: %v", id, port) - + logrus.Infof("Finish copy port forward output for %q port %d", id, port) wg.Done() }() wg.Wait() From 1317854ec1c2552f698fbfe02ed9bba21d24c529 Mon Sep 17 00:00:00 2001 From: Abhinandan Prativadi Date: Sat, 24 Mar 2018 08:29:45 -0700 Subject: [PATCH 3/4] Updating travis.yml and Readme to remove nsenter and socal Signed-off-by: abhi --- .travis.yml | 2 -- README.md | 9 +++------ 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/.travis.yml b/.travis.yml index 17b3621e1..c0e727cce 100644 --- a/.travis.yml +++ b/.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 diff --git a/README.md b/README.md index 476b04b5f..d23381c0b 100644 --- a/README.md +++ b/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 From c200cb464250c34d0a94a60a4a85b1c969e97089 Mon Sep 17 00:00:00 2001 From: abhi Date: Sat, 24 Mar 2018 08:40:14 -0700 Subject: [PATCH 4/4] Updating ansible installer Signed-off-by: abhi --- contrib/ansible/tasks/bootstrap_centos.yaml | 1 - contrib/ansible/tasks/bootstrap_ubuntu.yaml | 1 - 2 files changed, 2 deletions(-) diff --git a/contrib/ansible/tasks/bootstrap_centos.yaml b/contrib/ansible/tasks/bootstrap_centos.yaml index b5c23519f..2c24211a2 100644 --- a/contrib/ansible/tasks/bootstrap_centos.yaml +++ b/contrib/ansible/tasks/bootstrap_centos.yaml @@ -9,5 +9,4 @@ - btrfs-progs - libseccomp - util-linux - - socat - libselinux-python diff --git a/contrib/ansible/tasks/bootstrap_ubuntu.yaml b/contrib/ansible/tasks/bootstrap_ubuntu.yaml index 88337c42b..f8ae1f516 100644 --- a/contrib/ansible/tasks/bootstrap_ubuntu.yaml +++ b/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