service/snapshotter: move default to client

In order to enforce strict handling of snapshotter values on the
container object, the defaults have been moved to the client side. This
ensures that we correctly qualify the snapshotter under use when from
the container at the time it was created, rather than possibly losing
the metadata on a change of default.

Signed-off-by: Stephen J Day <stephen.day@docker.com>

client.go

@@ -192,6 +192,7 @@ func defaultRemoteContext() *RemoteContext {
 		Resolver: docker.NewResolver(docker.ResolverOptions{
 			Client: http.DefaultClient,
 		}),
+		Snapshotter: DefaultSnapshotter,
 	}
 }
 

container.go

@@ -174,6 +174,10 @@ func (c *container) NewTask(ctx context.Context, ioCreate IOCreation, opts ...Ne
 		Stderr:      cfg.Stderr,
 	}
 	if c.c.RootFS != "" {
+		if c.c.Snapshotter == "" {
+			return nil, errors.Wrapf(errdefs.ErrInvalidArgument, "unable to resolve rootfs mounts without snapshotter on container")
+		}
+
 		// get the rootfs from the snapshotter and add it to the request
 		mounts, err := c.client.SnapshotService(c.c.Snapshotter).Mounts(ctx, c.c.RootFS)
 		if err != nil {

container_opts.go

@@ -4,7 +4,9 @@ import (
 	"context"
 
 	"github.com/containerd/containerd/containers"
+	"github.com/containerd/containerd/errdefs"
 	"github.com/opencontainers/image-spec/identity"
+	"github.com/pkg/errors"
 )
 
 // NewContainerOpts allows the caller to set additional options when creating a container
@@ -38,6 +40,8 @@ func WithContainerLabels(labels map[string]string) NewContainerOpts {
 }
 
 // WithSnapshotter sets the provided snapshotter for use by the container
+//
+// This option must appear before other snapshotter options to have an effect.
 func WithSnapshotter(name string) NewContainerOpts {
 	return func(ctx context.Context, client *Client, c *containers.Container) error {
 		c.Snapshotter = name
@@ -48,6 +52,7 @@ func WithSnapshotter(name string) NewContainerOpts {
 // WithSnapshot uses an existing root filesystem for the container
 func WithSnapshot(id string) NewContainerOpts {
 	return func(ctx context.Context, client *Client, c *containers.Container) error {
+		setSnapshotterIfEmpty(c)
 		// check that the snapshot exists, if not, fail on creation
 		if _, err := client.SnapshotService(c.Snapshotter).Mounts(ctx, id); err != nil {
 			return err
@@ -65,6 +70,7 @@ func WithNewSnapshot(id string, i Image) NewContainerOpts {
 		if err != nil {
 			return err
 		}
+		setSnapshotterIfEmpty(c)
 		if _, err := client.SnapshotService(c.Snapshotter).Prepare(ctx, id, identity.ChainID(diffIDs).String()); err != nil {
 			return err
 		}
@@ -77,6 +83,9 @@ func WithNewSnapshot(id string, i Image) NewContainerOpts {
 // WithSnapshotCleanup deletes the rootfs allocated for the container
 func WithSnapshotCleanup(ctx context.Context, client *Client, c containers.Container) error {
 	if c.RootFS != "" {
+		if c.Snapshotter == "" {
+			return errors.Wrapf(errdefs.ErrInvalidArgument, "container.Snapshotter must be set to cleanup rootfs")
+		}
 		return client.SnapshotService(c.Snapshotter).Remove(ctx, c.RootFS)
 	}
 	return nil
@@ -90,6 +99,7 @@ func WithNewSnapshotView(id string, i Image) NewContainerOpts {
 		if err != nil {
 			return err
 		}
+		setSnapshotterIfEmpty(c)
 		if _, err := client.SnapshotService(c.Snapshotter).View(ctx, id, identity.ChainID(diffIDs).String()); err != nil {
 			return err
 		}
@@ -98,3 +108,9 @@ func WithNewSnapshotView(id string, i Image) NewContainerOpts {
 		return nil
 	}
 }
+
+func setSnapshotterIfEmpty(c *containers.Container) {
+	if c.Snapshotter == "" {
+		c.Snapshotter = DefaultSnapshotter
+	}
+}

container_opts_unix.go

@@ -46,6 +46,7 @@ func WithCheckpoint(desc v1.Descriptor, rootfsID string) NewContainerOpts {
 				if err != nil {
 					return err
 				}
+				setSnapshotterIfEmpty(c)
 				if _, err := client.SnapshotService(c.Snapshotter).Prepare(ctx, rootfsID, identity.ChainID(diffIDs).String()); err != nil {
 					if !errdefs.IsAlreadyExists(err) {
 						return err

services/snapshot/default_linux.go

@@ -1,5 +0,0 @@
-package snapshot
-
-const (
-	defaultSnapshotter = "overlayfs"
-)

services/snapshot/default_unix.go

@@ -1,7 +0,0 @@
-// +build darwin freebsd solaris
-
-package snapshot
-
-const (
-	defaultSnapshotter = "naive"
-)

services/snapshot/default_windows.go

@@ -1,5 +0,0 @@
-package snapshot
-
-const (
-	defaultSnapshotter = "windows"
-)

services/snapshot/service.go

@@ -20,11 +20,6 @@ import (
 	"google.golang.org/grpc"
 )
 
-type config struct {
-	// Default is the default snapshotter to use for the service
-	Default string `toml:"default,omitempty"`
-}
-
 func init() {
 	plugin.Register(&plugin.Registration{
 		Type: plugin.GRPCPlugin,
@@ -33,9 +28,6 @@ func init() {
 			plugin.SnapshotPlugin,
 			plugin.MetadataPlugin,
 		},
-		Config: &config{
-			Default: defaultSnapshotter,
-		},
 		Init: newService,
 	})
 }
@@ -43,9 +35,8 @@ func init() {
 var empty = &protoempty.Empty{}
 
 type service struct {
-	snapshotters           map[string]snapshot.Snapshotter
-	defaultSnapshotterName string
-	publisher              events.Publisher
+	snapshotters map[string]snapshot.Snapshotter
+	publisher    events.Publisher
 }
 
 func newService(ic *plugin.InitContext) (interface{}, error) {
@@ -62,26 +53,24 @@ func newService(ic *plugin.InitContext) (interface{}, error) {
 		snapshotters[name] = metadata.NewSnapshotter(md.(*bolt.DB), name, sn.(snapshot.Snapshotter))
 	}
 
-	cfg := ic.Config.(*config)
-	_, ok := snapshotters[cfg.Default]
-	if !ok {
-		return nil, errors.Errorf("default snapshotter not loaded: %s", cfg.Default)
+	if len(snapshotters) == 0 {
+		return nil, errors.Errorf("failed to create snapshotter service: no snapshotters loaded")
 	}
 
 	return &service{
-		snapshotters:           snapshotters,
-		defaultSnapshotterName: cfg.Default,
-		publisher:              ic.Events,
+		snapshotters: snapshotters,
+		publisher:    ic.Events,
 	}, nil
 }
 
 func (s *service) getSnapshotter(name string) (snapshot.Snapshotter, error) {
 	if name == "" {
-		name = s.defaultSnapshotterName
+		return nil, errdefs.ToGRPCf(errdefs.ErrInvalidArgument, "snapshotter argument missing")
 	}
+
 	sn, ok := s.snapshotters[name]
 	if !ok {
-		return nil, errors.Errorf("snapshotter not loaded: %s", name)
+		return nil, errdefs.ToGRPCf(errdefs.ErrInvalidArgument, "snapshotter not loaded: %s", name)
 	}
 	return sn, nil
 }

snapshot_test.go

@@ -15,7 +15,7 @@ func newSnapshotter(ctx context.Context, root string) (snapshot.Snapshotter, fun
 		return nil, nil, err
 	}
 
-	sn := client.SnapshotService("")
+	sn := client.SnapshotService(DefaultSnapshotter)
 
 	return sn, func() {
 		client.Close()

snapshotter_default_linux.go

@@ -0,0 +1,8 @@
+package containerd
+
+const (
+	// DefaultSnapshotter will set the default snapshotter for the platform.
+	// This will be based on the client compilation target, so take that into
+	// account when choosing this value.
+	DefaultSnapshotter = "overlayfs"
+)

snapshotter_default_unix.go

@@ -0,0 +1,10 @@
+// +build darwin freebsd solaris
+
+package containerd
+
+const (
+	// DefaultSnapshotter will set the default snapshotter for the platform.
+	// This will be based on the client compilation target, so take that into
+	// account when choosing this value.
+	DefaultSnapshotter = "naive"
+)

snapshotter_default_windows.go

@@ -0,0 +1,8 @@
+package containerd
+
+const (
+	// DefaultSnapshotter will set the default snapshotter for the platform.
+	// This will be based on the client compilation target, so take that into
+	// account when choosing this value.
+	DefaultSnapshotter = "windows"
+)

spec_opts_unix.go

@@ -233,6 +233,9 @@ func WithRemappedSnapshot(id string, i Image, uid, gid uint32) NewContainerOpts
 		if err != nil {
 			return err
 		}
+
+		setSnapshotterIfEmpty(c)
+
 		var (
 			snapshotter = client.SnapshotService(c.Snapshotter)
 			parent      = identity.ChainID(diffIDs).String()