Merge pull request #3046 from linxiulei/fix_shim_socket

Shorten the unix socket path for shim

container_test.go

@@ -32,7 +32,9 @@ import (
 	// Register the typeurl
 	"github.com/containerd/containerd/cio"
 	"github.com/containerd/containerd/containers"
+	"github.com/containerd/containerd/namespaces"
 	"github.com/containerd/containerd/oci"
+	"github.com/containerd/containerd/platforms"
 	_ "github.com/containerd/containerd/runtime"
 	"github.com/containerd/typeurl"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
@@ -1528,3 +1530,59 @@ func TestContainerHook(t *testing.T) {
 	}
 	defer task.Delete(ctx, WithProcessKill)
 }
+
+func TestShimSockLength(t *testing.T) {
+	t.Parallel()
+
+	// Max length of namespace should be 76
+	namespace := strings.Repeat("n", 76)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	ctx = namespaces.WithNamespace(ctx, namespace)
+
+	client, err := newClient(t, address)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer client.Close()
+
+	image, err := client.Pull(ctx, testImage,
+		WithPlatformMatcher(platforms.Default()),
+		WithPullUnpack,
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	id := strings.Repeat("c", 64)
+
+	// We don't have limitation with length of container name,
+	// but 64 bytes of sha256 is the common case
+	container, err := client.NewContainer(ctx, id,
+		WithNewSnapshot(id, image),
+		WithNewSpec(oci.WithImageConfig(image), withExitStatus(0)),
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer container.Delete(ctx, WithSnapshotCleanup)
+
+	task, err := container.NewTask(ctx, empty())
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer task.Delete(ctx)
+
+	statusC, err := task.Wait(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if err := task.Start(ctx); err != nil {
+		t.Fatal(err)
+	}
+
+	<-statusC
+}

runtime/v1/linux/bundle.go

@@ -20,6 +20,7 @@ package linux
 
 import (
 	"context"
+	"crypto/sha256"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -103,7 +104,7 @@ func ShimLocal(c *Config, exchange *exchange.Exchange) ShimOpt {
 // ShimConnect is a ShimOpt for connecting to an existing remote shim
 func ShimConnect(c *Config, onClose func()) ShimOpt {
 	return func(b *bundle, ns string, ropts *runctypes.RuncOptions) (shim.Config, client.Opt) {
-		return b.shimConfig(ns, c, ropts), client.WithConnect(b.shimAddress(ns), onClose)
+		return b.shimConfig(ns, c, ropts), client.WithConnect(b.decideShimAddress(ns), onClose)
 	}
 }
 
@@ -127,10 +128,32 @@ func (b *bundle) Delete() error {
 	return errors.Wrapf(err, "Failed to remove both bundle and workdir locations: %v", err2)
 }
 
-func (b *bundle) shimAddress(namespace string) string {
+func (b *bundle) legacyShimAddress(namespace string) string {
 	return filepath.Join(string(filepath.Separator), "containerd-shim", namespace, b.id, "shim.sock")
 }
 
+func (b *bundle) shimAddress(namespace string) string {
+	d := sha256.Sum256([]byte(filepath.Join(namespace, b.id)))
+	return filepath.Join(string(filepath.Separator), "containerd-shim", fmt.Sprintf("%x.sock", d))
+}
+
+func (b *bundle) loadAddress() (string, error) {
+	addressPath := filepath.Join(b.path, "address")
+	data, err := ioutil.ReadFile(addressPath)
+	if err != nil {
+		return "", err
+	}
+	return string(data), nil
+}
+
+func (b *bundle) decideShimAddress(namespace string) string {
+	address, err := b.loadAddress()
+	if err != nil {
+		return b.legacyShimAddress(namespace)
+	}
+	return address
+}
+
 func (b *bundle) shimConfig(namespace string, c *Config, runcOptions *runctypes.RuncOptions) shim.Config {
 	var (
 		criuPath      string

runtime/v1/shim/client/client.go

@@ -20,10 +20,12 @@ package client
 
 import (
 	"context"
+	"fmt"
 	"io"
 	"net"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"strings"
 	"sync"
 	"syscall"
@@ -107,6 +109,10 @@ func WithStart(binary, address, daemonAddress, cgroup string, debug bool, exitHa
 			"address": address,
 			"debug":   debug,
 		}).Infof("shim %s started", binary)
+
+		if err := writeAddress(filepath.Join(config.Path, "address"), address); err != nil {
+			return nil, nil, err
+		}
 		// set shim in cgroup if it is provided
 		if cgroup != "" {
 			if err := setCgroup(cgroup, cmd); err != nil {
@@ -166,6 +172,25 @@ func newCommand(binary, daemonAddress string, debug bool, config shim.Config, so
 	return cmd, nil
 }
 
+// writeAddress writes a address file atomically
+func writeAddress(path, address string) error {
+	path, err := filepath.Abs(path)
+	if err != nil {
+		return err
+	}
+	tempPath := filepath.Join(filepath.Dir(path), fmt.Sprintf(".%s", filepath.Base(path)))
+	f, err := os.OpenFile(tempPath, os.O_RDWR|os.O_CREATE|os.O_EXCL|os.O_SYNC, 0666)
+	if err != nil {
+		return err
+	}
+	_, err = f.WriteString(address)
+	f.Close()
+	if err != nil {
+		return err
+	}
+	return os.Rename(tempPath, path)
+}
+
 func newSocket(address string) (*net.UnixListener, error) {
 	if len(address) > 106 {
 		return nil, errors.Errorf("%q: unix socket path too long (> 106)", address)

runtime/v2/shim/util_unix.go

@@ -20,6 +20,8 @@ package shim
 
 import (
 	"context"
+	"crypto/sha256"
+	"fmt"
 	"net"
 	"path/filepath"
 	"strings"
@@ -50,7 +52,8 @@ func SocketAddress(ctx context.Context, id string) (string, error) {
 	if err != nil {
 		return "", err
 	}
-	return filepath.Join(string(filepath.Separator), "containerd-shim", ns, id, "shim.sock"), nil
+	d := sha256.Sum256([]byte(filepath.Join(ns, id)))
+	return filepath.Join(string(filepath.Separator), "containerd-shim", fmt.Sprintf("%x.sock", d)), nil
 }
 
 // AnonDialer returns a dialer for an abstract socket