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

Remove the ability to use [name:tag] in podman load command #8877

Merged
merged 2 commits into from
Jan 13, 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
23 changes: 4 additions & 19 deletions cmd/podman/images/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,8 @@ import (
"strings"

"github.com/containers/common/pkg/completion"
"github.com/containers/image/v5/docker/reference"
"github.com/containers/podman/v2/cmd/podman/parse"
"github.com/containers/podman/v2/cmd/podman/registry"
"github.com/containers/podman/v2/cmd/podman/validate"
"github.com/containers/podman/v2/pkg/domain/entities"
"github.com/containers/podman/v2/pkg/util"
"github.com/pkg/errors"
Expand All @@ -22,11 +21,11 @@ import (
var (
loadDescription = "Loads an image from a locally stored archive (tar file) into container storage."
loadCommand = &cobra.Command{
Use: "load [options] [NAME[:TAG]]",
Use: "load [options]",
Short: "Load image(s) from a tar archive",
Long: loadDescription,
RunE: load,
Args: cobra.MaximumNArgs(1),
Args: validate.NoArgs,
ValidArgsFunction: completion.AutocompleteNone,
}

Expand Down Expand Up @@ -71,22 +70,8 @@ func loadFlags(cmd *cobra.Command) {
}

func load(cmd *cobra.Command, args []string) error {
if len(args) > 0 {
ref, err := reference.Parse(args[0])
if err != nil {
return err
}
if t, ok := ref.(reference.Tagged); ok {
loadOpts.Tag = t.Tag()
} else {
loadOpts.Tag = "latest"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn’t the loadOpts.Tag field, and similarly all the way down (other than fixed APIs), be removed then, as well? Otherwise we won’t get rid of the ambiguity throughout the codebase.

}
if r, ok := ref.(reference.Named); ok {
loadOpts.Name = r.Name()
}
}
if len(loadOpts.Input) > 0 {
if err := parse.ValidateFileName(loadOpts.Input); err != nil {
if _, err := os.Stat(loadOpts.Input); err != nil {
return err
}
} else {
Expand Down
9 changes: 4 additions & 5 deletions docs/source/markdown/podman-load.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,12 @@
podman\-load - Load image(s) from a tar archive into container storage

## SYNOPSIS
**podman load** [*options*] [*name*[:*tag*]]
**podman load** [*options*]

**podman image load** [*options*] [*name*[:*tag*]]
**podman image load** [*options*]

## DESCRIPTION
**podman load** loads an image from either an **oci-archive** or a **docker-archive** stored on the local machine into container storage. **podman load** reads from stdin by default or a file if the **input** option is set.
You can also specify a name for the image if the archive is of single image and load will tag an additional image with the name:tag.
**podman load** is used for loading from the archive generated by **podman save**, that includes the image parent layers. To load the archive of container's filesystem created by **podman export**, use **podman import**.

The local client further supports loading an **oci-dir** or a **docker-dir** as created with **podman save** (1).
Expand All @@ -23,7 +22,7 @@ Note: `:` is a restricted character and cannot be part of the file name.

**podman load [GLOBAL OPTIONS]**

**podman load [OPTIONS] NAME[:TAG]**
**podman load [OPTIONS]**

## OPTIONS

Expand Down Expand Up @@ -78,7 +77,7 @@ Loaded image: registry.fedoraproject.org/fedora:latest
```

## SEE ALSO
podman(1), podman-save(1), podman-tag(1)
podman(1), podman-save(1)

## HISTORY
July 2017, Originally compiled by Urvashi Mohnani <[email protected]>
7 changes: 5 additions & 2 deletions libpod/runtime_img.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ func (r *Runtime) LoadImage(ctx context.Context, inputFile string, writer io.Wri
if newImages, err := r.LoadAllImageFromArchive(ctx, writer, inputFile, signaturePolicy); err == nil {
return newImages, nil
}

return r.LoadImageFromSingleImageArchive(ctx, writer, inputFile, signaturePolicy)
}

Expand All @@ -293,7 +294,7 @@ func (r *Runtime) LoadAllImageFromArchive(ctx context.Context, writer io.Writer,

// LoadImageFromSingleImageArchive load image from the archive of single image that inputFile points to.
func (r *Runtime) LoadImageFromSingleImageArchive(ctx context.Context, writer io.Writer, inputFile, signaturePolicy string) (string, error) {
var err error
var saveErr error
for _, referenceFn := range []func() (types.ImageReference, error){
func() (types.ImageReference, error) {
return dockerarchive.ParseReference(inputFile)
Expand All @@ -312,10 +313,12 @@ func (r *Runtime) LoadImageFromSingleImageArchive(ctx context.Context, writer io
if err == nil && src != nil {
if newImages, err := r.ImageRuntime().LoadFromArchiveReference(ctx, src, signaturePolicy, writer); err == nil {
return getImageNames(newImages), nil
} else {
saveErr = err
}
}
}
return "", errors.Wrapf(err, "error pulling image")
return "", errors.Wrapf(saveErr, "error pulling image")
}

func getImageNames(images []*image.Image) string {
Expand Down
15 changes: 1 addition & 14 deletions pkg/api/handlers/libpod/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -353,20 +353,7 @@ func ImagesLoad(w http.ResponseWriter, r *http.Request) {
utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "unable to load image"))
return
}
split := strings.Split(loadedImage, ",")
newImage, err := runtime.ImageRuntime().NewFromLocal(split[0])
if err != nil {
utils.InternalServerError(w, err)
return
}
// TODO this should go into libpod proper at some point.
if len(query.Reference) > 0 {
if err := newImage.TagImage(query.Reference); err != nil {
utils.InternalServerError(w, err)
return
}
}
utils.WriteResponse(w, http.StatusOK, entities.ImageLoadReport{Names: split})
utils.WriteResponse(w, http.StatusOK, entities.ImageLoadReport{Names: strings.Split(loadedImage, ",")})
}

func ImagesImport(w http.ResponseWriter, r *http.Request) {
Expand Down
4 changes: 0 additions & 4 deletions pkg/api/server/register_images.go
Original file line number Diff line number Diff line change
Expand Up @@ -797,10 +797,6 @@ func (s *APIServer) registerImagesHandlers(r *mux.Router) error {
// summary: Load image
// description: Load an image (oci-archive or docker-archive) stream.
// parameters:
// - in: query
// name: reference
// description: "Optional Name[:TAG] for the image"
// type: string
// - in: formData
// name: upload
// description: tarball of container image
Expand Down
11 changes: 2 additions & 9 deletions pkg/bindings/images/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,20 +113,13 @@ func History(ctx context.Context, nameOrID string, options *HistoryOptions) ([]*
return history, response.Process(&history)
}

func Load(ctx context.Context, r io.Reader, options *LoadOptions) (*entities.ImageLoadReport, error) {
if options == nil {
options = new(LoadOptions)
}
func Load(ctx context.Context, r io.Reader) (*entities.ImageLoadReport, error) {
var report entities.ImageLoadReport
conn, err := bindings.GetClient(ctx)
if err != nil {
return nil, err
}
params, err := options.ToParams()
if err != nil {
return nil, err
}
response, err := conn.DoRequest(r, http.MethodPost, "/images/load", params, nil)
response, err := conn.DoRequest(r, http.MethodPost, "/images/load", nil, nil)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/bindings/test/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ func (b *bindingTest) RestoreImagesFromCache() {
}
}
func (b *bindingTest) restoreImageFromCache(i testImage) {
p := b.runPodman([]string{"load", "-i", filepath.Join(ImageCacheDir, i.tarballName), i.name})
p := b.runPodman([]string{"load", "-i", filepath.Join(ImageCacheDir, i.tarballName)})
p.Wait(45)
}

Expand Down
14 changes: 2 additions & 12 deletions pkg/bindings/test/images_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ var _ = Describe("Podman images", func() {
f, err := os.Open(filepath.Join(ImageCacheDir, alpine.tarballName))
defer f.Close()
Expect(err).To(BeNil())
names, err := images.Load(bt.conn, f, nil)
names, err := images.Load(bt.conn, f)
Expect(err).To(BeNil())
Expect(names.Names[0]).To(Equal(alpine.name))
exists, err = images.Exists(bt.conn, alpine.name)
Expand All @@ -234,14 +234,9 @@ var _ = Describe("Podman images", func() {
exists, err = images.Exists(bt.conn, alpine.name)
Expect(err).To(BeNil())
Expect(exists).To(BeFalse())
newName := "quay.io/newname:fizzle"
options := new(images.LoadOptions).WithReference(newName)
names, err = images.Load(bt.conn, f, options)
names, err = images.Load(bt.conn, f)
Expect(err).To(BeNil())
Expect(names.Names[0]).To(Equal(alpine.name))
exists, err = images.Exists(bt.conn, newName)
Expect(err).To(BeNil())
Expect(exists).To(BeTrue())

// load with a bad repo name should trigger a 500
f, err = os.Open(filepath.Join(ImageCacheDir, alpine.tarballName))
Expand All @@ -251,11 +246,6 @@ var _ = Describe("Podman images", func() {
exists, err = images.Exists(bt.conn, alpine.name)
Expect(err).To(BeNil())
Expect(exists).To(BeFalse())
options = new(images.LoadOptions).WithReference("quay.io/newName:fizzle")
_, err = images.Load(bt.conn, f, options)
Expect(err).ToNot(BeNil())
code, _ := bindings.CheckResponseCode(err)
Expect(code).To(BeNumerically("==", http.StatusInternalServerError))
})

It("Export Image", func() {
Expand Down
2 changes: 0 additions & 2 deletions pkg/domain/entities/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,6 @@ type ImageInspectReport struct {
}

type ImageLoadOptions struct {
Name string
Tag string
Input string
Quiet bool
SignaturePolicy string
Expand Down
14 changes: 1 addition & 13 deletions pkg/domain/infra/abi/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -453,19 +453,7 @@ func (ir *ImageEngine) Load(ctx context.Context, opts entities.ImageLoadOptions)
if err != nil {
return nil, err
}
names := strings.Split(name, ",")
if len(names) <= 1 {
newImage, err := ir.Libpod.ImageRuntime().NewFromLocal(name)
if err != nil {
return nil, errors.Wrap(err, "image loaded but no additional tags were created")
}
if len(opts.Name) > 0 {
if err := newImage.TagImage(fmt.Sprintf("%s:%s", opts.Name, opts.Tag)); err != nil {
return nil, errors.Wrapf(err, "error adding %q to image %q", opts.Name, newImage.InputName)
}
}
}
return &entities.ImageLoadReport{Names: names}, nil
return &entities.ImageLoadReport{Names: strings.Split(name, ",")}, nil
}

func (ir *ImageEngine) Import(ctx context.Context, opts entities.ImageImportOptions) (*entities.ImageImportReport, error) {
Expand Down
7 changes: 1 addition & 6 deletions pkg/domain/infra/tunnel/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,12 +215,7 @@ func (ir *ImageEngine) Load(ctx context.Context, opts entities.ImageLoadOptions)
if fInfo.IsDir() {
return nil, errors.Errorf("remote client supports archives only but %q is a directory", opts.Input)
}
ref := opts.Name
if len(opts.Tag) > 0 {
ref += ":" + opts.Tag
}
options := new(images.LoadOptions).WithReference(ref)
return images.Load(ir.ClientCtx, f, options)
return images.Load(ir.ClientCtx, f)
}

func (ir *ImageEngine) Import(ctx context.Context, opts entities.ImageImportOptions) (*entities.ImageImportReport, error) {
Expand Down
6 changes: 3 additions & 3 deletions pkg/systemd/generate/pods_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ WantedBy=multi-user.target default.target
ServiceName: "pod-123abc",
InfraNameOrID: "jadda-jadda-infra",
RestartPolicy: "always",
PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
PIDFile: "/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
StopTimeout: 42,
PodmanVersion: "CI",
RequiredServices: []string{"container-1", "container-2"},
Expand Down Expand Up @@ -204,7 +204,7 @@ WantedBy=multi-user.target default.target
ServiceName: "pod-123abc",
InfraNameOrID: "jadda-jadda-infra",
RestartPolicy: "on-failure",
PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
PIDFile: "/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
StopTimeout: 10,
PodmanVersion: "CI",
RequiredServices: []string{"container-1", "container-2"},
Expand All @@ -220,7 +220,7 @@ WantedBy=multi-user.target default.target
ServiceName: "pod-123abc",
InfraNameOrID: "jadda-jadda-infra",
RestartPolicy: "on-failure",
PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
PIDFile: "/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
StopTimeout: 10,
PodmanVersion: "CI",
RequiredServices: []string{"container-1", "container-2"},
Expand Down
28 changes: 2 additions & 26 deletions test/system/120-load.bats
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,13 @@ verify_iid_and_name() {
local new_tag=t1$(random_string 6 | tr A-Z a-z)
run_podman rmi $fqin

new_fqin=localhost/$new_name:$new_tag
run_podman load -i $archive $new_fqin
run_podman load -i $archive
run_podman images --format '{{.Repository}}:{{.Tag}}' --sort tag
is "${lines[0]}" "$IMAGE" "image is preserved"
is "${lines[1]}" "$fqin" "image is reloaded with old fqin"
is "${lines[2]}" "$new_fqin" "image is reloaded with new fqin too"

# Clean up
run_podman rmi $fqin $new_fqin
run_podman rmi $fqin
}


Expand Down Expand Up @@ -118,28 +116,6 @@ verify_iid_and_name() {
verify_iid_and_name $img_name
}

@test "podman load - NAME and NAME:TAG arguments work" {
get_iid_and_name
run_podman save $iid -o $archive
run_podman rmi $iid

# Load with just a name (note: names must be lower-case)
random_name=$(random_string 20 | tr A-Z a-z)
run_podman load -i $archive $random_name
verify_iid_and_name "localhost/$random_name:latest"

# Load with NAME:TAG arg
run_podman rmi $iid
random_tag=$(random_string 10 | tr A-Z a-z)
run_podman load -i $archive $random_name:$random_tag
verify_iid_and_name "localhost/$random_name:$random_tag"

# Cleanup: restore desired image name
run_podman tag $iid $img_name
run_podman rmi "$random_name:$random_tag"
}


@test "podman load - will not read from tty" {
if [ ! -t 0 ]; then
skip "STDIN is not a tty"
Expand Down