Merge pull request #3849 from imeoer/fix-provenance

solver: fix nydus force compression for provenance

.github/workflows/.test.yml

@@ -155,7 +155,6 @@ jobs:
           BUILDKITD_TAGS: ${{ matrix.tags }}
       -
         name: Test
-        continue-on-error: ${{ matrix.tags == 'nydus' }}
         run: |
           export TEST_REPORT_SUFFIX=-${{ github.job }}-$(echo "${{ matrix.pkg }}-${{ matrix.skip-integration-tests }}-${{ matrix.kind }}-${{ matrix.worker }}-${{ matrix.tags }}" | tr -dc '[:alnum:]-\n\r' | tr '[:upper:]' '[:lower:]')
           if [ -n "${{ matrix.tags }}" ]; then

cache/compression_nydus.go

@@ -11,7 +11,6 @@ import (
 	"github.com/containerd/containerd/content"
 	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/labels"
-	"github.com/moby/buildkit/cache/config"
 	"github.com/moby/buildkit/session"
 	"github.com/moby/buildkit/util/compression"
 	digest "github.com/opencontainers/go-digest"
@@ -28,20 +27,6 @@ func init() {
 	)
 }
 
-// Nydus compression type can't be mixed with other compression types in the same image,
-// so if `source` is this kind of layer, but the target is other compression type, we
-// should do the forced compression.
-func needsForceCompression(ctx context.Context, cs content.Store, source ocispecs.Descriptor, refCfg config.RefConfig) bool {
-	if refCfg.Compression.Force {
-		return true
-	}
-	isNydusBlob, _ := compression.Nydus.Is(ctx, cs, source)
-	if refCfg.Compression.Type == compression.Nydus {
-		return !isNydusBlob
-	}
-	return isNydusBlob
-}
-
 // MergeNydus does two steps:
 // 1. Extracts nydus bootstrap from nydus format (nydus blob + nydus bootstrap) for each layer.
 // 2. Merge all nydus bootstraps into a final bootstrap (will as an extra layer).

cache/remote.go

@@ -212,7 +212,7 @@ func (sr *immutableRef) getRemote(ctx context.Context, createIfNeeded bool, refC
 			}
 		}
 
-		if needsForceCompression(ctx, sr.cm.ContentStore, desc, refCfg) {
+		if refCfg.Compression.Force {
 			if needs, err := refCfg.Compression.Type.NeedsConversion(ctx, sr.cm.ContentStore, desc); err != nil {
 				return nil, err
 			} else if needs {

client/client_nydus_test.go

@@ -5,12 +5,11 @@ package client
 
 import (
 	"fmt"
-	"strconv"
 	"testing"
 
 	"github.com/containerd/containerd/images"
 	"github.com/containerd/containerd/namespaces"
-	nydusify "github.com/containerd/nydus-snapshotter/pkg/converter"
+	"github.com/containerd/nydus-snapshotter/pkg/converter"
 	"github.com/moby/buildkit/client/llb"
 	"github.com/moby/buildkit/identity"
 	"github.com/moby/buildkit/util/compression"
@@ -21,9 +20,9 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestNydusIntegration(t *testing.T) {
-	testIntegration(
-		t,
+func init() {
+	allTests = append(
+		allTests,
 		testBuildExportNydusWithHybrid,
 	)
 }
@@ -68,10 +67,11 @@ func testBuildExportNydusWithHybrid(t *testing.T, sb integration.Sandbox) {
 				{
 					Type: ExporterImage,
 					Attrs: map[string]string{
-						"name":           target,
-						"push":           "true",
-						"compression":    "nydus",
-						"oci-mediatypes": "true",
+						"name":              target,
+						"push":              "true",
+						"compression":       "nydus",
+						"oci-mediatypes":    "true",
+						"force-compression": "true",
 					},
 				},
 			},
@@ -85,12 +85,12 @@ func testBuildExportNydusWithHybrid(t *testing.T, sb integration.Sandbox) {
 		require.NoError(t, err)
 
 		require.Equal(t, len(manifest.Layers), 3)
-		require.Equal(t, "true", manifest.Layers[0].Annotations[nydusify.LayerAnnotationNydusBlob])
-		require.Equal(t, "true", manifest.Layers[1].Annotations[nydusify.LayerAnnotationNydusBlob])
-		require.Equal(t, "true", manifest.Layers[2].Annotations[nydusify.LayerAnnotationNydusBootstrap])
+		require.Equal(t, "true", manifest.Layers[0].Annotations[converter.LayerAnnotationNydusBlob])
+		require.Equal(t, "true", manifest.Layers[1].Annotations[converter.LayerAnnotationNydusBlob])
+		require.Equal(t, "true", manifest.Layers[2].Annotations[converter.LayerAnnotationNydusBootstrap])
 	}
 
-	buildOther := func(file string, compType compression.Type, forceCompression bool) {
+	buildOther := func(file string, compType compression.Type) {
 		orgImage := "docker.io/library/alpine:latest"
 		baseDef := llb.Image(orgImage).Run(llb.Args([]string{"/bin/touch", "/" + file}))
 		def, err := baseDef.Marshal(sb.Context())
@@ -110,7 +110,7 @@ func testBuildExportNydusWithHybrid(t *testing.T, sb integration.Sandbox) {
 						"push":              "true",
 						"compression":       compType.String(),
 						"oci-mediatypes":    "true",
-						"force-compression": strconv.FormatBool(forceCompression),
+						"force-compression": "true",
 					},
 				},
 			},
@@ -131,10 +131,10 @@ func testBuildExportNydusWithHybrid(t *testing.T, sb integration.Sandbox) {
 	// Make sure that the nydus compression layer is not mixed with other
 	// types of compression layers in an image.
 	buildNydus("foo")
-	buildOther("foo", compression.Gzip, false)
-	buildOther("foo", compression.Zstd, true)
+	buildOther("foo", compression.Gzip)
+	buildOther("foo", compression.Zstd)
 
-	buildOther("bar", compression.Gzip, false)
-	buildOther("bar", compression.Zstd, true)
+	buildOther("bar", compression.Gzip)
+	buildOther("bar", compression.Zstd)
 	buildNydus("bar")
 }

client/client_test.go

@@ -88,129 +88,130 @@ type nopWriteCloser struct {
 
 func (nopWriteCloser) Close() error { return nil }
 
+var allTests = []func(t *testing.T, sb integration.Sandbox){
+	testCacheExportCacheKeyLoop,
+	testRelativeWorkDir,
+	testFileOpMkdirMkfile,
+	testFileOpCopyRm,
+	testFileOpCopyIncludeExclude,
+	testFileOpRmWildcard,
+	testFileOpCopyUIDCache,
+	testCallDiskUsage,
+	testBuildMultiMount,
+	testBuildHTTPSource,
+	testBuildPushAndValidate,
+	testBuildExportWithUncompressed,
+	testBuildExportScratch,
+	testResolveAndHosts,
+	testUser,
+	testOCIExporter,
+	testOCIExporterContentStore,
+	testWhiteoutParentDir,
+	testFrontendImageNaming,
+	testDuplicateWhiteouts,
+	testSchema1Image,
+	testMountWithNoSource,
+	testInvalidExporter,
+	testReadonlyRootFS,
+	testBasicRegistryCacheImportExport,
+	testBasicLocalCacheImportExport,
+	testBasicS3CacheImportExport,
+	testBasicAzblobCacheImportExport,
+	testCachedMounts,
+	testCopyFromEmptyImage,
+	testProxyEnv,
+	testLocalSymlinkEscape,
+	testTmpfsMounts,
+	testSharedCacheMounts,
+	testSharedCacheMountsNoScratch,
+	testLockedCacheMounts,
+	testDuplicateCacheMount,
+	testRunCacheWithMounts,
+	testParallelLocalBuilds,
+	testSecretEnv,
+	testSecretMounts,
+	testExtraHosts,
+	testShmSize,
+	testUlimit,
+	testCgroupParent,
+	testNetworkMode,
+	testFrontendMetadataReturn,
+	testFrontendUseSolveResults,
+	testSSHMount,
+	testStdinClosed,
+	testHostnameLookup,
+	testHostnameSpecifying,
+	testPushByDigest,
+	testBasicInlineCacheImportExport,
+	testBasicGhaCacheImportExport,
+	testExportBusyboxLocal,
+	testBridgeNetworking,
+	testCacheMountNoCache,
+	testExporterTargetExists,
+	testTarExporterWithSocket,
+	testTarExporterWithSocketCopy,
+	testTarExporterSymlink,
+	testMultipleRegistryCacheImportExport,
+	testSourceMap,
+	testSourceMapFromRef,
+	testLazyImagePush,
+	testStargzLazyPull,
+	testStargzLazyInlineCacheImportExport,
+	testFileOpInputSwap,
+	testRelativeMountpoint,
+	testLocalSourceDiffer,
+	testOCILayoutSource,
+	testOCILayoutPlatformSource,
+	testBuildExportZstd,
+	testPullZstdImage,
+	testMergeOp,
+	testMergeOpCacheInline,
+	testMergeOpCacheMin,
+	testMergeOpCacheMax,
+	testRmSymlink,
+	testMoveParentDir,
+	testBuildExportWithForeignLayer,
+	testZstdLocalCacheExport,
+	testCacheExportIgnoreError,
+	testZstdRegistryCacheImportExport,
+	testZstdLocalCacheImportExport,
+	testUncompressedLocalCacheImportExport,
+	testUncompressedRegistryCacheImportExport,
+	testStargzLazyRegistryCacheImportExport,
+	testValidateDigestOrigin,
+	testCallInfo,
+	testPullWithLayerLimit,
+	testExportAnnotations,
+	testExportAnnotationsMediaTypes,
+	testExportAttestations,
+	testExportedImageLabels,
+	testAttestationDefaultSubject,
+	testSourceDateEpochLayerTimestamps,
+	testSourceDateEpochClamp,
+	testSourceDateEpochReset,
+	testSourceDateEpochLocalExporter,
+	testSourceDateEpochTarExporter,
+	testSourceDateEpochImageExporter,
+	testAttestationBundle,
+	testSBOMScan,
+	testSBOMScanSingleRef,
+	testSBOMSupplements,
+	testMultipleCacheExports,
+	testMountStubsDirectory,
+	testMountStubsTimestamp,
+	testSourcePolicy,
+	testImageManifestRegistryCacheImportExport,
+	testLLBMountPerformance,
+	testClientCustomGRPCOpts,
+	testMultipleRecordsWithSameLayersCacheImportExport,
+	testRegistryEmptyCacheExport,
+	testSnapshotWithMultipleBlobs,
+	testExportLocalNoPlatformSplit,
+	testExportLocalNoPlatformSplitOverwrite,
+}
+
 func TestIntegration(t *testing.T) {
-	testIntegration(
-		t,
-		testCacheExportCacheKeyLoop,
-		testRelativeWorkDir,
-		testFileOpMkdirMkfile,
-		testFileOpCopyRm,
-		testFileOpCopyIncludeExclude,
-		testFileOpRmWildcard,
-		testFileOpCopyUIDCache,
-		testCallDiskUsage,
-		testBuildMultiMount,
-		testBuildHTTPSource,
-		testBuildPushAndValidate,
-		testBuildExportWithUncompressed,
-		testBuildExportScratch,
-		testResolveAndHosts,
-		testUser,
-		testOCIExporter,
-		testOCIExporterContentStore,
-		testWhiteoutParentDir,
-		testFrontendImageNaming,
-		testDuplicateWhiteouts,
-		testSchema1Image,
-		testMountWithNoSource,
-		testInvalidExporter,
-		testReadonlyRootFS,
-		testBasicRegistryCacheImportExport,
-		testBasicLocalCacheImportExport,
-		testBasicS3CacheImportExport,
-		testBasicAzblobCacheImportExport,
-		testCachedMounts,
-		testCopyFromEmptyImage,
-		testProxyEnv,
-		testLocalSymlinkEscape,
-		testTmpfsMounts,
-		testSharedCacheMounts,
-		testSharedCacheMountsNoScratch,
-		testLockedCacheMounts,
-		testDuplicateCacheMount,
-		testRunCacheWithMounts,
-		testParallelLocalBuilds,
-		testSecretEnv,
-		testSecretMounts,
-		testExtraHosts,
-		testShmSize,
-		testUlimit,
-		testCgroupParent,
-		testNetworkMode,
-		testFrontendMetadataReturn,
-		testFrontendUseSolveResults,
-		testSSHMount,
-		testStdinClosed,
-		testHostnameLookup,
-		testHostnameSpecifying,
-		testPushByDigest,
-		testBasicInlineCacheImportExport,
-		testBasicGhaCacheImportExport,
-		testExportBusyboxLocal,
-		testBridgeNetworking,
-		testCacheMountNoCache,
-		testExporterTargetExists,
-		testTarExporterWithSocket,
-		testTarExporterWithSocketCopy,
-		testTarExporterSymlink,
-		testMultipleRegistryCacheImportExport,
-		testSourceMap,
-		testSourceMapFromRef,
-		testLazyImagePush,
-		testStargzLazyPull,
-		testStargzLazyInlineCacheImportExport,
-		testFileOpInputSwap,
-		testRelativeMountpoint,
-		testLocalSourceDiffer,
-		testOCILayoutSource,
-		testOCILayoutPlatformSource,
-		testBuildExportZstd,
-		testPullZstdImage,
-		testMergeOp,
-		testMergeOpCacheInline,
-		testMergeOpCacheMin,
-		testMergeOpCacheMax,
-		testRmSymlink,
-		testMoveParentDir,
-		testBuildExportWithForeignLayer,
-		testZstdLocalCacheExport,
-		testCacheExportIgnoreError,
-		testZstdRegistryCacheImportExport,
-		testZstdLocalCacheImportExport,
-		testUncompressedLocalCacheImportExport,
-		testUncompressedRegistryCacheImportExport,
-		testStargzLazyRegistryCacheImportExport,
-		testValidateDigestOrigin,
-		testCallInfo,
-		testPullWithLayerLimit,
-		testExportAnnotations,
-		testExportAnnotationsMediaTypes,
-		testExportAttestations,
-		testExportedImageLabels,
-		testAttestationDefaultSubject,
-		testSourceDateEpochLayerTimestamps,
-		testSourceDateEpochClamp,
-		testSourceDateEpochReset,
-		testSourceDateEpochLocalExporter,
-		testSourceDateEpochTarExporter,
-		testSourceDateEpochImageExporter,
-		testAttestationBundle,
-		testSBOMScan,
-		testSBOMScanSingleRef,
-		testSBOMSupplements,
-		testMultipleCacheExports,
-		testMountStubsDirectory,
-		testMountStubsTimestamp,
-		testSourcePolicy,
-		testImageManifestRegistryCacheImportExport,
-		testLLBMountPerformance,
-		testClientCustomGRPCOpts,
-		testMultipleRecordsWithSameLayersCacheImportExport,
-		testRegistryEmptyCacheExport,
-		testSnapshotWithMultipleBlobs,
-		testExportLocalNoPlatformSplit,
-		testExportLocalNoPlatformSplitOverwrite,
-	)
+	testIntegration(t, allTests...)
 }
 
 func testIntegration(t *testing.T, funcs ...func(t *testing.T, sb integration.Sandbox)) {