From 17771d0af0b7295982d8f10cfd7ba4924e906c5f Mon Sep 17 00:00:00 2001 From: bin liu Date: Fri, 29 Jul 2022 10:31:10 +0800 Subject: [PATCH] Refactor: Abstract the way to handle remote snapshotter As more and more snapshotter are added, the original processing code is getting longer and longer. By refactoring these processing into an interface, we can do some decoupling and abstraction work to make the code simple and clean. Signed-off-by: bin liu --- pkg/imgutil/imgutil.go | 59 +----------- pkg/imgutil/snapshotter.go | 134 ++++++++++++++++++++++++++ pkg/imgutil/snapshotter_test.go | 162 ++++++++++++++++++++++++++++++++ 3 files changed, 301 insertions(+), 54 deletions(-) create mode 100644 pkg/imgutil/snapshotter.go create mode 100644 pkg/imgutil/snapshotter_test.go diff --git a/pkg/imgutil/imgutil.go b/pkg/imgutil/imgutil.go index 806bae08cdc..34f2780f655 100644 --- a/pkg/imgutil/imgutil.go +++ b/pkg/imgutil/imgutil.go @@ -22,7 +22,6 @@ import ( "fmt" "io" "reflect" - "strings" "github.com/containerd/containerd" "github.com/containerd/containerd/content" @@ -30,15 +29,12 @@ import ( "github.com/containerd/containerd/platforms" refdocker "github.com/containerd/containerd/reference/docker" "github.com/containerd/containerd/remotes" - "github.com/containerd/containerd/snapshots" "github.com/containerd/imgcrypt" "github.com/containerd/imgcrypt/images/encryption" "github.com/containerd/nerdctl/pkg/errutil" "github.com/containerd/nerdctl/pkg/idutil/imagewalker" "github.com/containerd/nerdctl/pkg/imgutil/dockerconfigresolver" "github.com/containerd/nerdctl/pkg/imgutil/pull" - nyduslabel "github.com/containerd/nydus-snapshotter/pkg/label" - "github.com/containerd/stargz-snapshotter/fs/source" "github.com/docker/docker/errdefs" ocispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -77,7 +73,7 @@ func GetExistingImage(ctx context.Context, client *containerd.Client, snapshotte Image: image, ImageConfig: *imgConfig, Snapshotter: snapshotter, - Remote: isStargz(snapshotter) || isOverlaybd(snapshotter), + Remote: getSnapshotterOpts(snapshotter).isRemote(), } if unpacked, err := image.IsUnpacked(ctx, snapshotter); err == nil && !unpacked { if err := image.Unpack(ctx, snapshotter); err != nil { @@ -222,7 +218,7 @@ func PullImage(ctx context.Context, client *containerd.Client, stdout, stderr io unpackB = len(ocispecPlatforms) == 1 } - var sgz, overlaybd, nydus bool + snOpt := getSnapshotterOpts(snapshotter) if unpackB { logrus.Debugf("The image will be unpacked for platform %q, snapshotter %q.", ocispecPlatforms[0], snapshotter) imgcryptPayload := imgcrypt.Payload{} @@ -231,35 +227,8 @@ func PullImage(ctx context.Context, client *containerd.Client, stdout, stderr io containerd.WithPullUnpack, containerd.WithUnpackOpts([]containerd.UnpackOpt{imgcryptUnpackOpt})) - sgz = isStargz(snapshotter) - if sgz { - // TODO: support "skip-content-verify" - config.RemoteOpts = append( - config.RemoteOpts, - containerd.WithImageHandlerWrapper(source.AppendDefaultLabelsHandlerWrapper(ref, 10*1024*1024)), - ) - } - nydus = isNydus(snapshotter) - if nydus { - config.RemoteOpts = append( - config.RemoteOpts, - containerd.WithImageHandlerWrapper(nyduslabel.AppendLabelsHandlerWrapper(ref)), - ) - } - overlaybd = isOverlaybd(snapshotter) - if overlaybd { - snlabel := map[string]string{"containerd.io/snapshot/image-ref": ref} - logrus.Debugf("append remote opts: %s", snlabel) - - config.RemoteOpts = append( - config.RemoteOpts, - containerd.WithPullSnapshotter(snapshotter, snapshots.WithLabels(snlabel)), - ) - } else { - config.RemoteOpts = append( - config.RemoteOpts, - containerd.WithPullSnapshotter(snapshotter)) - } + // different remote snapshotters will update pull.Config separately + snOpt.apply(config, ref) } else { logrus.Debugf("The image will not be unpacked. Platforms=%v.", ocispecPlatforms) } @@ -276,30 +245,12 @@ func PullImage(ctx context.Context, client *containerd.Client, stdout, stderr io Image: containerdImage, ImageConfig: *imgConfig, Snapshotter: snapshotter, - Remote: (sgz || overlaybd || nydus), + Remote: snOpt.isRemote(), } return res, nil } -func isStargz(sn string) bool { - if !strings.Contains(sn, "stargz") { - return false - } - if sn != "stargz" { - logrus.Debugf("assuming %q to be a stargz-compatible snapshotter", sn) - } - return true -} - -func isOverlaybd(sn string) bool { - return sn == "overlaybd" -} - -func isNydus(sn string) bool { - return sn == "nydus" -} - func getImageConfig(ctx context.Context, image containerd.Image) (*ocispec.ImageConfig, error) { desc, err := image.Config(ctx) if err != nil { diff --git a/pkg/imgutil/snapshotter.go b/pkg/imgutil/snapshotter.go new file mode 100644 index 00000000000..48e3d19731b --- /dev/null +++ b/pkg/imgutil/snapshotter.go @@ -0,0 +1,134 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package imgutil + +import ( + "strings" + + "github.com/containerd/containerd" + "github.com/containerd/containerd/snapshots" + "github.com/containerd/nerdctl/pkg/imgutil/pull" + nyduslabel "github.com/containerd/nydus-snapshotter/pkg/label" + "github.com/containerd/stargz-snapshotter/fs/source" + "github.com/sirupsen/logrus" +) + +const ( + snapshotterNameOverlaybd = "overlaybd" + snapshotterNameStargz = "stargz" + snapshotterNameNydus = "nydus" + + // prefetch size for stargz + prefetchSize = 10 * 1024 * 1024 + + overlaybdLabelImageRef = "containerd.io/snapshot/image-ref" +) + +// remote snapshotters explicitly handled by nerdctl +var builtinRemoteSnapshotterOpts = map[string]snapshotterOpts{ + snapshotterNameOverlaybd: &overlaybdSnapshotterOpts{}, + snapshotterNameStargz: &stargzSnapshotterOpts{}, + snapshotterNameNydus: &nydusSnapshotterOpts{}, +} + +// snapshotterOpts is used to update pull config +// for different snapshotters +type snapshotterOpts interface { + apply(config *pull.Config, ref string) + isRemote() bool +} + +// getSnapshotterOpts get snapshotter opts by fuzzy matching of the snapshotter name +func getSnapshotterOpts(snapshotter string) snapshotterOpts { + for sn, sno := range builtinRemoteSnapshotterOpts { + if strings.Contains(snapshotter, sn) { + if snapshotter != sn { + logrus.Debugf("assuming %s to be a %s-compatible snapshotter", snapshotter, sn) + } + return sno + } + } + + return &defaultSnapshotterOpts{snapshotter: snapshotter} +} + +// remoteSnapshotter is used as a default implementation for +// interface `snapshotterOpts.isRemote()` function +type remoteSnapshotter struct{} + +func (rs *remoteSnapshotter) isRemote() bool { + return true +} + +// defaultSnapshotterOpts is for snapshotters that +// not handled separately +type defaultSnapshotterOpts struct { + snapshotter string +} + +func (dsn *defaultSnapshotterOpts) apply(config *pull.Config, _ref string) { + config.RemoteOpts = append( + config.RemoteOpts, + containerd.WithPullSnapshotter(dsn.snapshotter)) +} + +// defaultSnapshotterOpts is not a remote snapshotter +func (dsn *defaultSnapshotterOpts) isRemote() bool { + return false +} + +// stargzSnapshotterOpts for stargz snapshotter +type stargzSnapshotterOpts struct { + remoteSnapshotter +} + +func (ssn *stargzSnapshotterOpts) apply(config *pull.Config, ref string) { + // TODO: support "skip-content-verify" + config.RemoteOpts = append( + config.RemoteOpts, + containerd.WithImageHandlerWrapper(source.AppendDefaultLabelsHandlerWrapper(ref, prefetchSize)), + containerd.WithPullSnapshotter(snapshotterNameStargz), + ) +} + +// overlaybdSnapshotterOpts for overlaybd snapshotter +type overlaybdSnapshotterOpts struct { + remoteSnapshotter +} + +func (osn *overlaybdSnapshotterOpts) apply(config *pull.Config, ref string) { + snlabel := map[string]string{overlaybdLabelImageRef: ref} + logrus.Debugf("append remote opts: %s", snlabel) + + config.RemoteOpts = append( + config.RemoteOpts, + containerd.WithPullSnapshotter(snapshotterNameOverlaybd, snapshots.WithLabels(snlabel)), + ) +} + +// nydusSnapshotterOpts for nydus snapshotter +type nydusSnapshotterOpts struct { + remoteSnapshotter +} + +func (nsn *nydusSnapshotterOpts) apply(config *pull.Config, ref string) { + config.RemoteOpts = append( + config.RemoteOpts, + containerd.WithImageHandlerWrapper(nyduslabel.AppendLabelsHandlerWrapper(ref)), + containerd.WithPullSnapshotter(snapshotterNameNydus), + ) +} diff --git a/pkg/imgutil/snapshotter_test.go b/pkg/imgutil/snapshotter_test.go new file mode 100644 index 00000000000..f435326125d --- /dev/null +++ b/pkg/imgutil/snapshotter_test.go @@ -0,0 +1,162 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package imgutil + +import ( + "context" + "fmt" + "reflect" + "testing" + + "github.com/containerd/containerd" + "github.com/containerd/containerd/snapshots" + "github.com/containerd/nerdctl/pkg/imgutil/pull" + nyduslabel "github.com/containerd/nydus-snapshotter/pkg/label" + "github.com/containerd/stargz-snapshotter/fs/config" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "gotest.tools/v3/assert" +) + +const ( + targetRefLabel = "containerd.io/snapshot/remote/stargz.reference" + testRef = "test:latest" +) + +func TestGetSnapshotterOpts(t *testing.T) { + type testCase struct { + sns []string + want snapshotterOpts + } + testCases := []testCase{ + { + sns: []string{"overlayfs"}, + want: &defaultSnapshotterOpts{snapshotter: "overlayfs"}, + }, + { + sns: []string{"overlayfs2"}, + want: &defaultSnapshotterOpts{snapshotter: "overlayfs2"}, + }, + { + sns: []string{"stargz", "stargz-v1"}, + want: &stargzSnapshotterOpts{}, + }, + { + sns: []string{"overlaybd", "overlaybd-v2"}, + want: &overlaybdSnapshotterOpts{}, + }, + { + sns: []string{"nydus", "nydus-v3"}, + want: &nydusSnapshotterOpts{}, + }, + } + for _, tc := range testCases { + for i := range tc.sns { + got := getSnapshotterOpts(tc.sns[i]) + if !reflect.DeepEqual(got, tc.want) { + t.Errorf("getSnapshotterOpts() got = %v, want %v", got, tc.want) + } + } + } +} + +func getAndApplyRemoteOpts(t *testing.T, sn string) *containerd.RemoteContext { + config := &pull.Config{} + snOpts := getSnapshotterOpts(sn) + snOpts.apply(config, testRef) + + rc := &containerd.RemoteContext{} + for _, o := range config.RemoteOpts { + // here passing a nil client is safe + // because the remote opts will not use client + if err := o(nil, rc); err != nil { + t.Errorf("failed to apply remote opts: %s", err) + } + } + + return rc +} + +func TestDefaultSnapshotterOpts(t *testing.T) { + rc := getAndApplyRemoteOpts(t, "overlayfs") + assert.Equal(t, rc.Snapshotter, "overlayfs") +} + +// dummyImageHandler will return a dummy layer +// see https://github.com/containerd/containerd/blob/77d53d2d230c3bcd3f02e6f493019a72905c875b/images/mediatypes.go#L115 +type dummyImageHandler struct{} + +func (dih *dummyImageHandler) Handle(_ctx context.Context, _desc ocispec.Descriptor) (subdescs []ocispec.Descriptor, err error) { + return []ocispec.Descriptor{ + { + MediaType: "application/vnd.oci.image.layer.dummy", + }, + }, nil +} + +func TestNydusSnapshotterOpts(t *testing.T) { + rc := getAndApplyRemoteOpts(t, "nydus") + assert.Equal(t, rc.Snapshotter, snapshotterNameNydus) + + desc := ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageManifest, + } + + h := &dummyImageHandler{} + got, err := rc.HandlerWrapper(h).Handle(context.Background(), desc) + + assert.NilError(t, err) + assert.Check(t, len(got) == 1) + assert.Check(t, got[0].Annotations != nil) + assert.Equal(t, got[0].Annotations[nyduslabel.CRIImageRef], testRef) +} + +func TestOverlaybdSnapshotterOpts(t *testing.T) { + rc := getAndApplyRemoteOpts(t, "overlaybd") + assert.Equal(t, rc.Snapshotter, snapshotterNameOverlaybd) + + info := &snapshots.Info{} + assert.Check(t, rc.SnapshotterOpts != nil) + + for _, o := range rc.SnapshotterOpts { + err := o(info) + assert.NilError(t, err) + } + + assert.Check(t, info != nil) + assert.Check(t, info.Labels != nil) + assert.Check(t, len(info.Labels) == 1) + + assert.Equal(t, info.Labels[overlaybdLabelImageRef], testRef) +} + +func TestStargzSnapshotterOpts(t *testing.T) { + rc := getAndApplyRemoteOpts(t, "stargz") + assert.Equal(t, rc.Snapshotter, snapshotterNameStargz) + + desc := ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageManifest, + } + + h := &dummyImageHandler{} + got, err := rc.HandlerWrapper(h).Handle(context.Background(), desc) + + assert.NilError(t, err) + assert.Check(t, len(got) == 1) + assert.Check(t, got[0].Annotations != nil) + assert.Equal(t, got[0].Annotations[config.TargetPrefetchSizeLabel], fmt.Sprintf("%d", prefetchSize)) + assert.Equal(t, got[0].Annotations[targetRefLabel], testRef) +}