Skip to content

Commit

Permalink
Enhance default platform comparision for image shareablity #2810
Browse files Browse the repository at this point in the history
Partially Addresses: #2810

There are cases where --platform flag may be passed during nerdctl build. In such a situation if
platform matches the runtime platform the image is shareable.

Signed-off-by: Shubhranshu Mahapatra <[email protected]>
  • Loading branch information
Shubhranshu153 committed Mar 5, 2024
1 parent 0e0fc4d commit f853ab8
Show file tree
Hide file tree
Showing 4 changed files with 278 additions and 1 deletion.
32 changes: 32 additions & 0 deletions cmd/nerdctl/builder_build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"testing"

"github.com/containerd/nerdctl/v2/pkg/testutil"
"github.com/containerd/platforms"
"gotest.tools/v3/assert"
)

Expand Down Expand Up @@ -57,6 +58,37 @@ CMD ["echo", "nerdctl-build-test-string"]
}
}

func TestBuildIsShareableForCompatiblePlatform(t *testing.T) {
testutil.RequiresBuild(t)
base := testutil.NewBase(t)
defer base.Cmd("builder", "prune").Run()
imageName := testutil.Identifier(t)
defer base.Cmd("rmi", imageName).Run()

dockerfile := fmt.Sprintf(`FROM %s
CMD ["echo", "nerdctl-build-test-string"]
`, testutil.CommonImage)

buildCtx, err := createBuildContext(dockerfile)
assert.NilError(t, err)
defer os.RemoveAll(buildCtx)

base.Cmd("build", buildCtx, "-t", imageName).AssertErrNotContains("tarball")

d := platforms.DefaultSpec()
platformConfig := fmt.Sprintf("%s/%s", d.OS, d.Architecture)
base.Cmd("build", buildCtx, "-t", imageName, "--platform", platformConfig).AssertOK()
base.Cmd("build", buildCtx, "-t", imageName, "--platform", platformConfig, "--progress", "plain").AssertErrNotContains("tarball")

n := platforms.Platform{OS: "linux", Architecture: "arm", Variant: ""}
if n.OS != d.OS && n.Architecture != d.Architecture {
notCompatiblePlatformConfig := fmt.Sprintf("%s/%s", n.OS, n.Architecture)
base.Cmd("build", buildCtx, "-t", imageName, "--platform", notCompatiblePlatformConfig).AssertOK()
base.Cmd("build", buildCtx, "-t", imageName, "--platform", notCompatiblePlatformConfig, "--progress", "plain").AssertErrContains("tarball")
}

}

// TestBuildBaseImage tests if an image can be built on the previously built image.
// This isn't currently supported by nerdctl with BuildKit OCI worker.
func TestBuildBaseImage(t *testing.T) {
Expand Down
41 changes: 40 additions & 1 deletion pkg/cmd/builder/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,21 @@ import (
"github.com/containerd/platforms"
)

type PlatformParser interface {
Parse(platform string) (platforms.Platform, error)
DefaultSpec() platforms.Platform
}

type platformParser struct{}

func (p platformParser) Parse(platform string) (platforms.Platform, error) {
return platforms.Parse(platform)
}

func (p platformParser) DefaultSpec() platforms.Platform {
return platforms.DefaultSpec()
}

func Build(ctx context.Context, client *containerd.Client, options types.BuilderBuildOptions) error {
buildctlBinary, buildctlArgs, needsLoading, metaFile, tags, cleanup, err := generateBuildctlArgs(ctx, client, options)
if err != nil {
Expand Down Expand Up @@ -419,6 +434,29 @@ func getDigestFromMetaFile(path string) (string, error) {
return digest, nil
}

func isMatchingRuntimePlatform(platform string, parser PlatformParser) bool {
p, err := parser.Parse(platform)
if err != nil {
return false
}
d := parser.DefaultSpec()

if p.OS == d.OS && p.Architecture == d.Architecture && (p.Variant == "" || p.Variant == d.Variant) {
return true
}

return false
}

func isBuildPlatformDefault(platform []string, parser PlatformParser) bool {
if len(platform) == 0 {
return true
} else if len(platform) == 1 {
return isMatchingRuntimePlatform(platform[0], parser)
}
return false
}

func isImageSharable(buildkitHost, namespace, uuid, snapshotter string, platform []string) (bool, error) {
labels, err := buildkitutil.GetWorkerLabels(buildkitHost)
if err != nil {
Expand All @@ -445,5 +483,6 @@ func isImageSharable(buildkitHost, namespace, uuid, snapshotter string, platform
// Dockerfile doesn't contain instructions require base images like RUN) even if `--output type=image,unpack=true`
// is passed to BuildKit. Thus, we need to use `type=docker` or `type=oci` when nerdctl builds non-default platform
// image using `platform` option.
return executor == "containerd" && containerdUUID == uuid && containerdNamespace == namespace && workerSnapshotter == snapshotter && len(platform) == 0, nil
parser := new(platformParser)
return executor == "containerd" && containerdUUID == uuid && containerdNamespace == namespace && workerSnapshotter == snapshotter && isBuildPlatformDefault(platform, parser), nil
}
189 changes: 189 additions & 0 deletions pkg/cmd/builder/build_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,189 @@
/*
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 builder

import (
"reflect"
"testing"

"github.com/containerd/containerd/platforms"
"go.uber.org/mock/gomock"
"gotest.tools/v3/assert"
)

type MockParse struct {
ctrl *gomock.Controller
recorder *MockParseRecorder
}

type MockParseRecorder struct {
mock *MockParse
}

func newMockParser(ctrl *gomock.Controller) *MockParse {
mock := &MockParse{ctrl: ctrl}
mock.recorder = &MockParseRecorder{mock}
return mock
}

func (m *MockParse) EXPECT() *MockParseRecorder {
return m.recorder
}

func (m *MockParse) Parse(platform string) (platforms.Platform, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "Parse")
ret0, _ := ret[0].(platforms.Platform)
ret1, _ := ret[1].(error)
return ret0, ret1
}

func (m *MockParseRecorder) Parse(platform string) *gomock.Call {
m.mock.ctrl.T.Helper()
return m.mock.ctrl.RecordCallWithMethodType(m.mock, "Parse", reflect.TypeOf((*MockParse)(nil).Parse))
}

func (m *MockParse) DefaultSpec() platforms.Platform {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "DefaultSpec")
ret0, _ := ret[0].(platforms.Platform)
return ret0
}

func (m *MockParseRecorder) DefaultSpec() *gomock.Call {
m.mock.ctrl.T.Helper()
return m.mock.ctrl.RecordCallWithMethodType(m.mock, "DefaultSpec", reflect.TypeOf((*MockParse)(nil).DefaultSpec))
}

func TestIsMatchingRuntimePlatform(t *testing.T) {
t.Parallel()

testCases := []struct {
name string
mock func(*MockParse)
want bool
}{
{
name: "Image is shareable when Runtime and build platform match for os, arch and variant",
mock: func(mockParser *MockParse) {
mockParser.EXPECT().Parse("test").Return(platforms.Platform{OS: "mockOS", Architecture: "mockArch", Variant: "mockVariant"}, nil)
mockParser.EXPECT().DefaultSpec().Return(platforms.Platform{OS: "mockOS", Architecture: "mockArch", Variant: "mockVariant"})
},
want: true,
},
{
name: "Image is shareable when Runtime and build platform match for os, arch. Variant is not defined",
mock: func(mockParser *MockParse) {
mockParser.EXPECT().Parse("test").Return(platforms.Platform{OS: "mockOS", Architecture: "mockArch", Variant: ""}, nil)
mockParser.EXPECT().DefaultSpec().Return(platforms.Platform{OS: "mockOS", Architecture: "mockArch", Variant: "mockVariant"})
},
want: true,
},
{
name: "Image is not shareable when Runtime and build platform donot math OS",
mock: func(mockParser *MockParse) {
mockParser.EXPECT().Parse("test").Return(platforms.Platform{OS: "OS", Architecture: "mockArch", Variant: ""}, nil)
mockParser.EXPECT().DefaultSpec().Return(platforms.Platform{OS: "mockOS", Architecture: "mockArch", Variant: "mockVariant"})
},
want: false,
},
{
name: "Image is not shareable when Runtime and build platform donot math Arch",
mock: func(mockParser *MockParse) {
mockParser.EXPECT().Parse("test").Return(platforms.Platform{OS: "mockOS", Architecture: "Arch", Variant: ""}, nil)
mockParser.EXPECT().DefaultSpec().Return(platforms.Platform{OS: "mockOS", Architecture: "mockArch", Variant: "mockVariant"})
},
want: false,
},
{
name: "Image is not shareable when Runtime and build platform donot math Variant",
mock: func(mockParser *MockParse) {
mockParser.EXPECT().Parse("test").Return(platforms.Platform{OS: "mockOS", Architecture: "mockArch", Variant: "Variant"}, nil)
mockParser.EXPECT().DefaultSpec().Return(platforms.Platform{OS: "mockOS", Architecture: "mockArch", Variant: "mockVariant"})
},
want: false,
},
}

for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

ctrl := gomock.NewController(t)
mockParser := newMockParser(ctrl)
tc.mock(mockParser)
r := isMatchingRuntimePlatform("test", mockParser)
assert.Equal(t, r, tc.want, tc.name)
})
}
}

func TestIsBuildPlatformDefault(t *testing.T) {
t.Parallel()

testCases := []struct {
name string
mock func(*MockParse)
platform []string
want bool
}{
{
name: "Image is shreable when len of platform is 0",
platform: make([]string, 0),
want: true,
},
{
name: "Image is shareable when Runtime and build platform match for os, arch and variant",
platform: []string{"test"},
mock: func(mockParser *MockParse) {
mockParser.EXPECT().Parse("test").Return(platforms.Platform{OS: "mockOS", Architecture: "mockArch", Variant: "mockVariant"}, nil)
mockParser.EXPECT().DefaultSpec().Return(platforms.Platform{OS: "mockOS", Architecture: "mockArch", Variant: "mockVariant"})
},
want: true,
},
{
name: "Image is not shareable when Runtime build platform dont match",
platform: []string{"test"},
mock: func(mockParser *MockParse) {
mockParser.EXPECT().Parse("test").Return(platforms.Platform{OS: "OS", Architecture: "mockArch", Variant: "mockVariant"}, nil)
mockParser.EXPECT().DefaultSpec().Return(platforms.Platform{OS: "mockOS", Architecture: "mockArch", Variant: "mockVariant"})
},
want: false,
},
{
name: "Image is not shareable when more than 2 platforms are passed",
platform: []string{"test1", "test2"},
want: false,
},
}

for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

ctrl := gomock.NewController(t)
mockParser := newMockParser(ctrl)
if len(tc.platform) == 1 {
tc.mock(mockParser)
}
r := isBuildPlatformDefault(tc.platform, mockParser)
assert.Equal(t, r, tc.want, tc.name)
})
}
}
17 changes: 17 additions & 0 deletions pkg/testutil/testutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,14 @@ func (c *Cmd) AssertOutContains(s string) {
c.Assert(expected)
}

func (c *Cmd) AssertErrContains(s string) {
c.Base.T.Helper()
expected := icmd.Expected{
Err: s,
}
c.Assert(expected)
}

func (c *Cmd) AssertCombinedOutContains(s string) {
c.Base.T.Helper()
res := c.Run()
Expand Down Expand Up @@ -430,6 +438,15 @@ func (c *Cmd) AssertOutNotContains(s string) {
})
}

func (c *Cmd) AssertErrNotContains(s string) {
c.AssertOutWithFunc(func(stderr string) error {
if strings.Contains(stderr, s) {
return fmt.Errorf("expected stdout to not contain %q", s)
}
return nil
})
}

func (c *Cmd) AssertOutExactly(s string) {
c.Base.T.Helper()
fn := func(stdout string) error {
Expand Down

0 comments on commit f853ab8

Please sign in to comment.