From 34b2dfd5231137ecd1ce695281f21a14715b485c Mon Sep 17 00:00:00 2001 From: Yan Song Date: Thu, 2 Nov 2023 03:41:30 +0000 Subject: [PATCH 1/2] nydusify: fix copy race issue 1. Fix lost namespace on containerd image pull context: ``` pull source image: namespace is required: failed precondition ``` 2. Fix possible semaphore Acquire race on the same one context: ``` panic: semaphore: released more than held ``` Signed-off-by: Yan Song --- contrib/nydusify/pkg/copier/copier.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/contrib/nydusify/pkg/copier/copier.go b/contrib/nydusify/pkg/copier/copier.go index 43032ff40dc..a07c4ea8b5a 100644 --- a/contrib/nydusify/pkg/copier/copier.go +++ b/contrib/nydusify/pkg/copier/copier.go @@ -16,6 +16,7 @@ import ( "github.com/containerd/containerd/content" containerdErrdefs "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/images" + "github.com/containerd/containerd/namespaces" "github.com/containerd/containerd/platforms" "github.com/containerd/containerd/reference/docker" "github.com/containerd/nydus-snapshotter/pkg/converter" @@ -151,7 +152,7 @@ func pushBlobFromBackend( for idx := range blobIDs { func(idx int) { eg.Go(func() error { - sem.Acquire(ctx, 1) + sem.Acquire(context.Background(), 1) defer sem.Release(1) blobID := blobIDs[idx] @@ -244,6 +245,9 @@ func getPlatform(platform *ocispec.Platform) string { } func Copy(ctx context.Context, opt Opt) error { + // Containerd image fetch requires a namespace context. + ctx = namespaces.WithNamespace(ctx, "nydusify") + platformMC, err := platformutil.ParsePlatforms(opt.AllPlatforms, opt.Platforms) if err != nil { return err @@ -319,7 +323,7 @@ func Copy(ctx context.Context, opt Opt) error { for idx := range sourceDescs { func(idx int) { eg.Go(func() error { - sem.Acquire(ctx, 1) + sem.Acquire(context.Background(), 1) defer sem.Release(1) sourceDesc := sourceDescs[idx] From 0be353860912886dcc04892d9ac5b0586350f3f5 Mon Sep 17 00:00:00 2001 From: Yan Song Date: Thu, 2 Nov 2023 03:47:55 +0000 Subject: [PATCH 2/2] smoke: add basic nydusify copy test Signed-off-by: Yan Song --- smoke/tests/compatibility_test.go | 2 +- smoke/tests/image_test.go | 23 +++++++++++++++++++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/smoke/tests/compatibility_test.go b/smoke/tests/compatibility_test.go index a757af9b528..00ed813c5c7 100644 --- a/smoke/tests/compatibility_test.go +++ b/smoke/tests/compatibility_test.go @@ -86,7 +86,7 @@ func (c *CompatibilityTestSuite) TestConvertImages() test.Generator { image := c.prepareImage(c.t, scenario.GetString(paramImage)) return scenario.Str(), func(t *testing.T) { imageTest := &ImageTestSuite{T: t} - imageTest.TestConvertImage(t, *ctx, image) + imageTest.TestConvertAndCopyImage(t, *ctx, image, false) } } } diff --git a/smoke/tests/image_test.go b/smoke/tests/image_test.go index c7c9e730dfb..6cea6d73a16 100644 --- a/smoke/tests/image_test.go +++ b/smoke/tests/image_test.go @@ -63,12 +63,12 @@ func (i *ImageTestSuite) TestConvertImages() test.Generator { image := i.prepareImage(i.T, scenario.GetString(paramImage)) return scenario.Str(), func(t *testing.T) { - i.TestConvertImage(t, *ctx, image) + i.TestConvertAndCopyImage(t, *ctx, image, true) } } } -func (i *ImageTestSuite) TestConvertImage(t *testing.T, ctx tool.Context, source string) { +func (i *ImageTestSuite) TestConvertAndCopyImage(t *testing.T, ctx tool.Context, source string, testCopy bool) { // Prepare work directory ctx.PrepareWorkDir(t) @@ -119,6 +119,25 @@ func (i *ImageTestSuite) TestConvertImage(t *testing.T, ctx tool.Context, source nydusifyPath, logLevel, source, target, ctx.Binary.Builder, ctx.Binary.Nydusd, filepath.Join(ctx.Env.WorkDir, "check"), ) tool.RunWithoutOutput(t, checkCmd) + + if !testCopy { + return + } + + // Copy image + targetCopied := fmt.Sprintf("%s_copied", target) + copyCmd := fmt.Sprintf( + "%s %s copy --source %s --target %s --nydus-image %s --work-dir %s", + ctx.Binary.Nydusify, logLevel, target, targetCopied, ctx.Binary.Builder, ctx.Env.WorkDir, + ) + tool.RunWithoutOutput(t, copyCmd) + + // Check copied image + checkCmd = fmt.Sprintf( + "%s %s check --source %s --target %s --nydus-image %s --nydusd %s --work-dir %s", + nydusifyPath, logLevel, source, targetCopied, ctx.Binary.Builder, ctx.Binary.Nydusd, filepath.Join(ctx.Env.WorkDir, "check"), + ) + tool.RunWithoutOutput(t, checkCmd) } func (i *ImageTestSuite) prepareImage(t *testing.T, image string) string {