diff --git a/cmd/ctr/commands/images/import.go b/cmd/ctr/commands/images/import.go index 5d663df3b..740ff96b4 100644 --- a/cmd/ctr/commands/images/import.go +++ b/cmd/ctr/commands/images/import.go @@ -130,10 +130,10 @@ If foobar.tar contains an OCI ref named "latest" and anonymous ref "sha256:deadb } var platSpec ocispec.Platform - //Only when all-platforms not specified, we will check platform value - //Implicitly if the platforms is empty, it means all-platforms + // Only when all-platforms not specified, we will check platform value + // Implicitly if the platforms is empty, it means all-platforms if !context.Bool("all-platforms") { - //If platform specified, use that one, if not use default + // If platform specified, use that one, if not use default if platform := context.String("platform"); platform != "" { platSpec, err = platforms.Parse(platform) if err != nil { @@ -147,12 +147,12 @@ If foobar.tar contains an OCI ref named "latest" and anonymous ref "sha256:deadb if !context.Bool("no-unpack") { snapshotter := context.String("snapshotter") - //If OS field is not empty, it means platSpec was updated in the above block - //i.e all-platforms was not specified + // If OS field is not empty, it means platSpec was updated in the above block + // i.e all-platforms was not specified if platSpec.OS != "" { opts = append(opts, image.WithUnpack(platSpec, snapshotter)) } else { - //empty spec means all platforms + // Empty spec means all platforms var emptySpec ocispec.Platform opts = append(opts, image.WithUnpack(emptySpec, snapshotter)) } diff --git a/cmd/ctr/commands/images/pull.go b/cmd/ctr/commands/images/pull.go index d4f245729..39833e350 100644 --- a/cmd/ctr/commands/images/pull.go +++ b/cmd/ctr/commands/images/pull.go @@ -114,7 +114,7 @@ command. As part of this process, we do the following: } sopts = append(sopts, image.WithPlatforms(p...)) - //set unpack configuration + // Set unpack configuration for _, platform := range p { sopts = append(sopts, image.WithUnpack(platform, context.String("snapshotter"))) } @@ -125,8 +125,8 @@ command. As part of this process, we do the following: if context.Bool("metadata-only") { sopts = append(sopts, image.WithAllMetadata) // Any with an empty set is None - // TODO: Specify way to specify not default platorm - //config.PlatformMatcher = platforms.Any() + // TODO: Specify way to specify not default platform + // config.PlatformMatcher = platforms.Any() } else if context.Bool("all-metadata") { sopts = append(sopts, image.WithAllMetadata) } diff --git a/pkg/transfer/local/import.go b/pkg/transfer/local/import.go index 9b7dcd9ea..cf9944c72 100644 --- a/pkg/transfer/local/import.go +++ b/pkg/transfer/local/import.go @@ -89,7 +89,7 @@ func (ts *localTransferService) importStream(ctx context.Context, i transfer.Ima handler = images.Handlers(handlerFunc) // First find suitable platforms to unpack into - //If image storer is also an unpacker type, i.e implemented UnpackPlatforms() func + // If image storer is also an unpacker type, i.e implemented UnpackPlatforms() func if iu, ok := is.(transfer.ImageUnpacker); ok { unpacks := iu.UnpackPlatforms() if len(unpacks) > 0 { diff --git a/pkg/transfer/local/pull.go b/pkg/transfer/local/pull.go index b6891f90f..204d33434 100644 --- a/pkg/transfer/local/pull.go +++ b/pkg/transfer/local/pull.go @@ -116,7 +116,7 @@ func (ts *localTransferService) pull(ctx context.Context, ir transfer.ImageFetch return err } - //Set up baseHandlers from service configuration if present or create a new one + // Set up baseHandlers from service configuration if present or create a new one if ts.config.BaseHandlers != nil { baseHandlers = ts.config.BaseHandlers } else { @@ -151,12 +151,12 @@ func (ts *localTransferService) pull(ctx context.Context, ir transfer.ImageFetch )...) // First find suitable platforms to unpack into - //If image storer is also an unpacker type, i.e implemented UnpackPlatforms() func + // If image storer is also an unpacker type, i.e implemented UnpackPlatforms() func if iu, ok := is.(transfer.ImageUnpacker); ok { unpacks := iu.UnpackPlatforms() if len(unpacks) > 0 { uopts := []unpack.UnpackerOpt{} - //Only unpack if requested unpackconfig matches default/supported unpackconfigs + // Only unpack if requested unpackconfig matches default/supported unpackconfigs for _, u := range unpacks { matched, mu := getSupportedPlatform(u, ts.config.UnpackPlatforms) if matched { @@ -254,11 +254,11 @@ func fetchHandler(ingester content.Ingester, fetcher remotes.Fetcher, pt *Progre func getSupportedPlatform(uc transfer.UnpackConfiguration, supportedPlatforms []unpack.Platform) (bool, unpack.Platform) { var u unpack.Platform for _, sp := range supportedPlatforms { - //If both platform and snapshotter match, return the supportPlatform - //If platform matched and SnapshotterKey is empty, we assume client didn't pass SnapshotterKey - //use default Snapshotter + // If both platform and snapshotter match, return the supportPlatform + // If platform matched and SnapshotterKey is empty, we assume client didn't pass SnapshotterKey + // use default Snapshotter if sp.Platform.Match(uc.Platform) { - //assuming sp.SnapshotterKey is not empty + // Assume sp.SnapshotterKey is not empty if uc.Snapshotter == sp.SnapshotterKey { return true, sp } else if uc.Snapshotter == "" && sp.SnapshotterKey == containerd.DefaultSnapshotter { diff --git a/pkg/transfer/local/pull_test.go b/pkg/transfer/local/pull_test.go new file mode 100644 index 000000000..011dddf48 --- /dev/null +++ b/pkg/transfer/local/pull_test.go @@ -0,0 +1,153 @@ +/* + 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 local + +import ( + "testing" + + "github.com/containerd/containerd" + "github.com/containerd/containerd/pkg/transfer" + "github.com/containerd/containerd/pkg/unpack" + "github.com/containerd/containerd/platforms" +) + +func TestGetSupportedPlatform(t *testing.T) { + supportedPlatforms := []unpack.Platform{ + { + Platform: platforms.OnlyStrict(platforms.MustParse("linux/amd64")), + SnapshotterKey: "native", + }, + { + Platform: platforms.OnlyStrict(platforms.MustParse("linux/amd64")), + SnapshotterKey: "devmapper", + }, + { + Platform: platforms.OnlyStrict(platforms.MustParse("linux/arm64")), + SnapshotterKey: "native", + }, + { + Platform: platforms.OnlyStrict(platforms.MustParse("linux/arm")), + SnapshotterKey: "native", + }, + { + Platform: platforms.DefaultStrict(), + SnapshotterKey: containerd.DefaultSnapshotter, + }, + } + + for _, testCase := range []struct { + // Name is the name of the test + Name string + + // Input + UnpackConfig transfer.UnpackConfiguration + SupportedPlatforms []unpack.Platform + + // Expected + Match bool + ExpectedPlatform transfer.UnpackConfiguration + }{ + { + Name: "No match on input linux/arm64 and devmapper snapshotter", + UnpackConfig: transfer.UnpackConfiguration{ + Platform: platforms.MustParse("linux/arm64"), + Snapshotter: "devmapper", + }, + SupportedPlatforms: supportedPlatforms, + Match: false, + ExpectedPlatform: transfer.UnpackConfiguration{}, + }, + { + Name: "No match on input linux/386 and native snapshotter", + UnpackConfig: transfer.UnpackConfiguration{ + Platform: platforms.MustParse("linux/386"), + Snapshotter: "native", + }, + SupportedPlatforms: supportedPlatforms, + Match: false, + ExpectedPlatform: transfer.UnpackConfiguration{}, + }, + { + Name: "Match linux/amd64 and native snapshotter", + UnpackConfig: transfer.UnpackConfiguration{ + Platform: platforms.MustParse("linux/amd64"), + Snapshotter: "native", + }, + SupportedPlatforms: supportedPlatforms, + Match: true, + ExpectedPlatform: transfer.UnpackConfiguration{ + Platform: platforms.MustParse("linux/amd64"), + Snapshotter: "native", + }, + }, + { + Name: "Match linux/arm64 and native snapshotter", + UnpackConfig: transfer.UnpackConfiguration{ + Platform: platforms.MustParse("linux/arm64"), + Snapshotter: "native", + }, + SupportedPlatforms: supportedPlatforms, + Match: true, + ExpectedPlatform: transfer.UnpackConfiguration{ + Platform: platforms.MustParse("linux/arm64"), + Snapshotter: "native", + }, + }, + { + Name: "Default platform input only match with defaultSnapshotter", + UnpackConfig: transfer.UnpackConfiguration{ + Platform: platforms.DefaultSpec(), + }, + SupportedPlatforms: supportedPlatforms, + Match: true, + ExpectedPlatform: transfer.UnpackConfiguration{ + Platform: platforms.DefaultSpec(), + Snapshotter: containerd.DefaultSnapshotter, + }, + }, + } { + testCase := testCase + t.Run(testCase.Name, func(t *testing.T) { + m, sp := getSupportedPlatform(testCase.UnpackConfig, testCase.SupportedPlatforms) + + // Match result should match expected + if m != testCase.Match { + t.Fatalf("Expect match result %v, but got %v", testCase.Match, m) + } + + // If match result is false, the Platform should be nil too + if !m && sp.Platform != nil { + t.Fatalf("Expect nil Platform when we don't have a match") + } + + // Snapshotter should match, empty string can be compared too + if sp.SnapshotterKey != testCase.ExpectedPlatform.Snapshotter { + t.Fatalf("Expect SnapshotterKey %v, but got %v", testCase.ExpectedPlatform.Snapshotter, sp.SnapshotterKey) + } + + // If the matched Platform is not nil, it should match the expected Platform + if sp.Platform != nil && !sp.Platform.Match(testCase.ExpectedPlatform.Platform) { + t.Fatalf("Expect Platform %v doesn't match", testCase.ExpectedPlatform.Platform) + } + // If the ExectedPlatform is not empty, the matched Platform shoule not be nil either + if sp.Platform == nil && testCase.ExpectedPlatform.Platform.OS != "" { + t.Fatalf("Expect Platform %v doesn't match", testCase.ExpectedPlatform.Platform) + } + }) + } + +} diff --git a/pkg/transfer/local/transfer.go b/pkg/transfer/local/transfer.go index 2148ecb97..3345f1a2a 100644 --- a/pkg/transfer/local/transfer.go +++ b/pkg/transfer/local/transfer.go @@ -39,9 +39,9 @@ type localTransferService struct { leases leases.Manager content content.Store images images.Store - //limiter for upload + // limiter for upload limiterU *semaphore.Weighted - //limiter for download operation + // limiter for download operation limiterD *semaphore.Weighted config TransferConfig } @@ -168,7 +168,7 @@ type TransferConfig struct { // handlers. BaseHandlers []images.Handler - //UnpackPlatforms are used to specify supported combination of platforms and snapshotters + // UnpackPlatforms are used to specify supported combination of platforms and snapshotters UnpackPlatforms []unpack.Platform `toml:"unpack_platforms"` // RegistryConfigPath is a path to the root directory containing registry-specific configurations