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 <pholzing@redhat.com>
This commit is contained in:
Paul Holzinger
2022-12-19 19:45:06 +01:00
parent 56982a9236
commit 3ac5d10098
7 changed files with 37 additions and 47 deletions

View File

@ -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)
}

View File

@ -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

View File

@ -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
}

View File

@ -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))
// 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 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))
return
}
defer rdr.Close()
utils.WriteResponse(w, http.StatusOK, rdr)
}

View File

@ -181,7 +181,7 @@ type CommitReport struct {
}
type ContainerExportOptions struct {
Output string
Output io.Writer
}
type CheckpointOptions struct {

View File

@ -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) {

View File

@ -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