From 38091fed3336701fd36b1e53d9842c381b0a643b Mon Sep 17 00:00:00 2001 From: Matthieu Nottale Date: Wed, 20 Jun 2018 15:36:46 +0200 Subject: [PATCH] split, merge: Handle in-place conversion, move out of experimental. Signed-off-by: Matthieu Nottale --- cmd/docker-app/merge.go | 65 +++++++++++++++++++++++++++++++----- cmd/docker-app/root.go | 4 +-- cmd/docker-app/split.go | 31 ++++++++++++----- e2e/binary_test.go | 14 ++++---- internal/packager/extract.go | 49 +++++++++++++++++---------- 5 files changed, 121 insertions(+), 42 deletions(-) diff --git a/cmd/docker-app/merge.go b/cmd/docker-app/merge.go index f39027e81..12a910b61 100644 --- a/cmd/docker-app/merge.go +++ b/cmd/docker-app/merge.go @@ -1,29 +1,66 @@ package main import ( + "fmt" "io" + "io/ioutil" "os" + "strings" "github.com/docker/app/internal" "github.com/docker/app/internal/packager" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" + "github.com/pkg/errors" "github.com/spf13/cobra" ) var mergeOutputFile string +// Check appname directory for extra files and return them +func extraFiles(appname string) ([]string, error) { + files, err := ioutil.ReadDir(appname) + if err != nil { + return nil, err + } + var res []string + for _, f := range files { + hit := false + for _, afn := range internal.FileNames { + if afn == f.Name() { + hit = true + break + } + } + if !hit { + res = append(res, f.Name()) + } + } + return res, nil +} + func mergeCmd(dockerCli command.Cli) *cobra.Command { cmd := &cobra.Command{ - Use: "merge [] [-o output_dir]", + Use: "merge [] [-o output_file]", Short: "Merge the application as a single file multi-document YAML", Args: cli.RequiresMaxArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - appname, cleanup, err := packager.Extract(firstOrEmpty(args)) + extractedApp, err := packager.ExtractWithOrigin(firstOrEmpty(args)) if err != nil { return err } - defer cleanup() + defer extractedApp.Cleanup() + inPlace := mergeOutputFile == "" + if inPlace { + extra, err := extraFiles(extractedApp.AppName) + if err != nil { + return errors.Wrap(err, "error scanning application directory") + } + if len(extra) != 0 { + return fmt.Errorf("refusing to overwrite %s: extra files would be deleted: %s", extractedApp.OriginalAppName, strings.Join(extra, ",")) + } + mergeOutputFile = extractedApp.OriginalAppName + ".tmp" + } var target io.Writer if mergeOutputFile == "-" { target = dockerCli.Out() @@ -32,13 +69,25 @@ func mergeCmd(dockerCli command.Cli) *cobra.Command { if err != nil { return err } - defer target.(io.WriteCloser).Close() } - return packager.Merge(appname, target) + if err := packager.Merge(extractedApp.AppName, target); err != nil { + return err + } + if mergeOutputFile != "-" { + // Need to close for the Rename to work on windows. + target.(io.WriteCloser).Close() + } + if inPlace { + if err := os.RemoveAll(extractedApp.OriginalAppName); err != nil { + return errors.Wrap(err, "failed to erase previous application") + } + if err := os.Rename(mergeOutputFile, extractedApp.OriginalAppName); err != nil { + return errors.Wrap(err, "failed to rename new application") + } + } + return nil }, } - if internal.Experimental == "on" { - cmd.Flags().StringVarP(&mergeOutputFile, "output", "o", "-", "Output file (default: stdout)") - } + cmd.Flags().StringVarP(&mergeOutputFile, "output", "o", "", "Output file (default: in-place)") return cmd } diff --git a/cmd/docker-app/root.go b/cmd/docker-app/root.go index 004fe0c33..26cf5abd9 100644 --- a/cmd/docker-app/root.go +++ b/cmd/docker-app/root.go @@ -38,9 +38,11 @@ func addCommands(cmd *cobra.Command, dockerCli *command.DockerCli) { initCmd(), inspectCmd(dockerCli), lsCmd(), + mergeCmd(dockerCli), pushCmd(), renderCmd(dockerCli), saveCmd(dockerCli), + splitCmd(), versionCmd(dockerCli), ) if internal.Experimental == "on" { @@ -48,10 +50,8 @@ func addCommands(cmd *cobra.Command, dockerCli *command.DockerCli) { imageAddCmd(), imageLoadCmd(), loadCmd(), - mergeCmd(dockerCli), packCmd(dockerCli), pullCmd(), - splitCmd(), unpackCmd(), ) } diff --git a/cmd/docker-app/split.go b/cmd/docker-app/split.go index 6ffde23ee..8851656df 100644 --- a/cmd/docker-app/split.go +++ b/cmd/docker-app/split.go @@ -1,9 +1,11 @@ package main import ( - "github.com/docker/app/internal" + "os" + "github.com/docker/app/internal/packager" "github.com/docker/cli/cli" + "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -11,20 +13,33 @@ var splitOutputDir string func splitCmd() *cobra.Command { cmd := &cobra.Command{ - Use: "split [] [-o output_dir]", + Use: "split [] [-o output]", Short: "Split a single-file application into multiple files", Args: cli.RequiresMaxArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - appname, cleanup, err := packager.Extract(firstOrEmpty(args)) + extractedApp, err := packager.ExtractWithOrigin(firstOrEmpty(args)) if err != nil { return err } - defer cleanup() - return packager.Split(appname, splitOutputDir) + defer extractedApp.Cleanup() + inPlace := splitOutputDir == "" + if inPlace { + splitOutputDir = extractedApp.OriginalAppName + ".tmp" + } + if err := packager.Split(extractedApp.AppName, splitOutputDir); err != nil { + return err + } + if inPlace { + if err := os.RemoveAll(extractedApp.OriginalAppName); err != nil { + return errors.Wrap(err, "failed to erase previous application directory") + } + if err := os.Rename(splitOutputDir, extractedApp.OriginalAppName); err != nil { + return errors.Wrap(err, "failed to rename new application directory") + } + } + return nil }, } - if internal.Experimental == "on" { - cmd.Flags().StringVarP(&splitOutputDir, "output", "o", ".", "Output directory") - } + cmd.Flags().StringVarP(&splitOutputDir, "output", "o", "", "Output application directory (default: in-place)") return cmd } diff --git a/e2e/binary_test.go b/e2e/binary_test.go index 32b71db4f..4a1c43cf8 100644 --- a/e2e/binary_test.go +++ b/e2e/binary_test.go @@ -287,19 +287,19 @@ func TestHelmBinary(t *testing.T) { } func TestSplitMergeBinary(t *testing.T) { - dockerApp, hasExperimental := getBinary(t) - if !hasExperimental { - t.Skip("experimental mode needed for this test") - } + dockerApp, _ := getBinary(t) app := "render/envvariables" assertCommand(t, dockerApp, "merge", app, "-o", "remerged.dockerapp") defer os.Remove("remerged.dockerapp") // test that inspect works on single-file assertCommandOutput(t, "envvariables-inspect.golden", dockerApp, "inspect", "remerged") // split it - assertCommand(t, dockerApp, "split", "remerged", "-o", "splitted.dockerapp") - defer os.RemoveAll("splitted.dockerapp") - assertCommandOutput(t, "envvariables-inspect.golden", dockerApp, "inspect", "splitted") + assertCommand(t, dockerApp, "split", "remerged", "-o", "split.dockerapp") + defer os.RemoveAll("split.dockerapp") + assertCommandOutput(t, "envvariables-inspect.golden", dockerApp, "inspect", "split") + // test inplace + assertCommand(t, dockerApp, "merge", "split") + assertCommand(t, dockerApp, "split", "split") } func TestImageBinary(t *testing.T) { diff --git a/internal/packager/extract.go b/internal/packager/extract.go index 99f7c9709..7c8868ca3 100644 --- a/internal/packager/extract.go +++ b/internal/packager/extract.go @@ -14,6 +14,13 @@ import ( "github.com/pkg/errors" ) +// ExtractedApp represents a potentially extracted application package +type ExtractedApp struct { + OriginalAppName string + AppName string + Cleanup func() +} + var ( noop = func() {} ) @@ -47,7 +54,7 @@ func findApp() (string, error) { } // extractImage extracts a docker application in a docker image to a temporary directory -func extractImage(appname string) (string, func(), error) { +func extractImage(appname string) (ExtractedApp, error) { var imagename string if strings.Contains(appname, ":") { nametag := strings.Split(appname, ":") @@ -65,42 +72,49 @@ func extractImage(appname string) (string, func(), error) { } tempDir, err := ioutil.TempDir("", "dockerapp") if err != nil { - return "", noop, errors.Wrap(err, "failed to create temporary directory") + return ExtractedApp{}, errors.Wrap(err, "failed to create temporary directory") } defer os.RemoveAll(tempDir) err = Load(imagename, tempDir) if err != nil { if !strings.Contains(imagename, "/") { - return "", noop, fmt.Errorf("could not locate application in either filesystem or docker image") + return ExtractedApp{}, fmt.Errorf("could not locate application in either filesystem or docker image") } // Try to pull it cmd := exec.Command("docker", "pull", imagename) if err := cmd.Run(); err != nil { - return "", noop, fmt.Errorf("could not locate application in filesystem, docker image or registry") + return ExtractedApp{}, fmt.Errorf("could not locate application in filesystem, docker image or registry") } if err := Load(imagename, tempDir); err != nil { - return "", noop, errors.Wrap(err, "failed to load pulled image") + return ExtractedApp{}, errors.Wrap(err, "failed to load pulled image") } } // this gave us a compressed app, run through extract again - return Extract(filepath.Join(tempDir, appname)) + appname, cleanup, err := Extract(filepath.Join(tempDir, appname)) + return ExtractedApp{"", appname, cleanup}, err +} + +// Extract extracts the app content if it's an archive or single-file +func Extract(appname string) (string, func(), error) { + extracted, err := ExtractWithOrigin(appname) + return extracted.AppName, extracted.Cleanup, err } -// Extract extracts the app content if argument is an archive, or does nothing if a dir. -// It returns effective app name, and cleanup function +// ExtractWithOrigin extracts the app content if argument is an archive, or does nothing if a dir. +// It returns source file, effective app name, and cleanup function // If appname is empty, it looks into cwd, and all subdirs for a single matching .dockerapp // If nothing is found, it looks for an image and loads it -func Extract(appname string) (string, func(), error) { +func ExtractWithOrigin(appname string) (ExtractedApp, error) { if appname == "" { var err error if appname, err = findApp(); err != nil { - return "", nil, err + return ExtractedApp{}, err } } if appname == "." { var err error if appname, err = os.Getwd(); err != nil { - return "", nil, errors.Wrap(err, "cannot resolve current working directory") + return ExtractedApp{}, errors.Wrap(err, "cannot resolve current working directory") } } originalAppname := appname @@ -118,12 +132,12 @@ func Extract(appname string) (string, func(), error) { } if s.IsDir() { // directory: already decompressed - return appname, noop, nil + return ExtractedApp{appname, appname, noop}, nil } // not a dir: single-file or a tarball package, extract that in a temp dir tempDir, err := ioutil.TempDir("", "dockerapp") if err != nil { - return "", noop, errors.Wrap(err, "failed to create temporary directory") + return ExtractedApp{}, errors.Wrap(err, "failed to create temporary directory") } defer func() { if err != nil { @@ -132,16 +146,16 @@ func Extract(appname string) (string, func(), error) { }() appDir := filepath.Join(tempDir, filepath.Base(appname)) if err = os.Mkdir(appDir, 0755); err != nil { - return "", noop, errors.Wrap(err, "failed to create application in temporary directory") + return ExtractedApp{}, errors.Wrap(err, "failed to create application in temporary directory") } if err = extract(appname, appDir); err == nil { - return appDir, func() { os.RemoveAll(tempDir) }, nil + return ExtractedApp{appname, appDir, func() { os.RemoveAll(tempDir) }}, nil } if err = extractSingleFile(appname, appDir); err != nil { - return "", noop, err + return ExtractedApp{}, err } // not a tarball, single-file then - return appDir, func() { os.RemoveAll(tempDir) }, nil + return ExtractedApp{appname, appDir, func() { os.RemoveAll(tempDir) }}, nil } func extractSingleFile(appname, appDir string) error { @@ -177,6 +191,7 @@ func extract(appname, outputDir string) error { if err != nil { return errors.Wrap(err, "failed to open application package") } + defer f.Close() tarReader := tar.NewReader(f) outputDir = outputDir + "/" for {