Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

load: support buildkit archives #853

Merged
merged 1 commit into from
Dec 10, 2021
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
22 changes: 11 additions & 11 deletions libimage/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,6 @@ func (r *Runtime) Load(ctx context.Context, path string, options *LoadOptions) (
var loadErrors []error

for _, f := range []func() ([]string, string, error){
// DOCKER-ARCHIVE - must be first (see containers/podman/issues/10809)
func() ([]string, string, error) {
logrus.Debugf("-> Attempting to load %q as a Docker archive", path)
ref, err := dockerArchiveTransport.ParseReference(path)
if err != nil {
return nil, dockerArchiveTransport.Transport.Name(), err
}
images, err := r.loadMultiImageDockerArchive(ctx, ref, &options.CopyOptions)
return images, dockerArchiveTransport.Transport.Name(), err
},

// OCI
func() ([]string, string, error) {
logrus.Debugf("-> Attempting to load %q as an OCI directory", path)
Expand All @@ -68,6 +57,17 @@ func (r *Runtime) Load(ctx context.Context, path string, options *LoadOptions) (
return images, ociArchiveTransport.Transport.Name(), err
},

// DOCKER-ARCHIVE
func() ([]string, string, error) {
logrus.Debugf("-> Attempting to load %q as a Docker archive", path)
ref, err := dockerArchiveTransport.ParseReference(path)
if err != nil {
return nil, dockerArchiveTransport.Transport.Name(), err
}
images, err := r.loadMultiImageDockerArchive(ctx, ref, &options.CopyOptions)
return images, dockerArchiveTransport.Transport.Name(), err
},

// DIR
func() ([]string, string, error) {
logrus.Debugf("-> Attempting to load %q as a Docker dir", path)
Expand Down
2 changes: 2 additions & 0 deletions libimage/load_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,14 @@ func TestLoad(t *testing.T) {
{"testdata/docker-two-names.tar.xz", false, 1, []string{"localhost/pretty-empty:latest", "example.com/empty:latest"}},
{"testdata/docker-two-images.tar.xz", false, 2, []string{"example.com/empty:latest", "example.com/empty/but:different"}},
{"testdata/docker-unnamed.tar.xz", false, 1, []string{"sha256:ec9293436c2e66da44edb9efb8d41f6b13baf62283ebe846468bc992d76d7951"}},
{"testdata/buildkit-docker.tar", false, 1, []string{"github.com/buildkit/archive:docker"}},

// OCI ARCHIVE
{"testdata/oci-name-only.tar.gz", false, 1, []string{"localhost/pretty-empty:latest"}},
{"testdata/oci-non-docker-name.tar.gz", true, 0, nil},
{"testdata/oci-registry-name.tar.gz", false, 1, []string{"example.com/empty:latest"}},
{"testdata/oci-unnamed.tar.gz", false, 1, []string{"sha256:5c8aca8137ac47e84c69ae93ce650ce967917cc001ba7aad5494073fac75b8b6"}},
{"testdata/buildkit-oci.tar", false, 1, []string{"github.com/buildkit/archive:oci"}},
} {
loadedImages, err := runtime.Load(ctx, test.input, loadOptions)
if test.expectError {
Expand Down
24 changes: 20 additions & 4 deletions libimage/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/containers/image/v5/types"
"github.com/containers/storage"
digest "github.com/opencontainers/go-digest"
ociSpec "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)
Expand Down Expand Up @@ -169,6 +170,20 @@ func (r *Runtime) Pull(ctx context.Context, name string, pullPolicy config.PullP
return localImages, pullError
}

// nameFromAnnotations returns a reference string to be used as an image name,
// or an empty string. The annotations map may be nil.
func nameFromAnnotations(annotations map[string]string) string {
if annotations == nil {
return ""
}
// buildkit/containerd are using a custom annotation see
// containers/podman/issues/12560.
if annotations["io.containerd.image.name"] != "" {
return annotations["io.containerd.image.name"]
}
return annotations[ociSpec.AnnotationRefName]
}

// copyFromDefault is the default copier for a number of transports. Other
// transports require some specific dancing, sometimes Yoga.
func (r *Runtime) copyFromDefault(ctx context.Context, ref types.ImageReference, options *CopyOptions) ([]string, error) {
Expand Down Expand Up @@ -201,15 +216,16 @@ func (r *Runtime) copyFromDefault(ctx context.Context, ref types.ImageReference,
if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Pre-existing: Shouldn’t ociTransport above have exactly the same naming logic?

Copy link
Member Author

Choose a reason for hiding this comment

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

That sounds like a good idea

return nil, err
}
// if index.json has no reference name, compute the image ID instead
if manifestDescriptor.Annotations == nil || manifestDescriptor.Annotations["org.opencontainers.image.ref.name"] == "" {
storageName = nameFromAnnotations(manifestDescriptor.Annotations)
switch len(storageName) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why switch?

(BTW len(s) == 0 and s == "" generate exactly the same code, so I’d lean towards the more readable string comparison.)

Copy link
Member Author

Choose a reason for hiding this comment

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

I have no preference and probably flip-flop between the two depending on the mood

case 0:
// If there's no reference name in the annotations, compute an ID.
storageName, err = getImageID(ctx, ref, nil)
if err != nil {
return nil, err
}
imageName = "sha256:" + storageName[1:]
} else {
storageName = manifestDescriptor.Annotations["org.opencontainers.image.ref.name"]
default:
named, err := NormalizeName(storageName)
if err != nil {
return nil, err
Expand Down
Binary file added libimage/testdata/buildkit-docker.tar
Binary file not shown.
Binary file added libimage/testdata/buildkit-oci.tar
Binary file not shown.