Skip to content

Commit

Permalink
export: use io.Writer instead of file
Browse files Browse the repository at this point in the history
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 containers#16870

Signed-off-by: Paul Holzinger <[email protected]>
  • Loading branch information
Luap99 committed Dec 20, 2022
1 parent 56982a9 commit 3ac5d10
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 47 deletions.
21 changes: 16 additions & 5 deletions cmd/podman/containers/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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)
}
4 changes: 2 additions & 2 deletions libpod/container_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
Expand Down
10 changes: 2 additions & 8 deletions libpod/container_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
}

Expand Down
28 changes: 8 additions & 20 deletions pkg/api/handlers/compat/containers_export.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
}
2 changes: 1 addition & 1 deletion pkg/domain/entities/containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ type CommitReport struct {
}

type ContainerExportOptions struct {
Output string
Output io.Writer
}

type CheckpointOptions struct {
Expand Down
12 changes: 1 addition & 11 deletions pkg/domain/infra/tunnel/containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
7 changes: 7 additions & 0 deletions test/apiv2/25-containersMore.at
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 3ac5d10

Please sign in to comment.