From 98876454cb86fd01f400de2d6f2877f4b98d0155 Mon Sep 17 00:00:00 2001 From: Matt Heon Date: Sat, 14 Jun 2025 09:17:30 -0400 Subject: [PATCH] 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 --- cmd/podman/volumes/export.go | 26 ++++-- cmd/podman/volumes/import.go | 80 +++++-------------- .../source/markdown/podman-volume-export.1.md | 2 - .../source/markdown/podman-volume-import.1.md | 2 - libpod/volume.go | 32 +++++++- pkg/api/handlers/libpod/volumes.go | 32 +++++++- pkg/api/server/register_volumes.go | 35 +++++++- pkg/bindings/volumes/volumes.go | 33 ++++---- pkg/domain/entities/engine_container.go | 1 + pkg/domain/entities/volumes.go | 9 ++- pkg/domain/infra/abi/play.go | 4 +- pkg/domain/infra/abi/volumes.go | 32 ++++---- pkg/domain/infra/tunnel/volumes.go | 6 +- test/e2e/volume_create_test.go | 8 +- test/system/160-volumes.bats | 1 - utils/utils.go | 32 -------- 16 files changed, 181 insertions(+), 154 deletions(-) diff --git a/cmd/podman/volumes/export.go b/cmd/podman/volumes/export.go index 24d5d3eb4d..47f11f6a60 100644 --- a/cmd/podman/volumes/export.go +++ b/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 == "" { - return errors.New("expects output path, use --output=[path]") + if targetPath == "" && cmd.Flag("output").Changed { + return errors.New("must provide valid path for file to write to") } - if err := containerEngine.VolumeExport(ctx, args[0], cliExportOpts); err != nil { - return err + exportOpts := entities.VolumeExportOptions{} + + if targetPath != "" { + targetFile, err := os.Create(targetPath) + if err != nil { + return fmt.Errorf("unable to create target file path %q: %w", targetPath, err) + } + defer targetFile.Close() + exportOpts.Output = targetFile + } else { + exportOpts.Output = os.Stdout } - return nil + + return containerEngine.VolumeExport(ctx, args[0], exportOpts) } diff --git a/cmd/podman/volumes/import.go b/cmd/podman/volumes/import.go index 40d1e37b3f..2790d1bf3f 100644 --- a/cmd/podman/volumes/import.go +++ b/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 - 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] + opts := entities.VolumeImportOptions{} - if tarPath != "-" { - err := parse.ValidateFileName(tarPath) - if err != nil { - return err - } - - // open tar file - tarFile, err = os.Open(tarPath) - if err != nil { - return err - } + filepath := args[1] + if filepath == "-" { + opts.Input = os.Stdin } else { - tarFile = os.Stdin + if err := parse.ValidateFileName(filepath); err != nil { + return err + } + + targetFile, err := os.Open(filepath) + if err != nil { + return fmt.Errorf("unable open input file: %w", err) + } + defer targetFile.Close() + opts.Input = targetFile } - inspectOpts.Type = common.VolumeType - 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") - } - mountPoint := volumeData[0].VolumeConfigResponse.Mountpoint - driver := volumeData[0].VolumeConfigResponse.Driver - 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) + containerEngine := registry.ContainerEngine() + ctx := context.Background() + + return containerEngine.VolumeImport(ctx, args[0], opts) } diff --git a/docs/source/markdown/podman-volume-export.1.md b/docs/source/markdown/podman-volume-export.1.md index 1257c7d2a6..2fd2381e06 100644 --- a/docs/source/markdown/podman-volume-export.1.md +++ b/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 diff --git a/docs/source/markdown/podman-volume-import.1.md b/docs/source/markdown/podman-volume-import.1.md index ba59d9dd23..4aa0aaec72 100644 --- a/docs/source/markdown/podman-volume-import.1.md +++ b/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 diff --git a/libpod/volume.go b/libpod/volume.go index 238d5f5bfd..68c15009c3 100644 --- a/libpod/volume.go +++ b/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 +} diff --git a/pkg/api/handlers/libpod/volumes.go b/pkg/api/handlers/libpod/volumes.go index ec7ed4fb30..438a5657d2 100644 --- a/pkg/api/handlers/libpod/volumes.go +++ b/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, "") +} diff --git a/pkg/api/server/register_volumes.go b/pkg/api/server/register_volumes.go index 9c67b83d05..6d03409d2d 100644 --- a/pkg/api/server/register_volumes.go +++ b/pkg/api/server/register_volumes.go @@ -161,18 +161,47 @@ func (s *APIServer) registerVolumeHandlers(r *mux.Router) error { // description: the name or ID of the volume // produces: // - application/x-tar - // responses: - // 200: + // responses: + // 200: // description: no error // schema: // type: string - // format: binary + // format: binary // 404: // $ref: "#/responses/volumeNotFound" // 500: // $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 */ diff --git a/pkg/bindings/volumes/volumes.go b/pkg/bindings/volumes/volumes.go index 9440fd8176..ae6fdd8d8b 100644 --- a/pkg/bindings/volumes/volumes.go +++ b/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 { - 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() - +func Export(ctx context.Context, nameOrID string, exportTo io.Writer) error { 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) +} diff --git a/pkg/domain/entities/engine_container.go b/pkg/domain/entities/engine_container.go index 18a12e092d..2f3516ed4b 100644 --- a/pkg/domain/entities/engine_container.go +++ b/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 } diff --git a/pkg/domain/entities/volumes.go b/pkg/domain/entities/volumes.go index 54fa8acba1..fe310f4fe3 100644 --- a/pkg/domain/entities/volumes.go +++ b/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 } diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go index 3a07ca2ada..9d22e88cdb 100644 --- a/pkg/domain/infra/abi/play.go +++ b/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 diff --git a/pkg/domain/infra/abi/volumes.go b/pkg/domain/infra/abi/volumes.go index 4b57a1dc82..052e05724e 100644 --- a/pkg/domain/infra/abi/volumes.go +++ b/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 == "" { - 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) + vol, err := ic.Libpod.LookupVolume(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 { - return fmt.Errorf("writing volume %s to file: %w", vol.Name(), err) + if _, err := io.Copy(options.Output, contents); err != nil { + 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 diff --git a/pkg/domain/infra/tunnel/volumes.go b/pkg/domain/infra/tunnel/volumes.go index bc14430d02..d8ba4007f1 100644 --- a/pkg/domain/infra/tunnel/volumes.go +++ b/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) } diff --git a/test/e2e/volume_create_test.go b/test/e2e/volume_create_test.go index dfdee43fe2..f8a1ef6b43 100644 --- a/test/e2e/volume_create_test.go +++ b/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() { diff --git a/test/system/160-volumes.bats b/test/system/160-volumes.bats index 2438a535e4..703d5671f5 100644 --- a/test/system/160-volumes.bats +++ b/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 diff --git a/utils/utils.go b/utils/utils.go index c5f773964a..9461124e90 100644 --- a/utils/utils.go +++ b/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) {