Skip to content

Commit

Permalink
podman volume export/import: give better error
Browse files Browse the repository at this point in the history
When the volume does not exist we should output an error stating so and
not some generic one.

Fixes containers#14411

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
  • Loading branch information
Luap99 committed May 30, 2022
1 parent a550af2 commit ec576a5
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 10 deletions.
6 changes: 5 additions & 1 deletion cmd/podman/volumes/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/containers/podman/v4/cmd/podman/common"
"github.com/containers/podman/v4/cmd/podman/registry"
"github.com/containers/podman/v4/pkg/domain/entities"
"github.com/containers/podman/v4/pkg/errorhandling"
"github.com/containers/podman/v4/utils"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -58,10 +59,13 @@ func export(cmd *cobra.Command, args []string) error {
return errors.New("expects output path, use --output=[path]")
}
inspectOpts.Type = common.VolumeType
volumeData, _, err := containerEngine.VolumeInspect(ctx, args, inspectOpts)
volumeData, errs, err := containerEngine.VolumeInspect(ctx, args, inspectOpts)
if err != nil {
return err
}
if len(errs) > 0 {
return errorhandling.JoinErrors(errs)
}
if len(volumeData) < 1 {
return errors.New("no volume data found")
}
Expand Down
7 changes: 6 additions & 1 deletion cmd/podman/volumes/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/containers/podman/v4/cmd/podman/parse"
"github.com/containers/podman/v4/cmd/podman/registry"
"github.com/containers/podman/v4/pkg/domain/entities"
"github.com/containers/podman/v4/pkg/errorhandling"
"github.com/containers/podman/v4/utils"
"github.com/pkg/errors"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -60,10 +61,14 @@ func importVol(cmd *cobra.Command, args []string) error {
}

inspectOpts.Type = common.VolumeType
volumeData, _, err := containerEngine.VolumeInspect(ctx, volumes, inspectOpts)
inspectOpts.Type = common.VolumeType
volumeData, errs, err := containerEngine.VolumeInspect(ctx, volumes, inspectOpts)
if err != nil {
return err
}
if len(errs) > 0 {
return errorhandling.JoinErrors(errs)
}
if len(volumeData) < 1 {
return errors.New("no volume data found")
}
Expand Down
6 changes: 3 additions & 3 deletions docs/source/markdown/podman-volume-import.1.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
% podman-volume-import(1)

## NAME
podman\-volume\-import - Import tarball contents into a podman volume
podman\-volume\-import - Import tarball contents into an existing podman volume

## SYNOPSIS
**podman volume import** *volume* [*source*]
Expand All @@ -11,9 +11,9 @@ podman\-volume\-import - Import tarball contents into a podman volume
**podman volume import** imports the contents of a tarball into the podman volume's mount point.
**podman volume import** can consume piped input when using `-` as source path.

Note: Following command is not supported by podman-remote.
The given volume must already exist and will not be created by podman volume import.

**podman volume import VOLUME [SOURCE]**
Note: Following command is not supported by podman-remote.

#### **--help**

Expand Down
2 changes: 1 addition & 1 deletion docs/source/markdown/podman-volume.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ podman volume is a set of subcommands that manage volumes.
| create | [podman-volume-create(1)](podman-volume-create.1.md) | Create a new volume. |
| exists | [podman-volume-exists(1)](podman-volume-exists.1.md) | Check if the given volume exists. |
| export | [podman-volume-export(1)](podman-volume-export.1.md) | Exports volume to external tar. |
| import | [podman-volume-import(1)](podman-volume-import.1.md) | Import tarball contents into a podman volume. |
| import | [podman-volume-import(1)](podman-volume-import.1.md) | Import tarball contents into an existing podman volume. |
| inspect | [podman-volume-inspect(1)](podman-volume-inspect.1.md) | Get detailed information on one or more volumes. |
| ls | [podman-volume-ls(1)](podman-volume-ls.1.md) | List all the available volumes. |
| mount | [podman-volume-mount(1)](podman-volume-mount.1.md) | Mount a volume filesystem. |
Expand Down
17 changes: 13 additions & 4 deletions test/e2e/volume_create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,15 +110,24 @@ var _ = Describe("Podman volume create", func() {
Expect(session.OutputToString()).To(ContainSubstring("hello"))
})

It("podman import volume should fail", func() {
It("podman import/export volume should fail", func() {
// try import on volume or source which does not exists
if podmanTest.RemoteTest {
Skip("Volume export check does not work with a remote client")
}
SkipIfRemote("Volume export check does not work with a remote client")

session := podmanTest.Podman([]string{"volume", "import", "notfound", "notfound.tar"})
session.WaitWithDefaultTimeout()
Expect(session).To(ExitWithError())
Expect(session.ErrorToString()).To(ContainSubstring("open notfound.tar: no such file or directory"))

session = podmanTest.Podman([]string{"volume", "import", "notfound", "-"})
session.WaitWithDefaultTimeout()
Expect(session).To(ExitWithError())
Expect(session.ErrorToString()).To(ContainSubstring("no such volume notfound"))

session = podmanTest.Podman([]string{"volume", "export", "notfound"})
session.WaitWithDefaultTimeout()
Expect(session).To(ExitWithError())
Expect(session.ErrorToString()).To(ContainSubstring("no such volume notfound"))
})

It("podman create volume with bad volume option", func() {
Expand Down

2 comments on commit ec576a5

@benblasco
Copy link

Choose a reason for hiding this comment

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

In test/e2e/volume_create_test.go (new) line 125 and line 130 the string "no such volume notfound" doesn't make sense to me unless I have misunderstood something. It could instead say "no such volume found".

@Luap99
Copy link
Owner Author

@Luap99 Luap99 commented on ec576a5 May 31, 2022

Choose a reason for hiding this comment

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

notfound is the name, the error is no such volume <NAME>

Please sign in to comment.