Skip to content

Commit

Permalink
Add integration test for no image index
Browse files Browse the repository at this point in the history
Signed-off-by: David Son <davbson@amazon.com>
  • Loading branch information
sondavidb committed Jul 19, 2024
1 parent 0efaa8e commit 846397c
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 32 deletions.
6 changes: 3 additions & 3 deletions fs/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ func (c *sociContext) Init(fsCtx context.Context, ctx context.Context, imageRef,

desc, err := client.SelectReferrer(ctx, ocispec.Descriptor{Digest: imgDigest}, defaultIndexSelectionPolicy)
if err != nil {
retErr = fmt.Errorf("cannot fetch list of referrers: %w", err)
retErr = fmt.Errorf("%w: cannot fetch list of referrers: %w", snapshot.ErrNoIndex, err)
return
}
indexDesc = desc
Expand All @@ -347,7 +347,7 @@ func (c *sociContext) Init(fsCtx context.Context, ctx context.Context, imageRef,

index, err := FetchSociArtifacts(fsCtx, refspec, indexDesc, store, remoteStore)
if err != nil {
retErr = fmt.Errorf("error trying to fetch SOCI artifacts: %w", err)
retErr = fmt.Errorf("%w: error trying to fetch SOCI artifacts: %w", snapshot.ErrNoIndex, err)
return
}
c.sociIndex = index
Expand Down Expand Up @@ -482,7 +482,7 @@ func (fs *filesystem) Mount(ctx context.Context, mountpoint string, labels map[s
client := src[0].Hosts[0].Client
c, err := fs.getSociContext(ctx, imageRef, sociIndexDigest, imgDigest, client)
if err != nil {
return fmt.Errorf("%w: unable to fetch SOCI artifacts for image %q: %w", snapshot.ErrUnableToLazyLoadImage, imageRef, err)
return fmt.Errorf("unable to fetch SOCI artifacts for image %q: %w", imageRef, err)
}

// Resolve the target layer
Expand Down
45 changes: 43 additions & 2 deletions integration/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,36 @@ func TestOverlayFallbackMetric(t *testing.T) {
{
name: "image with fs.Mount errors results in non-zero overlay fallback",
image: rabbitmqImage,
indexDigestFn: func(sh *shell.Shell, contentStoreType store.ContentStoreType, image imageInfo) string {
indexDigest := buildIndex(sh, image, withMinLayerSize(defaultSpanSize), withContentStoreType(contentStoreType))
contentStorePath := store.DefaultSociContentStorePath
if contentStoreType == "containerd" {
contentStorePath = store.DefaultContainerdContentStorePath
}

output := strings.Trim(string(sh.O("soci", "ztoc", "list")), "\n")
outputLines := strings.Split(output, "\n")
if len(outputLines) < 2 {
t.Fatalf("soci ztoc list output has no ztocs, actual output: %s", output)
}

// Choose a random ztoc to corrupt
ztocInfo := strings.Fields(outputLines[1])
corruptZtocDigest := strings.Split(ztocInfo[0], ":")[1]
// Do a random substitution to corrupt the specific ztoc
sh.X("sed", "-i", "s/a/abc/g", fmt.Sprintf("%s/blobs/sha256/%s", contentStorePath, corruptZtocDigest))

return indexDigest
},
expectedFallbackCount: 1,
},
{
name: "image with no soci index results in no overlay fallback",
image: rabbitmqImage,
indexDigestFn: func(_ *shell.Shell, _ store.ContentStoreType, _ imageInfo) string {
return "invalid index string"
},
expectedFallbackCount: 10,
expectedFallbackCount: 0,
},
}

Expand Down Expand Up @@ -210,6 +236,15 @@ log_fuse_operations = true
expectedCount: 1,
expectFuseOperationFailure: true,
},
{
name: "image without a ztoc doesn't cause fuse failure",
image: pinnedRabbitmqImage,
indexDigestFn: func(t *testing.T, sh *shell.Shell, image imageInfo) string {
return ""
},
metricToCheck: commonmetrics.FuseFailureState,
expectFuseOperationFailure: false,
},
}

for _, tc := range testCases {
Expand All @@ -220,7 +255,13 @@ log_fuse_operations = true
sh.X("nerdctl", "pull", "-q", imgInfo.ref)
indexDigest := tc.indexDigestFn(t, sh, imgInfo)

sh.X(append(imagePullCmd, "--soci-index-digest", indexDigest, imgInfo.ref)...)
args := imagePullCmd
if indexDigest != "" {
args = append(args, "--soci-index-digest", indexDigest)
}
args = append(args, imgInfo.ref)

sh.X(args...)
// this command may fail due to fuse operation failure, use XLog to avoid crashing shell
sh.XLog(append(runSociCmd, "--name", "test", "--rm", imgInfo.ref, "echo", "hi")...)

Expand Down
66 changes: 39 additions & 27 deletions snapshot/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,12 @@ const (
)

var (
// ErrUnableToLazyLoadImage is returned by `fs.Mount` when the image cannot/should not be
// lazy loaded. (eg: If a SOCI index does not exist for an image)
ErrUnableToLazyLoadImage = errors.New("unable to lazy load image")
// ErrNoIndex is returned by `fs.Mount` when an image should not be lazy loaded
// because a SOCI index was not found
ErrNoIndex = errors.New("no valid SOCI index found")
// ErrDeferToContainerRuntime is called when we cannot prepare a remote or local snapshot,
// and must ask the container runtime to handle it instead.
ErrDeferToContainerRuntime = errors.New("deferring to container runtime")
// ErrNoZtoc is returned by `fs.Mount` when there is no zTOC for a particular layer.
ErrNoZtoc = errors.New("no ztoc for layer")
)
Expand Down Expand Up @@ -328,12 +331,15 @@ func (o *snapshotter) Prepare(ctx context.Context, key, parent string, opts ...s
// possible has done some work on this "upper" directory.
return nil, err
}

log.G(lCtx).WithField(remoteSnapshotLogKey, prepareFailed).WithError(err).Warn("failed to prepare remote snapshot")
if !errors.Is(err, ErrNoZtoc) {
switch {
case errors.Is(err, ErrNoZtoc):
// no-op
case errors.Is(err, ErrNoIndex):
skipLazyLoadingImage = true
default:
commonmetrics.IncOperationCount(commonmetrics.FuseMountFailureCount, digest.Digest(""))
if errors.Is(err, ErrUnableToLazyLoadImage) {
skipLazyLoadingImage = true
}
}
}

Expand All @@ -343,28 +349,34 @@ func (o *snapshotter) Prepare(ctx context.Context, key, parent string, opts ...s
// don't fallback here, since there was an error getting mounts
return nil, err
}
// If the underlying FileSystem deems that the image is unable to be lazy loaded, then we
// should completely fallback to the underlying runtime (containerd) to handle pulling and
// unpacking all the layers in the image.
if !skipLazyLoadingImage {
log.G(ctx).WithField("layerDigest", base.Labels[ctdsnapshotters.TargetLayerDigestLabel]).Info("preparing snapshot as local snapshot")
err = o.prepareLocalSnapshot(lCtx, key, base.Labels, mounts)
if err == nil {
err := o.commit(ctx, false, target, key, append(opts, snapshots.WithLabels(base.Labels))...)
if err == nil || errdefs.IsAlreadyExists(err) {
// count also AlreadyExists as "success"
// there's no need to provide any details on []mount.Mount because mounting is already taken care of
// by snapshotter
log.G(lCtx).Info("local snapshot successfully prepared")
return nil, fmt.Errorf("target snapshot %q: %w", target, errdefs.ErrAlreadyExists)
}
log.G(lCtx).WithError(err).Warn("failed to internally commit local snapshot")
// Don't fallback here (= prohibit to use this key again) because the FileSystem
// possible has done some work on this "upper" directory.
return nil, err

// If the underlying FileSystem deems that the image is unable to be lazy loaded,
// then we should completely fallback to the container runtime to handle
// pulling and unpacking all the layers in the image.
if skipLazyLoadingImage {
log.G(lCtx).WithError(err).Warnf("%v; %v", ErrNoIndex, ErrDeferToContainerRuntime)
return mounts, nil
}

log.G(ctx).WithField("layerDigest", base.Labels[ctdsnapshotters.TargetLayerDigestLabel]).Info("preparing snapshot as local snapshot")
err = o.prepareLocalSnapshot(lCtx, key, base.Labels, mounts)
if err == nil {
err := o.commit(ctx, false, target, key, append(opts, snapshots.WithLabels(base.Labels))...)
if err == nil || errdefs.IsAlreadyExists(err) {
// count also AlreadyExists as "success"
// there's no need to provide any details on []mount.Mount because mounting is already taken care of
// by snapshotter
log.G(lCtx).Info("local snapshot successfully prepared")
return nil, fmt.Errorf("target snapshot %q: %w", target, errdefs.ErrAlreadyExists)
}
log.G(lCtx).WithError(err).Warn("failed to internally commit local snapshot")
// Don't fallback here (= prohibit to use this key again) because the FileSystem
// possible has done some work on this "upper" directory.
return nil, err
}
log.G(lCtx).WithError(err).Warn("failed to prepare snapshot; deferring to container runtime")

// Local snapshot setup failed. Generally means something critical has gone wrong.
log.G(lCtx).WithError(err).Warnf("failed to prepare local snapshot; %v", ErrDeferToContainerRuntime)
return mounts, nil
}

Expand Down

0 comments on commit 846397c

Please sign in to comment.