Refactor volume import to support the remote client

As with `volume export`, this was coded up exclusively in cmd/
instead of in libpod. Move it into Libpod, add a REST endpoint,
add bindings, and now everything talks using the ContainerEngine
wiring.

Also similar to `volume export` this also makes things work much
better with volumes that require mounting - we can now guarantee
they're actually mounted, instead of just hoping.

Includes some refactoring of `volume export` as well, to simplify
its implementation and ensure both Import and Export work with
readers/writers, as opposed to just files.

Fixes #26409

Signed-off-by: Matt Heon <mheon@redhat.com>

cmd/podman/volumes/export.go

@@ -3,6 +3,8 @@ package volumes
 import (
 	"context"
 	"errors"
+	"fmt"
+	"os"
 
 	"github.com/containers/common/pkg/completion"
 	"github.com/containers/podman/v5/cmd/podman/common"
@@ -27,7 +29,7 @@ Allow content of volume to be exported into external tar.`
 )
 
 var (
-	cliExportOpts entities.VolumeExportOptions
+	targetPath string
 )
 
 func init() {
@@ -38,7 +40,7 @@ func init() {
 	flags := exportCommand.Flags()
 
 	outputFlagName := "output"
-	flags.StringVarP(&cliExportOpts.OutputPath, outputFlagName, "o", "/dev/stdout", "Write to a specified file (default: stdout, which must be redirected)")
+	flags.StringVarP(&targetPath, outputFlagName, "o", "", "Write to a specified file (default: stdout, which must be redirected)")
 	_ = exportCommand.RegisterFlagCompletionFunc(outputFlagName, completion.AutocompleteDefault)
 }
 
@@ -46,12 +48,22 @@ func export(cmd *cobra.Command, args []string) error {
 	containerEngine := registry.ContainerEngine()
 	ctx := context.Background()
 
-	if cliExportOpts.OutputPath == "" {
+	if targetPath == "" && cmd.Flag("output").Changed {
-		return errors.New("expects output path, use --output=[path]")
+		return errors.New("must provide valid path for file to write to")
 	}
 
-	if err := containerEngine.VolumeExport(ctx, args[0], cliExportOpts); err != nil {
+	exportOpts := entities.VolumeExportOptions{}
-		return err
+
+	if targetPath != "" {
+		targetFile, err := os.Create(targetPath)
+		if err != nil {
+			return fmt.Errorf("unable to create target file path %q: %w", targetPath, err)
 		}
-	return nil
+		defer targetFile.Close()
+		exportOpts.Output = targetFile
+	} else {
+		exportOpts.Output = os.Stdout
+	}
+
+	return containerEngine.VolumeExport(ctx, args[0], exportOpts)
 }

cmd/podman/volumes/import.go

@@ -1,7 +1,7 @@
 package volumes
 
 import (
-	"errors"
+	"context"
 	"fmt"
 	"os"
 
@@ -9,15 +9,12 @@ import (
 	"github.com/containers/podman/v5/cmd/podman/parse"
 	"github.com/containers/podman/v5/cmd/podman/registry"
 	"github.com/containers/podman/v5/pkg/domain/entities"
-	"github.com/containers/podman/v5/pkg/errorhandling"
-	"github.com/containers/podman/v5/utils"
 	"github.com/spf13/cobra"
 )
 
 var (
 	importDescription = `Imports contents into a podman volume from specified tarball (.tar, .tar.gz, .tgz, .bzip, .tar.xz, .txz).`
 	importCommand     = &cobra.Command{
-		Annotations:       map[string]string{registry.EngineMode: registry.ABIMode},
 		Use:               "import VOLUME [SOURCE]",
 		Short:             "Import a tarball contents into a podman volume",
 		Long:              importDescription,
@@ -37,65 +34,26 @@ func init() {
 }
 
 func importVol(cmd *cobra.Command, args []string) error {
-	var inspectOpts entities.InspectOptions
+	opts := entities.VolumeImportOptions{}
-	var tarFile *os.File
-	containerEngine := registry.ContainerEngine()
-	ctx := registry.Context()
-	// create a slice of volumes since inspect expects slice as arg
-	volumes := []string{args[0]}
-	tarPath := args[1]
 
-	if tarPath != "-" {
+	filepath := args[1]
-		err := parse.ValidateFileName(tarPath)
+	if filepath == "-" {
-		if err != nil {
+		opts.Input = os.Stdin
-			return err
-		}
-
-		// open tar file
-		tarFile, err = os.Open(tarPath)
-		if err != nil {
-			return err
-		}
 	} else {
-		tarFile = os.Stdin
+		if err := parse.ValidateFileName(filepath); err != nil {
+			return err
 		}
 
-	inspectOpts.Type = common.VolumeType
+		targetFile, err := os.Open(filepath)
-	inspectOpts.Type = common.VolumeType
-	volumeData, errs, err := containerEngine.VolumeInspect(ctx, volumes, inspectOpts)
 		if err != nil {
-		return err
+			return fmt.Errorf("unable open input file: %w", err)
 		}
-	if len(errs) > 0 {
+		defer targetFile.Close()
-		return errorhandling.JoinErrors(errs)
+		opts.Input = targetFile
 	}
-	if len(volumeData) < 1 {
+
-		return errors.New("no volume data found")
+	containerEngine := registry.ContainerEngine()
-	}
+	ctx := context.Background()
-	mountPoint := volumeData[0].VolumeConfigResponse.Mountpoint
+
-	driver := volumeData[0].VolumeConfigResponse.Driver
+	return containerEngine.VolumeImport(ctx, args[0], opts)
-	volumeOptions := volumeData[0].VolumeConfigResponse.Options
-	volumeMountStatus, err := containerEngine.VolumeMounted(ctx, args[0])
-	if err != nil {
-		return err
-	}
-	if mountPoint == "" {
-		return errors.New("volume is not mounted anywhere on host")
-	}
-	// Check if volume is using external plugin and export only if volume is mounted
-	if driver != "" && driver != "local" {
-		if !volumeMountStatus.Value {
-			return fmt.Errorf("volume is using a driver %s and volume is not mounted on %s", driver, mountPoint)
-		}
-	}
-	// Check if volume is using `local` driver and has mount options type other than tmpfs
-	if driver == "local" {
-		if mountOptionType, ok := volumeOptions["type"]; ok {
-			if mountOptionType != "tmpfs" && !volumeMountStatus.Value {
-				return fmt.Errorf("volume is using a driver %s and volume is not mounted on %s", driver, mountPoint)
-			}
-		}
-	}
-	// dont care if volume is mounted or not we are gonna import everything to mountPoint
-	return utils.UntarToFileSystem(mountPoint, tarFile, nil)
 }

docs/source/markdown/podman-volume-export.1.md

@@ -12,8 +12,6 @@ podman\-volume\-export - Export volume to external tar
 on the local machine. **podman volume export** writes to STDOUT by default and can be
 redirected to a file using the `--output` flag.
 
-Note: Following command is not supported by podman-remote.
-
 **podman volume export [OPTIONS] VOLUME**
 
 ## OPTIONS

docs/source/markdown/podman-volume-import.1.md

@@ -14,8 +14,6 @@ The contents of the volume is merged with the content of the tarball with the la
 
 The given volume must already exist and is not created by podman volume import.
 
-Note: Following command is not supported by podman-remote.
-
 #### **--help**
 
 Print usage statement

libpod/volume.go

@@ -11,6 +11,7 @@ import (
 	"github.com/containers/podman/v5/libpod/lock"
 	"github.com/containers/podman/v5/libpod/plugin"
 	"github.com/containers/podman/v5/utils"
+	"github.com/containers/storage/pkg/archive"
 	"github.com/containers/storage/pkg/directory"
 	"github.com/sirupsen/logrus"
 )
@@ -299,10 +300,12 @@ func (v *Volume) NeedsMount() bool {
 	return v.needsMount()
 }
 
+// Export volume to tar.
 // Returns a ReadCloser which points to a tar of all the volume's contents.
-func (v *Volume) ExportVolume() (io.ReadCloser, error) {
+func (v *Volume) Export() (io.ReadCloser, error) {
 	v.lock.Lock()
 	err := v.mount()
+	mountPoint := v.mountPoint()
 	v.lock.Unlock()
 	if err != nil {
 		return nil, err
@@ -316,10 +319,35 @@ func (v *Volume) ExportVolume() (io.ReadCloser, error) {
 		}
 	}()
 
-	volContents, err := utils.TarWithChroot(v.mountPoint())
+	volContents, err := utils.TarWithChroot(mountPoint)
 	if err != nil {
 		return nil, fmt.Errorf("creating tar of volume %s contents: %w", v.Name(), err)
 	}
 
 	return volContents, nil
 }
+
+// Import a volume from a tar file, provided as an io.Reader.
+func (v *Volume) Import(r io.Reader) error {
+	v.lock.Lock()
+	err := v.mount()
+	mountPoint := v.mountPoint()
+	v.lock.Unlock()
+	if err != nil {
+		return err
+	}
+	defer func() {
+		v.lock.Lock()
+		defer v.lock.Unlock()
+
+		if err := v.unmount(false); err != nil {
+			logrus.Errorf("Error unmounting volume %s: %v", v.Name(), err)
+		}
+	}()
+
+	if err := archive.Untar(r, mountPoint, nil); err != nil {
+		return fmt.Errorf("extracting into volume %s: %w", v.Name(), err)
+	}
+
+	return nil
+}

pkg/api/handlers/libpod/volumes.go

@@ -4,12 +4,11 @@ package libpod
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/url"
 
-	"errors"
-
 	"github.com/containers/podman/v5/libpod"
 	"github.com/containers/podman/v5/libpod/define"
 	"github.com/containers/podman/v5/pkg/api/handlers/utils"
@@ -223,14 +222,39 @@ func ExportVolume(w http.ResponseWriter, r *http.Request) {
 
 	vol, err := runtime.GetVolume(name)
 	if err != nil {
-		utils.Error(w, http.StatusNotFound, err)
+		utils.VolumeNotFound(w, name, err)
 		return
 	}
 
-	contents, err := vol.ExportVolume()
+	contents, err := vol.Export()
 	if err != nil {
 		utils.Error(w, http.StatusInternalServerError, err)
 		return
 	}
 	utils.WriteResponse(w, http.StatusOK, contents)
 }
+
+// ImportVolume imports a volume
+func ImportVolume(w http.ResponseWriter, r *http.Request) {
+	runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime)
+	name := utils.GetName(r)
+
+	vol, err := runtime.GetVolume(name)
+	if err != nil {
+		utils.VolumeNotFound(w, name, err)
+		return
+	}
+
+	if r.Body == nil {
+		utils.Error(w, http.StatusInternalServerError, errors.New("must provide tar file to import in request body"))
+		return
+	}
+	defer r.Body.Close()
+
+	if err := vol.Import(r.Body); err != nil {
+		utils.Error(w, http.StatusInternalServerError, err)
+		return
+	}
+
+	utils.WriteResponse(w, http.StatusNoContent, "")
+}

pkg/api/server/register_volumes.go

@@ -173,6 +173,35 @@ func (s *APIServer) registerVolumeHandlers(r *mux.Router) error {
 	//     $ref: "#/responses/internalError"
 	r.Handle(VersionedPath("/libpod/volumes/{name}/export"), s.APIHandler(libpod.ExportVolume)).Methods(http.MethodGet)
 
+	// swagger:operation POST /libpod/volumes/{name}/import libpod VolumeImportLibpod
+	// ---
+	// tags:
+	//  - volumes
+	// summary: Populate a volume by importing provided tar
+	// parameters:
+	//  - in: path
+	//    name: name
+	//    type: string
+	//    required: true
+	//    description: the name or ID of the volume
+	//  - in: body
+	//    name: inputStream
+	//    description: |
+	//      An uncompressed tar archive
+	//    schema:
+	//      type: string
+	//      format: binary
+	// produces:
+	// - application/json
+	// responses:
+	//   204:
+	//     description: Successful import
+	//   404:
+	//     $ref: "#/responses/volumeNotFound"
+	//   500:
+	//     $ref: "#/responses/internalError"
+	r.Handle(VersionedPath("/libpod/volumes/{name}/import"), s.APIHandler(libpod.ImportVolume)).Methods(http.MethodPost)
+
 	/*
 	 * Docker compatibility endpoints
 	 */

pkg/bindings/volumes/volumes.go

@@ -2,15 +2,12 @@ package volumes
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"io"
 	"net/http"
-	"os"
 	"strings"
 
 	"github.com/containers/podman/v5/pkg/bindings"
-	"github.com/containers/podman/v5/pkg/domain/entities"
 	"github.com/containers/podman/v5/pkg/domain/entities/reports"
 	entitiesTypes "github.com/containers/podman/v5/pkg/domain/entities/types"
 	jsoniter "github.com/json-iterator/go"
@@ -146,17 +143,7 @@ func Exists(ctx context.Context, nameOrID string, options *ExistsOptions) (bool,
 }
 
 // Export exports a volume to the given path
-func Export(ctx context.Context, nameOrID string, options entities.VolumeExportOptions) error {
+func Export(ctx context.Context, nameOrID string, exportTo io.Writer) error {
-	if options.OutputPath == "" {
-		return errors.New("must provide valid path for file to write to")
-	}
-
-	targetFile, err := os.Create(options.OutputPath)
-	if err != nil {
-		return fmt.Errorf("unable to create target file path %q: %w", options.OutputPath, err)
-	}
-	defer targetFile.Close()
-
 	conn, err := bindings.GetClient(ctx)
 	if err != nil {
 		return err
@@ -168,9 +155,25 @@ func Export(ctx context.Context, nameOrID string, options entities.VolumeExportO
 	defer response.Body.Close()
 
 	if response.IsSuccess() || response.IsRedirection() {
-		if _, err := io.Copy(targetFile, response.Body); err != nil {
+		if _, err := io.Copy(exportTo, response.Body); err != nil {
 			return fmt.Errorf("writing volume %s contents to file: %w", nameOrID, err)
 		}
 	}
 	return response.Process(nil)
 }
+
+// Import imports the given tar into the given volume
+func Import(ctx context.Context, nameOrID string, importFrom io.Reader) error {
+	conn, err := bindings.GetClient(ctx)
+	if err != nil {
+		return err
+	}
+
+	response, err := conn.DoRequest(ctx, importFrom, http.MethodPost, "/volumes/%s/import", nil, nil, nameOrID)
+	if err != nil {
+		return err
+	}
+	defer response.Body.Close()
+
+	return response.Process(nil)
+}

pkg/domain/entities/engine_container.go

@@ -117,4 +117,5 @@ type ContainerEngine interface { //nolint:interfacebloat
 	VolumeUnmount(ctx context.Context, namesOrIds []string) ([]*VolumeUnmountReport, error)
 	VolumeReload(ctx context.Context) (*VolumeReloadReport, error)
 	VolumeExport(ctx context.Context, nameOrID string, options VolumeExportOptions) error
+	VolumeImport(ctx context.Context, nameOrID string, options VolumeImportOptions) error
 }

pkg/domain/entities/volumes.go

@@ -1,6 +1,7 @@
 package entities
 
 import (
+	"io"
 	"net/url"
 
 	"github.com/containers/podman/v5/pkg/domain/entities/types"
@@ -45,5 +46,11 @@ type VolumeUnmountReport = types.VolumeUnmountReport
 
 // VolumeExportOptions describes the options required to export a volume.
 type VolumeExportOptions struct {
-	OutputPath string
+	Output io.Writer
+}
+
+// VolumeImportOptions describes the options required to import a volume
+type VolumeImportOptions struct {
+	// Input will be closed upon being fully consumed
+	Input io.Reader
 }

pkg/domain/infra/abi/play.go

@@ -38,7 +38,7 @@ import (
 	"github.com/containers/podman/v5/pkg/specgenutil"
 	"github.com/containers/podman/v5/pkg/systemd/notifyproxy"
 	"github.com/containers/podman/v5/pkg/util"
-	"github.com/containers/podman/v5/utils"
+	"github.com/containers/storage/pkg/archive"
 	"github.com/containers/storage/pkg/fileutils"
 	"github.com/coreos/go-systemd/v22/daemon"
 	"github.com/opencontainers/go-digest"
@@ -1504,7 +1504,7 @@ func (ic *ContainerEngine) importVolume(ctx context.Context, vol *libpod.Volume,
 	}
 
 	// dont care if volume is mounted or not we are gonna import everything to mountPoint
-	return utils.UntarToFileSystem(mountPoint, tarFile, nil)
+	return archive.Untar(tarFile, mountPoint, nil)
 }
 
 // readConfigMapFromFile returns a kubernetes configMap obtained from --configmap flag

pkg/domain/infra/abi/volumes.go

@@ -7,7 +7,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"os"
 
 	"github.com/containers/podman/v5/libpod"
 	"github.com/containers/podman/v5/libpod/define"
@@ -243,29 +242,32 @@ func (ic *ContainerEngine) VolumeReload(ctx context.Context) (*entities.VolumeRe
 }
 
 func (ic *ContainerEngine) VolumeExport(ctx context.Context, nameOrID string, options entities.VolumeExportOptions) error {
-	if options.OutputPath == "" {
+	vol, err := ic.Libpod.LookupVolume(nameOrID)
-		return errors.New("must provide valid path for file to write to")
-	}
-
-	targetFile, err := os.Create(options.OutputPath)
-	if err != nil {
-		return fmt.Errorf("unable to create target file path %q: %w", options.OutputPath, err)
-	}
-	defer targetFile.Close()
-
-	vol, err := ic.Libpod.GetVolume(nameOrID)
 	if err != nil {
 		return err
 	}
 
-	contents, err := vol.ExportVolume()
+	contents, err := vol.Export()
 	if err != nil {
 		return err
 	}
 	defer contents.Close()
 
-	if _, err := io.Copy(targetFile, contents); err != nil {
+	if _, err := io.Copy(options.Output, contents); err != nil {
-		return fmt.Errorf("writing volume %s to file: %w", vol.Name(), err)
+		return fmt.Errorf("writing volume %s contents: %w", vol.Name(), err)
+	}
+
+	return nil
+}
+
+func (ic *ContainerEngine) VolumeImport(ctx context.Context, nameOrID string, options entities.VolumeImportOptions) error {
+	vol, err := ic.Libpod.LookupVolume(nameOrID)
+	if err != nil {
+		return err
+	}
+
+	if err := vol.Import(options.Input); err != nil {
+		return err
 	}
 
 	return nil

pkg/domain/infra/tunnel/volumes.go

@@ -115,5 +115,9 @@ func (ic *ContainerEngine) VolumeReload(ctx context.Context) (*entities.VolumeRe
 }
 
 func (ic *ContainerEngine) VolumeExport(ctx context.Context, nameOrID string, options entities.VolumeExportOptions) error {
-	return volumes.Export(ic.ClientCtx, nameOrID, options)
+	return volumes.Export(ic.ClientCtx, nameOrID, options.Output)
+}
+
+func (ic *ContainerEngine) VolumeImport(ctx context.Context, nameOrID string, options entities.VolumeImportOptions) error {
+	return volumes.Import(ic.ClientCtx, nameOrID, options.Input)
 }

test/e2e/volume_create_test.go

@@ -92,10 +92,6 @@ var _ = Describe("Podman volume create", func() {
 	})
 
 	It("podman create and import volume", func() {
-		if podmanTest.RemoteTest {
-			Skip("Volume export check does not work with a remote client")
-		}
-
 		volName := "my_vol_" + RandomString(10)
 		session := podmanTest.Podman([]string{"volume", "create", volName})
 		session.WaitWithDefaultTimeout()
@@ -135,11 +131,11 @@ var _ = Describe("Podman volume create", func() {
 
 		session = podmanTest.Podman([]string{"volume", "import", "notfound", "-"})
 		session.WaitWithDefaultTimeout()
-		Expect(session).To(ExitWithError(125, "no such volume notfound"))
+		Expect(session).To(ExitWithError(125, "no volume with name \"notfound\" found"))
 
 		session = podmanTest.Podman([]string{"volume", "export", "notfound"})
 		session.WaitWithDefaultTimeout()
-		Expect(session).To(ExitWithError(125, "no such volume notfound"))
+		Expect(session).To(ExitWithError(125, "no volume with name \"notfound\" found"))
 	})
 
 	It("podman create volume with bad volume option", func() {

test/system/160-volumes.bats

@@ -242,7 +242,6 @@ EOF
 
 # Podman volume import test
 @test "podman volume import test" {
-    skip_if_remote "volumes import is not applicable on podman-remote"
     run_podman volume create --driver local my_vol
     run_podman run --rm -v my_vol:/data $IMAGE sh -c "echo hello >> /data/test"
     run_podman volume create my_vol2

utils/utils.go

@@ -50,22 +50,6 @@ func ExecCmdWithStdStreams(stdin io.Reader, stdout, stderr io.Writer, env []stri
 	return nil
 }
 
-// UntarToFileSystem untars an os.file of a tarball to a destination in the filesystem
-func UntarToFileSystem(dest string, tarball *os.File, options *archive.TarOptions) error {
-	logrus.Debugf("untarring %s", tarball.Name())
-	return archive.Untar(tarball, dest, options)
-}
-
-// Creates a new tar file and writes bytes from io.ReadCloser
-func CreateTarFromSrc(source string, dest string) error {
-	file, err := os.Create(dest)
-	if err != nil {
-		return fmt.Errorf("could not create tarball file '%s': %w", dest, err)
-	}
-	defer file.Close()
-	return TarChrootToFilesystem(source, file)
-}
-
 // TarToFilesystem creates a tarball from source and writes to an os.file
 // provided
 func TarToFilesystem(source string, tarball *os.File) error {
@@ -88,22 +72,6 @@ func Tar(source string) (io.ReadCloser, error) {
 	return archive.Tar(source, archive.Uncompressed)
 }
 
-// TarChrootToFilesystem creates a tarball from source and writes to an os.file
-// provided while chrooted to the source.
-func TarChrootToFilesystem(source string, tarball *os.File) error {
-	tb, err := TarWithChroot(source)
-	if err != nil {
-		return err
-	}
-	defer tb.Close()
-	_, err = io.Copy(tarball, tb)
-	if err != nil {
-		return err
-	}
-	logrus.Debugf("wrote tarball file %s", tarball.Name())
-	return nil
-}
-
 // TarWithChroot creates a tarball from source and returns a readcloser of it
 // while chrooted to the source.
 func TarWithChroot(source string) (io.ReadCloser, error) {