Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,32 @@ jobs:
uses: codecov/codecov-action@v4
with:
token: ${{ secrets.CODECOV_TOKEN }}
test-and-build-linux-containerd:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Set up Docker with Containerd
uses: docker/setup-docker-action@v5
with:
set-host: true
daemon-config: |
{
"features": {
"containerd-snapshotter": true
}
}
- name: Set up go
uses: actions/setup-go@v5
with:
go-version-file: 'go.mod'
- name: Test
run: make test
env:
PLATFORM_AWARE_DOCKER: 'true'
- name: Upload Coverage to Codecov
uses: codecov/codecov-action@v4
with:
token: ${{ secrets.CODECOV_TOKEN }}
test-and-build-windows:
runs-on: windows-2022
steps:
Expand Down
57 changes: 57 additions & 0 deletions local/local_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/google/go-containerregistry/pkg/authn"
v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/moby/moby/client"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/sclevine/spec"
"github.com/sclevine/spec/report"

Expand Down Expand Up @@ -291,6 +292,62 @@ func testImage(t *testing.T, when spec.G, it spec.S) {
})
})

platformAwareWhen := when.Pend
if h.DockerIsPlatformAware() {
platformAwareWhen = when
}
platformAwareWhen("base image has multiple platforms available", func() {
it("uses the matching platform", func() {
// linux/arm64 busybox image
multiplatformBaseImageName := "busybox"
expectedArchitecture := "arm64"
expectedOSVersion := ""
distractingArchitecture := "amd64"
distractingOSVersion := ""
if daemonOS == "windows" {
// windows/arm nanoserver image
multiplatformBaseImageName = "mcr.microsoft.com/windows/nanoserver:10.0.17763.1040"
expectedArchitecture = "arm"
expectedOSVersion = "10.0.17763.1040"
distractingArchitecture = "amd64"
distractingOSVersion = "10.0.17763.1040"
}

h.PullWithPlatformIfMissing(t, dockerClient, multiplatformBaseImageName, ocispec.Platform{
OS: daemonOS,
Architecture: expectedArchitecture,
OSVersion: expectedOSVersion,
})
h.PullWithPlatformIfMissing(t, dockerClient, multiplatformBaseImageName, ocispec.Platform{
OS: daemonOS,
Architecture: distractingArchitecture,
OSVersion: distractingOSVersion,
})

img, err := local.NewImage(
newTestImageName(),
dockerClient,
local.FromBaseImage(multiplatformBaseImageName),
local.WithDefaultPlatform(imgutil.Platform{
OS: daemonOS,
Architecture: expectedArchitecture,
OSVersion: expectedOSVersion,
}),
)
h.AssertNil(t, err)
h.AssertNil(t, img.Save())
defer h.DockerRmi(dockerClient, img.Name())

imgArch, err := img.Architecture()
h.AssertNil(t, err)
h.AssertEq(t, imgArch, expectedArchitecture)

imgOSVersion, err := img.OSVersion()
h.AssertNil(t, err)
h.AssertEq(t, imgOSVersion, expectedOSVersion)
})
})

when("base image does not exist", func() {
it("returns an empty image based on platform fields", func() {
img, err := local.NewImage(
Expand Down
106 changes: 87 additions & 19 deletions local/new.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@ package local
import (
"context"
"fmt"
"runtime"

cerrdefs "github.com/containerd/errdefs"
v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/moby/moby/api/types/image"
"github.com/moby/moby/client"
"github.com/moby/moby/client/pkg/versions"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"

"github.com/buildpacks/imgutil"
)
Expand All @@ -21,12 +24,13 @@ func NewImage(repoName string, dockerClient DockerClient, ops ...imgutil.ImageOp
}

var err error
options.Platform, err = processPlatformOption(options.Platform, dockerClient)
var isPlatformAware bool
options.Platform, isPlatformAware, err = processPlatformOption(options.Platform, dockerClient)
if err != nil {
return nil, err
}

previousImage, err := processImageOption(options.PreviousImageRepoName, dockerClient, true)
previousImage, err := processImageOption(options.PreviousImageRepoName, isPlatformAware, options.Platform, dockerClient, true)
if err != nil {
return nil, err
}
Expand All @@ -38,7 +42,7 @@ func NewImage(repoName string, dockerClient DockerClient, ops ...imgutil.ImageOp
baseIdentifier string
store *Store
)
baseImage, err := processImageOption(options.BaseImageRepoName, dockerClient, false)
baseImage, err := processImageOption(options.BaseImageRepoName, isPlatformAware, options.Platform, dockerClient, false)
if err != nil {
return nil, err
}
Expand All @@ -47,7 +51,11 @@ func NewImage(repoName string, dockerClient DockerClient, ops ...imgutil.ImageOp
baseIdentifier = baseImage.identifier
store = baseImage.layerStore
} else {
store = NewStore(dockerClient)
if isPlatformAware {
store = NewStoreWithPlatform(dockerClient, options.Platform)
} else {
store = NewStore(dockerClient)
}
}

cnbImage, err := imgutil.NewCNBImage(*options)
Expand All @@ -64,30 +72,35 @@ func NewImage(repoName string, dockerClient DockerClient, ops ...imgutil.ImageOp
}, nil
}

func defaultPlatform(dockerClient DockerClient) (imgutil.Platform, error) {
func defaultPlatform(dockerClient DockerClient) (imgutil.Platform, bool, error) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks good - do you think we can get some tests with mocks? The new platform aware paths look right but tests mocking out the api version to confirm the called methods would make me feel a bit better.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up adding a test job that uses containerd in the workflow, plus a test which only gets called in that job and should exercise the platform-aware code paths.

daemonInfo, err := dockerClient.ServerVersion(context.Background(), client.ServerVersionOptions{})
if err != nil {
return imgutil.Platform{}, err
return imgutil.Platform{}, false, err
}
isPlatformAware := versions.GreaterThanOrEqualTo(daemonInfo.APIVersion, "1.49")
// When running on a different architecture than the daemon, we want to use images matching our own architecture
// https://github.com/buildpacks/lifecycle/issues/1599
if isPlatformAware {
return imgutil.Platform{
OS: runtime.GOOS,
Architecture: runtime.GOARCH,
}, isPlatformAware, nil
}
return imgutil.Platform{
OS: daemonInfo.Os,
Architecture: daemonInfo.Arch,
}, nil
}, isPlatformAware, nil
}

func processPlatformOption(requestedPlatform imgutil.Platform, dockerClient DockerClient) (imgutil.Platform, error) {
dockerPlatform, err := defaultPlatform(dockerClient)
func processPlatformOption(requestedPlatform imgutil.Platform, dockerClient DockerClient) (imgutil.Platform, bool, error) {
defaultPlatform, isPlatformAware, err := defaultPlatform(dockerClient)
if err != nil {
return imgutil.Platform{}, err
return imgutil.Platform{}, false, err
}
if (requestedPlatform == imgutil.Platform{}) {
return dockerPlatform, nil
}
if requestedPlatform.OS != "" && requestedPlatform.OS != dockerPlatform.OS {
return imgutil.Platform{},
fmt.Errorf("invalid os: platform os %q must match the daemon os %q", requestedPlatform.OS, dockerPlatform.OS)
return defaultPlatform, isPlatformAware, nil
}
return requestedPlatform, nil
return requestedPlatform, isPlatformAware, nil
}

type imageResult struct {
Expand All @@ -96,25 +109,52 @@ type imageResult struct {
layerStore *Store
}

func processImageOption(repoName string, dockerClient DockerClient, downloadLayersOnAccess bool) (imageResult, error) {
func processImageOption(repoName string, isPlatformAware bool, platform imgutil.Platform, dockerClient DockerClient, downloadLayersOnAccess bool) (imageResult, error) {
if repoName == "" {
return imageResult{}, nil
}
inspect, history, err := getInspectAndHistory(repoName, dockerClient)
if err != nil {
return imageResult{}, err
}

if inspect == nil {
return imageResult{}, nil
}
layerStore := NewStore(dockerClient)

// Always use the platform-unaware image ID
identifier := inspect.ID

// Try using the platform-specific inspected value if possible, otherwise fall back to the generic inspect
if isPlatformAware {
platformInspect, platformHistory, err := getPlatformAwareInspectAndHistory(repoName, platform, dockerClient)
if err != nil {
return imageResult{}, err
}
if platformInspect != nil && platformHistory != nil {
inspect = platformInspect
history = platformHistory
}
}

var layerStore *Store
if isPlatformAware {
layerStore = NewStoreWithPlatform(dockerClient, imgutil.Platform{
Architecture: inspect.Architecture,
OS: inspect.Os,
OSVersion: inspect.OsVersion,
Variant: inspect.Variant,
})
} else {
layerStore = NewStore(dockerClient)
}
v1Image, err := newV1ImageFacadeFromInspect(*inspect, history, layerStore, downloadLayersOnAccess)
if err != nil {
return imageResult{}, err
}
return imageResult{
image: v1Image,
identifier: inspect.ID,
identifier: identifier,
layerStore: layerStore,
}, nil
}
Expand All @@ -131,5 +171,33 @@ func getInspectAndHistory(repoName string, dockerClient DockerClient) (*image.In
if err != nil {
return nil, nil, fmt.Errorf("get history for image %q: %w", repoName, err)
}

return &inspect.InspectResponse, historyResult.Items, nil
}

func getPlatformAwareInspectAndHistory(repoName string, platform imgutil.Platform, dockerClient DockerClient) (*image.InspectResponse, []image.HistoryResponseItem, error) {
ociPlatform := ocispec.Platform{
Architecture: platform.Architecture,
OS: platform.OS,
OSVersion: platform.OSVersion,
Variant: platform.Variant,
}

platformHistoryResult, err := dockerClient.ImageHistory(context.Background(), repoName, client.ImageHistoryWithPlatform(ociPlatform))
if err != nil {
if cerrdefs.IsNotFound(err) {
return nil, nil, nil
}
return nil, nil, fmt.Errorf("get history for image %q: %w", repoName, err)
}

platformInspect, err := dockerClient.ImageInspect(context.Background(), repoName, client.ImageInspectWithPlatform(&ociPlatform))
if err != nil {
if cerrdefs.IsNotFound(err) {
return nil, nil, nil
}
return nil, nil, fmt.Errorf("inspecting platform-specific image %q: %w", repoName, err)
}

return &platformInspect.InspectResponse, platformHistoryResult.Items, nil
}
28 changes: 27 additions & 1 deletion local/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/moby/moby/api/types/image"
"github.com/moby/moby/api/types/jsonstream"
"github.com/moby/moby/client"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"golang.org/x/sync/errgroup"

"github.com/buildpacks/imgutil"
Expand All @@ -33,6 +34,7 @@ type Store struct {
// optional
downloadOnce *sync.Once
onDiskLayersByDiffID map[v1.Hash]annotatedLayer
platform imgutil.Platform
}

// DockerClient is subset of client.APIClient required by this package.
Expand Down Expand Up @@ -60,6 +62,15 @@ func NewStore(dockerClient DockerClient) *Store {
}
}

func NewStoreWithPlatform(dockerClient DockerClient, platform imgutil.Platform) *Store {
return &Store{
dockerClient: dockerClient,
downloadOnce: &sync.Once{},
onDiskLayersByDiffID: make(map[v1.Hash]annotatedLayer),
platform: platform,
}
}

// images

func (s *Store) Contains(identifier string) bool {
Expand Down Expand Up @@ -408,7 +419,22 @@ func (s *Store) doDownloadLayersFor(identifier string) error {
}
ctx := context.Background()

imageReader, err := s.dockerClient.ImageSave(ctx, []string{identifier})
var imageReader client.ImageSaveResult
var err error

if s.platform != (imgutil.Platform{}) {
// Download right platform on platform-aware Docker
ociPlatform := ocispec.Platform{
Architecture: s.platform.Architecture,
OS: s.platform.OS,
OSVersion: s.platform.OSVersion,
Variant: s.platform.Variant,
}
imageReader, err = s.dockerClient.ImageSave(ctx, []string{identifier}, client.ImageSaveWithPlatforms(ociPlatform))
} else {
imageReader, err = s.dockerClient.ImageSave(ctx, []string{identifier})
}

if err != nil {
return fmt.Errorf("saving image with ID %q from the docker daemon: %w", identifier, err)
}
Expand Down
6 changes: 3 additions & 3 deletions local/v1_facade.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@ func newV1ImageFacadeFromInspect(dockerInspect image.InspectResponse, history []
return nil, err
}
configFile := &v1.ConfigFile{
Architecture: dockerInspect.Architecture, // FIXME: this should come from options.Platform
Architecture: dockerInspect.Architecture,
Author: dockerInspect.Author,
Created: toV1Time(dockerInspect.Created),
History: imgutil.NormalizedHistory(toV1History(history), len(dockerInspect.RootFS.Layers)),
OS: dockerInspect.Os,
RootFS: rootFS,
Config: toV1Config(dockerInspect.Config),
OSVersion: dockerInspect.OsVersion, // FIXME: this should come from options.Platform
Variant: dockerInspect.Variant, // FIXME: this should come from options.Platform
OSVersion: dockerInspect.OsVersion,
Variant: dockerInspect.Variant,
}
layersToSet := newEmptyLayerListFrom(configFile, downloadLayersOnAccess, withStore, dockerInspect.ID)
return imageFrom(layersToSet, configFile, imgutil.DockerTypes) // FIXME: this should be configurable with options.MediaTypes
Expand Down
Loading
Loading