From a6046dceeff21bbeea71c0ab5c3d78ff931aa019 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Mon, 4 Jan 2021 13:31:57 -0500 Subject: [PATCH] Remove the ability to use [name:tag] in podman load command Docker does not support this, and it is confusing what to do if the image has more then one tag. We are dropping support for this in podman 3.0 Fixes: https://github.com/containers/podman/issues/7387 Signed-off-by: Daniel J Walsh --- cmd/podman/images/load.go | 23 ++++------------------ docs/source/markdown/podman-load.1.md | 9 ++++----- libpod/runtime_img.go | 7 +++++-- pkg/api/handlers/libpod/images.go | 15 +------------- pkg/api/server/register_images.go | 4 ---- pkg/bindings/images/images.go | 11 ++--------- pkg/bindings/test/common_test.go | 2 +- pkg/bindings/test/images_test.go | 14 ++------------ pkg/domain/entities/images.go | 2 -- pkg/domain/infra/abi/images.go | 14 +------------- pkg/domain/infra/tunnel/images.go | 7 +------ test/system/120-load.bats | 28 ++------------------------- 12 files changed, 23 insertions(+), 113 deletions(-) diff --git a/cmd/podman/images/load.go b/cmd/podman/images/load.go index a24f467813..59fc6f54c7 100644 --- a/cmd/podman/images/load.go +++ b/cmd/podman/images/load.go @@ -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" @@ -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, } @@ -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" - } - 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 { diff --git a/docs/source/markdown/podman-load.1.md b/docs/source/markdown/podman-load.1.md index dc2a632e57..3ff03e9e79 100644 --- a/docs/source/markdown/podman-load.1.md +++ b/docs/source/markdown/podman-load.1.md @@ -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). @@ -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 @@ -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 diff --git a/libpod/runtime_img.go b/libpod/runtime_img.go index 965333f77e..2c5442bd2d 100644 --- a/libpod/runtime_img.go +++ b/libpod/runtime_img.go @@ -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) } @@ -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) @@ -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 { diff --git a/pkg/api/handlers/libpod/images.go b/pkg/api/handlers/libpod/images.go index b2b93de17d..5b15527b7f 100644 --- a/pkg/api/handlers/libpod/images.go +++ b/pkg/api/handlers/libpod/images.go @@ -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) { diff --git a/pkg/api/server/register_images.go b/pkg/api/server/register_images.go index 7e6de87831..8d0c0800b9 100644 --- a/pkg/api/server/register_images.go +++ b/pkg/api/server/register_images.go @@ -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 diff --git a/pkg/bindings/images/images.go b/pkg/bindings/images/images.go index ecdd1f5532..ae6962c8c8 100644 --- a/pkg/bindings/images/images.go +++ b/pkg/bindings/images/images.go @@ -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 } diff --git a/pkg/bindings/test/common_test.go b/pkg/bindings/test/common_test.go index 232d7136f7..c2b1347d2a 100644 --- a/pkg/bindings/test/common_test.go +++ b/pkg/bindings/test/common_test.go @@ -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) } diff --git a/pkg/bindings/test/images_test.go b/pkg/bindings/test/images_test.go index c6b9c20f90..81959e8133 100644 --- a/pkg/bindings/test/images_test.go +++ b/pkg/bindings/test/images_test.go @@ -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) @@ -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)) @@ -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() { diff --git a/pkg/domain/entities/images.go b/pkg/domain/entities/images.go index d5f88502a4..0805152c34 100644 --- a/pkg/domain/entities/images.go +++ b/pkg/domain/entities/images.go @@ -256,8 +256,6 @@ type ImageInspectReport struct { } type ImageLoadOptions struct { - Name string - Tag string Input string Quiet bool SignaturePolicy string diff --git a/pkg/domain/infra/abi/images.go b/pkg/domain/infra/abi/images.go index 3487dc3f41..1c233d9d5a 100644 --- a/pkg/domain/infra/abi/images.go +++ b/pkg/domain/infra/abi/images.go @@ -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) { diff --git a/pkg/domain/infra/tunnel/images.go b/pkg/domain/infra/tunnel/images.go index fba60235ec..8ab8325991 100644 --- a/pkg/domain/infra/tunnel/images.go +++ b/pkg/domain/infra/tunnel/images.go @@ -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) { diff --git a/test/system/120-load.bats b/test/system/120-load.bats index 272e2ae938..902cd9f5ec 100644 --- a/test/system/120-load.bats +++ b/test/system/120-load.bats @@ -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 } @@ -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"