From 3ac5d100985421314328d3aaf6d6745e31ae359c Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 19 Dec 2022 19:45:06 +0100 Subject: [PATCH] export: use io.Writer instead of file This allows use to use STDOUT directly without having to call open again, also this makes the export API endpoint much more performant since it no longer needs to copy to a temp file. I noticed that there was no export API test so I added one. And lastly opening /dev/stdout will not work on windows. Fixes #16870 Signed-off-by: Paul Holzinger --- cmd/podman/containers/export.go | 21 +++++++++++---- libpod/container_api.go | 4 +-- libpod/container_internal.go | 10 ++----- pkg/api/handlers/compat/containers_export.go | 28 ++++++-------------- pkg/domain/entities/containers.go | 2 +- pkg/domain/infra/tunnel/containers.go | 12 +-------- test/apiv2/25-containersMore.at | 7 +++++ 7 files changed, 37 insertions(+), 47 deletions(-) diff --git a/cmd/podman/containers/export.go b/cmd/podman/containers/export.go index d78bfe9608..96f7c8644f 100644 --- a/cmd/podman/containers/export.go +++ b/cmd/podman/containers/export.go @@ -43,13 +43,14 @@ var ( var ( exportOpts entities.ContainerExportOptions + outputFile string ) func exportFlags(cmd *cobra.Command) { flags := cmd.Flags() outputFlagName := "output" - flags.StringVarP(&exportOpts.Output, outputFlagName, "o", "", "Write to a specified file (default: stdout, which must be redirected)") + flags.StringVarP(&outputFile, outputFlagName, "o", "", "Write to a specified file (default: stdout, which must be redirected)") _ = cmd.RegisterFlagCompletionFunc(outputFlagName, completion.AutocompleteDefault) } @@ -67,14 +68,24 @@ func init() { } func export(cmd *cobra.Command, args []string) error { - if len(exportOpts.Output) == 0 { + if len(outputFile) == 0 { file := os.Stdout if term.IsTerminal(int(file.Fd())) { return errors.New("refusing to export to terminal. Use -o flag or redirect") } - exportOpts.Output = "/dev/stdout" - } else if err := parse.ValidateFileName(exportOpts.Output); err != nil { - return err + exportOpts.Output = file + } else { + if err := parse.ValidateFileName(outputFile); err != nil { + return err + } + // open file here with WRONLY since on MacOS it can fail to open /dev/stderr in read mode for example + // https://github.com/containers/podman/issues/16870 + file, err := os.OpenFile(outputFile, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644) + if err != nil { + return err + } + defer file.Close() + exportOpts.Output = file } return registry.ContainerEngine().ContainerExport(context.Background(), args[0], exportOpts) } diff --git a/libpod/container_api.go b/libpod/container_api.go index 9abe0a189e..71f97a240b 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -462,7 +462,7 @@ func (c *Container) Unpause() error { // Export exports a container's root filesystem as a tar archive // The archive will be saved as a file at the given path -func (c *Container) Export(path string) error { +func (c *Container) Export(out io.Writer) error { if !c.batched { c.lock.Lock() defer c.lock.Unlock() @@ -477,7 +477,7 @@ func (c *Container) Export(path string) error { } defer c.newContainerEvent(events.Mount) - return c.export(path) + return c.export(out) } // AddArtifact creates and writes to an artifact file for the container diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 4abd701d49..c3fee6552a 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -745,7 +745,7 @@ func (c *Container) removeConmonFiles() error { return nil } -func (c *Container) export(path string) error { +func (c *Container) export(out io.Writer) error { mountPoint := c.state.Mountpoint if !c.state.Mounted { containerMount, err := c.runtime.store.Mount(c.ID(), c.config.MountLabel) @@ -765,13 +765,7 @@ func (c *Container) export(path string) error { return fmt.Errorf("reading container directory %q: %w", c.ID(), err) } - outFile, err := os.Create(path) - if err != nil { - return fmt.Errorf("creating file %q: %w", path, err) - } - defer outFile.Close() - - _, err = io.Copy(outFile, input) + _, err = io.Copy(out, input) return err } diff --git a/pkg/api/handlers/compat/containers_export.go b/pkg/api/handlers/compat/containers_export.go index 03e547411e..0392ad6de6 100644 --- a/pkg/api/handlers/compat/containers_export.go +++ b/pkg/api/handlers/compat/containers_export.go @@ -3,7 +3,6 @@ package compat import ( "fmt" "net/http" - "os" "github.com/containers/podman/v4/libpod" "github.com/containers/podman/v4/pkg/api/handlers/utils" @@ -18,25 +17,14 @@ func ExportContainer(w http.ResponseWriter, r *http.Request) { utils.ContainerNotFound(w, name, err) return } - tmpfile, err := os.CreateTemp("", "api.tar") - if err != nil { - utils.Error(w, http.StatusInternalServerError, fmt.Errorf("unable to create tempfile: %w", err)) - return - } - defer os.Remove(tmpfile.Name()) - if err := tmpfile.Close(); err != nil { - utils.Error(w, http.StatusInternalServerError, fmt.Errorf("unable to close tempfile: %w", err)) - return - } - if err := con.Export(tmpfile.Name()); err != nil { - utils.Error(w, http.StatusInternalServerError, fmt.Errorf("failed to save image: %w", err)) - return - } - rdr, err := os.Open(tmpfile.Name()) - if err != nil { - utils.Error(w, http.StatusInternalServerError, fmt.Errorf("failed to read the exported tarfile: %w", err)) + + // set the correct header + w.Header().Set("Content-Type", "application/x-tar") + // NOTE: As described in w.Write() it automatically sets the http code to + // 200 on first write if no other code was set. + + if err := con.Export(w); err != nil { + utils.Error(w, http.StatusInternalServerError, fmt.Errorf("failed to export container: %w", err)) return } - defer rdr.Close() - utils.WriteResponse(w, http.StatusOK, rdr) } diff --git a/pkg/domain/entities/containers.go b/pkg/domain/entities/containers.go index 2deceaa12b..940e6b0889 100644 --- a/pkg/domain/entities/containers.go +++ b/pkg/domain/entities/containers.go @@ -181,7 +181,7 @@ type CommitReport struct { } type ContainerExportOptions struct { - Output string + Output io.Writer } type CheckpointOptions struct { diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index 2e0b47e485..1b6bbaecdd 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -355,17 +355,7 @@ func (ic *ContainerEngine) ContainerCommit(ctx context.Context, nameOrID string, } func (ic *ContainerEngine) ContainerExport(ctx context.Context, nameOrID string, options entities.ContainerExportOptions) error { - var ( - err error - w io.Writer - ) - if len(options.Output) > 0 { - w, err = os.Create(options.Output) - if err != nil { - return err - } - } - return containers.Export(ic.ClientCtx, nameOrID, w, nil) + return containers.Export(ic.ClientCtx, nameOrID, options.Output, nil) } func (ic *ContainerEngine) ContainerCheckpoint(ctx context.Context, namesOrIds []string, opts entities.CheckpointOptions) ([]*entities.CheckpointReport, error) { diff --git a/test/apiv2/25-containersMore.at b/test/apiv2/25-containersMore.at index 9cdc2a33f5..5198f988b1 100644 --- a/test/apiv2/25-containersMore.at +++ b/test/apiv2/25-containersMore.at @@ -51,6 +51,13 @@ like "$output" ".*merged" "Check container mount" # Unmount the container t POST libpod/containers/foo/unmount 204 +# export the container fs to tarball + +t GET libpod/containers/foo/export 200 +like "$(<$WORKDIR/curl.headers.out)" ".*"Content-Type\: application/x-tar".*" +tar_tf=$(tar tf $WORKDIR/curl.result.out) +like "$tar_tf" ".*bin/cat.*" "fetched tarball: contains bin/cat path" + t DELETE libpod/containers/foo?force=true 200 # Create 3 stopped containers to test containers prune