diff --git a/pkg/server/sandbox_portforward.go b/pkg/server/sandbox_portforward.go index 7106cb673..e544dadb3 100644 --- a/pkg/server/sandbox_portforward.go +++ b/pkg/server/sandbox_portforward.go @@ -23,7 +23,6 @@ import ( "os/exec" "strings" - "github.com/containernetworking/plugins/pkg/ns" "github.com/pkg/errors" "github.com/sirupsen/logrus" "golang.org/x/net/context" @@ -45,31 +44,28 @@ func (c *criService) PortForward(ctx context.Context, r *runtime.PortForwardRequ return c.streamServer.GetPortForward(r) } -// portForward requires `socat` on the node. It uses netns to enter the sandbox namespace, -// and run `socat` insidethe namespace to forward stream for a specific port. The `socat` -// command keeps running until it exits or client disconnect. +// portForward requires `socat` on the node. It runs `socat` to forward stream for a +// specific port. The `socat` command keeps running until it exits or client disconnect. 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) } - var netNSDo func(func(ns.NetNS) error) error - // netNSPath is the network namespace path for logging. - var netNSPath string + if s.Status.Get().State != sandboxstore.StateReady { + return errors.Wrap(err, "sandbox container is not running") + } + // We use pod ip instead of netns.Do + localhost. One reason is that + // for sandbox container e.g. gvisor and kata, the network stack is + // inside sandbox, netns.Do + localhost will not work. However, accessing + // pod ip from host should always work, because it is an assumption of + // Kubernetes network model. (Related issue containerd/cri-containerd#524) + var addr string securityContext := s.Config.GetLinux().GetSecurityContext() hostNet := securityContext.GetNamespaceOptions().GetNetwork() == runtime.NamespaceMode_NODE if !hostNet { - if s.NetNS == nil || s.NetNS.Closed() { - return errors.Errorf("network namespace for sandbox %q is closed", id) - } - netNSDo = s.NetNS.GetNs().Do - netNSPath = s.NetNS.GetPath() + addr = s.IP } else { - // Run the function directly for host network. - netNSDo = func(do func(_ ns.NetNS) error) error { - return do(nil) - } - netNSPath = "host" + addr = "localhost" } socat, err := exec.LookPath("socat") @@ -78,46 +74,39 @@ func (c *criService) portForward(id string, port int32, stream io.ReadWriteClose } // Check https://linux.die.net/man/1/socat for meaning of the options. - args := []string{socat, "-", fmt.Sprintf("TCP4:localhost:%d", port)} + args := []string{socat, "-", fmt.Sprintf("TCP4:%s:%d", addr, port)} - logrus.Infof("Executing port forwarding command %q in network namespace %q", strings.Join(args, " "), netNSPath) - err = netNSDo(func(_ ns.NetNS) error { - cmd := exec.Command(args[0], args[1:]...) - cmd.Stdout = stream + logrus.Infof("Executing port forwarding command %q", strings.Join(args, " ")) + cmd := exec.Command(args[0], args[1:]...) + cmd.Stdout = stream - stderr := new(bytes.Buffer) - cmd.Stderr = stderr + 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) - } - in.Close() - logrus.Debugf("Finish copying port forward input for %q port %d", id, port) - }() - - if err := cmd.Run(); err != nil { - return errors.Errorf("socat command returns error: %v, stderr: %q", err, stderr.String()) - } - return nil - }) + // 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.Wrapf(err, "failed to execute portforward in network namespace %q", netNSPath) + 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) + } + in.Close() + logrus.Debugf("Finish copying port forward input for %q port %d", id, port) + }() + + if err := cmd.Run(); err != nil { + return errors.Errorf("socat command returns error: %v, stderr: %q", err, stderr.String()) } logrus.Infof("Finish port forwarding for %q port %d", id, port) - return nil }