Skip to content

Commit 677257f

Browse files
stevvooestevvooe
committed
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>
1 parent 783ed05 commit 677257f

13 files changed

+61
-38
lines changed

‎client.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@ func defaultRemoteContext() *RemoteContext {
192192
Resolver: docker.NewResolver(docker.ResolverOptions{
193193
Client: http.DefaultClient,
194194
}),
195+
Snapshotter: DefaultSnapshotter,
195196
}
196197
}
197198

‎container.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,10 @@ func (c *container) NewTask(ctx context.Context, ioCreate IOCreation, opts ...Ne
174174
Stderr: cfg.Stderr,
175175
}
176176
if c.c.RootFS != "" {
177+
if c.c.Snapshotter == "" {
178+
return nil, errors.Wrapf(errdefs.ErrInvalidArgument, "unable to resolve rootfs mounts without snapshotter on container")
179+
}
180+
177181
// get the rootfs from the snapshotter and add it to the request
178182
mounts, err := c.client.SnapshotService(c.c.Snapshotter).Mounts(ctx, c.c.RootFS)
179183
if err != nil {

‎container_opts.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ import (
44
"context"
55

66
"github.com/containerd/containerd/containers"
7+
"github.com/containerd/containerd/errdefs"
78
"github.com/opencontainers/image-spec/identity"
9+
"github.com/pkg/errors"
810
)
911

1012
// NewContainerOpts allows the caller to set additional options when creating a container
@@ -38,6 +40,8 @@ func WithContainerLabels(labels map[string]string) NewContainerOpts {
3840
}
3941

4042
// WithSnapshotter sets the provided snapshotter for use by the container
43+
//
44+
// This option must appear before other snapshotter options to have an effect.
4145
func WithSnapshotter(name string) NewContainerOpts {
4246
return func(ctx context.Context, client *Client, c *containers.Container) error {
4347
c.Snapshotter = name
@@ -48,6 +52,7 @@ func WithSnapshotter(name string) NewContainerOpts {
4852
// WithSnapshot uses an existing root filesystem for the container
4953
func WithSnapshot(id string) NewContainerOpts {
5054
return func(ctx context.Context, client *Client, c *containers.Container) error {
55+
setSnapshotterIfEmpty(c)
5156
// check that the snapshot exists, if not, fail on creation
5257
if _, err := client.SnapshotService(c.Snapshotter).Mounts(ctx, id); err != nil {
5358
return err
@@ -65,6 +70,7 @@ func WithNewSnapshot(id string, i Image) NewContainerOpts {
6570
if err != nil {
6671
return err
6772
}
73+
setSnapshotterIfEmpty(c)
6874
if _, err := client.SnapshotService(c.Snapshotter).Prepare(ctx, id, identity.ChainID(diffIDs).String()); err != nil {
6975
return err
7076
}
@@ -77,6 +83,9 @@ func WithNewSnapshot(id string, i Image) NewContainerOpts {
7783
// WithSnapshotCleanup deletes the rootfs allocated for the container
7884
func WithSnapshotCleanup(ctx context.Context, client *Client, c containers.Container) error {
7985
if c.RootFS != "" {
86+
if c.Snapshotter == "" {
87+
return errors.Wrapf(errdefs.ErrInvalidArgument, "container.Snapshotter must be set to cleanup rootfs")
88+
}
8089
return client.SnapshotService(c.Snapshotter).Remove(ctx, c.RootFS)
8190
}
8291
return nil
@@ -90,6 +99,7 @@ func WithNewSnapshotView(id string, i Image) NewContainerOpts {
9099
if err != nil {
91100
return err
92101
}
102+
setSnapshotterIfEmpty(c)
93103
if _, err := client.SnapshotService(c.Snapshotter).View(ctx, id, identity.ChainID(diffIDs).String()); err != nil {
94104
return err
95105
}
@@ -98,3 +108,9 @@ func WithNewSnapshotView(id string, i Image) NewContainerOpts {
98108
return nil
99109
}
100110
}
111+
112+
func setSnapshotterIfEmpty(c *containers.Container) {
113+
if c.Snapshotter == "" {
114+
c.Snapshotter = DefaultSnapshotter
115+
}
116+
}

‎container_opts_unix.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ func WithCheckpoint(desc v1.Descriptor, rootfsID string) NewContainerOpts {
4646
if err != nil {
4747
return err
4848
}
49+
setSnapshotterIfEmpty(c)
4950
if _, err := client.SnapshotService(c.Snapshotter).Prepare(ctx, rootfsID, identity.ChainID(diffIDs).String()); err != nil {
5051
if !errdefs.IsAlreadyExists(err) {
5152
return err

‎services/snapshot/default_linux.go‎

Lines changed: 0 additions & 5 deletions
This file was deleted.

‎services/snapshot/default_unix.go‎

Lines changed: 0 additions & 7 deletions
This file was deleted.

‎services/snapshot/default_windows.go‎

Lines changed: 0 additions & 5 deletions
This file was deleted.

‎services/snapshot/service.go‎

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,6 @@ import (
2020
"google.golang.org/grpc"
2121
)
2222

23-
type config struct {
24-
// Default is the default snapshotter to use for the service
25-
Default string `toml:"default,omitempty"`
26-
}
27-
2823
func init() {
2924
plugin.Register(&plugin.Registration{
3025
Type: plugin.GRPCPlugin,
@@ -33,19 +28,15 @@ func init() {
3328
plugin.SnapshotPlugin,
3429
plugin.MetadataPlugin,
3530
},
36-
Config: &config{
37-
Default: defaultSnapshotter,
38-
},
3931
Init: newService,
4032
})
4133
}
4234

4335
var empty = &protoempty.Empty{}
4436

4537
type service struct {
46-
snapshotters map[string]snapshot.Snapshotter
47-
defaultSnapshotterName string
48-
publisher events.Publisher
38+
snapshotters map[string]snapshot.Snapshotter
39+
publisher events.Publisher
4940
}
5041

5142
func newService(ic *plugin.InitContext) (interface{}, error) {
@@ -62,26 +53,24 @@ func newService(ic *plugin.InitContext) (interface{}, error) {
6253
snapshotters[name] = metadata.NewSnapshotter(md.(*bolt.DB), name, sn.(snapshot.Snapshotter))
6354
}
6455

65-
cfg := ic.Config.(*config)
66-
_, ok := snapshotters[cfg.Default]
67-
if !ok {
68-
return nil, errors.Errorf("default snapshotter not loaded: %s", cfg.Default)
56+
if len(snapshotters) == 0 {
57+
return nil, errors.Errorf("failed to create snapshotter service: no snapshotters loaded")
6958
}
7059

7160
return &service{
72-
snapshotters: snapshotters,
73-
defaultSnapshotterName: cfg.Default,
74-
publisher: ic.Events,
61+
snapshotters: snapshotters,
62+
publisher: ic.Events,
7563
}, nil
7664
}
7765

7866
func (s *service) getSnapshotter(name string) (snapshot.Snapshotter, error) {
7967
if name == "" {
80-
name = s.defaultSnapshotterName
68+
return nil, errdefs.ToGRPCf(errdefs.ErrInvalidArgument, "snapshotter argument missing")
8169
}
70+
8271
sn, ok := s.snapshotters[name]
8372
if !ok {
84-
return nil, errors.Errorf("snapshotter not loaded: %s", name)
73+
return nil, errdefs.ToGRPCf(errdefs.ErrInvalidArgument, "snapshotter not loaded: %s", name)
8574
}
8675
return sn, nil
8776
}

‎snapshot_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ func newSnapshotter(ctx context.Context, root string) (snapshot.Snapshotter, fun
1515
return nil, nil, err
1616
}
1717

18-
sn := client.SnapshotService("")
18+
sn := client.SnapshotService(DefaultSnapshotter)
1919

2020
return sn, func() {
2121
client.Close()

‎snapshotter_default_linux.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
package containerd
2+
3+
const (
4+
// DefaultSnapshotter will set the default snapshotter for the platform.
5+
// This will be based on the client compilation target, so take that into
6+
// account when choosing this value.
7+
DefaultSnapshotter = "overlayfs"
8+
)

0 commit comments

Comments
 (0)