From 9ef8233da180cc635bcc75ac99ef564446daf094 Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Tue, 27 Mar 2018 13:16:32 +0900 Subject: [PATCH] client: support passing io.WriteCloser via SolveOpt for FSSyncTargetFile Signed-off-by: Akihiro Suda --- client/client_test.go | 53 ++++++++----------- client/exporters.go | 3 -- client/solve.go | 59 +++++++++++---------- cmd/buildctl/build.go | 63 ++++++++++++++++++----- examples/build-using-dockerfile/main.go | 39 +++++++------- frontend/dockerfile/dockerfile_test.go | 68 +++++++++---------------- session/filesync/filesync.go | 27 ++++------ 7 files changed, 158 insertions(+), 154 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index bfb457624..a11418b36 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -125,10 +125,8 @@ func testBuildHTTPSource(t *testing.T, sb integration.Sandbox) { defer os.RemoveAll(tmpdir) err = c.Solve(context.TODO(), def, SolveOpt{ - Exporter: ExporterLocal, - ExporterAttrs: map[string]string{ - exporterLocalOutputDir: tmpdir, - }, + Exporter: ExporterLocal, + ExporterOutputDir: tmpdir, }, nil) require.NoError(t, err) @@ -146,10 +144,8 @@ func testBuildHTTPSource(t *testing.T, sb integration.Sandbox) { require.NoError(t, err) err = c.Solve(context.TODO(), def, SolveOpt{ - Exporter: ExporterLocal, - ExporterAttrs: map[string]string{ - exporterLocalOutputDir: tmpdir, - }, + Exporter: ExporterLocal, + ExporterOutputDir: tmpdir, }, nil) require.NoError(t, err) @@ -195,10 +191,8 @@ func testResolveAndHosts(t *testing.T, sb integration.Sandbox) { defer os.RemoveAll(destDir) err = c.Solve(context.TODO(), def, SolveOpt{ - Exporter: ExporterLocal, - ExporterAttrs: map[string]string{ - "output": destDir, - }, + Exporter: ExporterLocal, + ExporterOutputDir: destDir, }, nil) require.NoError(t, err) @@ -241,10 +235,8 @@ func testUser(t *testing.T, sb integration.Sandbox) { defer os.RemoveAll(destDir) err = c.Solve(context.TODO(), def, SolveOpt{ - Exporter: ExporterLocal, - ExporterAttrs: map[string]string{ - "output": destDir, - }, + Exporter: ExporterLocal, + ExporterOutputDir: destDir, }, nil) require.NoError(t, err) @@ -294,14 +286,16 @@ func testOCIExporter(t *testing.T, sb integration.Sandbox) { defer os.RemoveAll(destDir) out := filepath.Join(destDir, "out.tar") + outW, err := os.Create(out) + require.NoError(t, err) target := "example.com/buildkit/testoci:latest" err = c.Solve(context.TODO(), def, SolveOpt{ Exporter: exp, ExporterAttrs: map[string]string{ - "output": out, - "name": target, + "name": target, }, + ExporterOutput: outW, }, nil) require.NoError(t, err) @@ -413,10 +407,8 @@ func testBuildPushAndValidate(t *testing.T, sb integration.Sandbox) { defer os.RemoveAll(destDir) err = c.Solve(context.TODO(), def, SolveOpt{ - Exporter: ExporterLocal, - ExporterAttrs: map[string]string{ - "output": destDir, - }, + Exporter: ExporterLocal, + ExporterOutputDir: destDir, }, nil) require.NoError(t, err) @@ -582,12 +574,12 @@ func testDuplicateWhiteouts(t *testing.T, sb integration.Sandbox) { defer os.RemoveAll(destDir) out := filepath.Join(destDir, "out.tar") + outW, err := os.Create(out) + require.NoError(t, err) err = c.Solve(context.TODO(), def, SolveOpt{ - Exporter: ExporterOCI, - ExporterAttrs: map[string]string{ - "output": out, - }, + Exporter: ExporterOCI, + ExporterOutput: outW, }, nil) require.NoError(t, err) @@ -650,12 +642,11 @@ func testWhiteoutParentDir(t *testing.T, sb integration.Sandbox) { defer os.RemoveAll(destDir) out := filepath.Join(destDir, "out.tar") - + outW, err := os.Create(out) + require.NoError(t, err) err = c.Solve(context.TODO(), def, SolveOpt{ - Exporter: ExporterOCI, - ExporterAttrs: map[string]string{ - "output": out, - }, + Exporter: ExporterOCI, + ExporterOutput: outW, }, nil) require.NoError(t, err) diff --git a/client/exporters.go b/client/exporters.go index 2e9acb47e..4160d92a7 100644 --- a/client/exporters.go +++ b/client/exporters.go @@ -5,7 +5,4 @@ const ( ExporterLocal = "local" ExporterOCI = "oci" ExporterDocker = "docker" - - exporterLocalOutputDir = "output" - exporterOCIDestination = "output" ) diff --git a/client/solve.go b/client/solve.go index a6877af9a..1d8d81b92 100644 --- a/client/solve.go +++ b/client/solve.go @@ -8,7 +8,6 @@ import ( "strings" "time" - "github.com/containerd/console" controlapi "github.com/moby/buildkit/api/services/control" "github.com/moby/buildkit/client/llb" "github.com/moby/buildkit/identity" @@ -24,14 +23,16 @@ import ( ) type SolveOpt struct { - Exporter string - ExporterAttrs map[string]string - LocalDirs map[string]string - SharedKey string - Frontend string - FrontendAttrs map[string]string - ExportCache string - ImportCache string + Exporter string + ExporterAttrs map[string]string + ExporterOutput io.WriteCloser // for ExporterOCI and ExporterDocker + ExporterOutputDir string // for ExporterLocal + LocalDirs map[string]string + SharedKey string + Frontend string + FrontendAttrs map[string]string + ExportCache string + ImportCache string // Session string } @@ -79,28 +80,30 @@ func (c *Client) Solve(ctx context.Context, def *llb.Definition, opt SolveOpt, s switch opt.Exporter { case ExporterLocal: - outputDir, ok := opt.ExporterAttrs[exporterLocalOutputDir] - if !ok { - return errors.Errorf("output directory is required for local exporter") + if opt.ExporterOutput != nil { + logrus.Warnf("output file writer is ignored for local exporter") + } + // it is ok to have empty output dir (just ignored) + // FIXME(AkihiroSuda): maybe disallow empty output dir? (breaks integration tests) + if opt.ExporterOutputDir != "" { + s.Allow(filesync.NewFSSyncTargetDir(opt.ExporterOutputDir)) } - s.Allow(filesync.NewFSSyncTarget(outputDir)) case ExporterOCI, ExporterDocker: - outputFile, ok := opt.ExporterAttrs[exporterOCIDestination] - if ok { - fi, err := os.Stat(outputFile) - if err != nil && !os.IsNotExist(err) { - return errors.Wrapf(err, "invlid destination file: %s", outputFile) - } - if err == nil && fi.IsDir() { - return errors.Errorf("destination file is a directory") - } - } else { - if _, err := console.ConsoleFromFile(os.Stdout); err == nil { - return errors.Errorf("output file is required for %s exporter. refusing to write to console", opt.Exporter) - } - outputFile = "" + if opt.ExporterOutputDir != "" { + logrus.Warnf("output directory %s is ignored for %s exporter", opt.ExporterOutputDir, opt.Exporter) + } + // it is ok to have empty output file (just ignored) + // FIXME(AkihiroSuda): maybe disallow empty output file? (breaks integration tests) + if opt.ExporterOutput != nil { + s.Allow(filesync.NewFSSyncTarget(opt.ExporterOutput)) + } + default: + if opt.ExporterOutput != nil { + logrus.Warnf("output file writer is ignored for %s exporter", opt.Exporter) + } + if opt.ExporterOutputDir != "" { + logrus.Warnf("output directory %s is ignored for %s exporter", opt.ExporterOutputDir, opt.Exporter) } - s.Allow(filesync.NewFSSyncTargetFile(outputFile)) } eg.Go(func() error { diff --git a/cmd/buildctl/build.go b/cmd/buildctl/build.go index 8fe190387..5741019e3 100644 --- a/cmd/buildctl/build.go +++ b/cmd/buildctl/build.go @@ -117,17 +117,33 @@ func build(clicontext *cli.Context) error { displayCh := make(chan *client.SolveStatus) eg, ctx := errgroup.WithContext(commandContext(clicontext)) - exporterAttrs, err := attrMap(clicontext.StringSlice("exporter-opt")) + solveOpt := client.SolveOpt{ + Exporter: clicontext.String("exporter"), + // ExporterAttrs is set later + // LocalDirs is set later + Frontend: clicontext.String("frontend"), + // FrontendAttrs is set later + ExportCache: clicontext.String("export-cache"), + ImportCache: clicontext.String("import-cache"), + } + solveOpt.ExporterAttrs, err = attrMap(clicontext.StringSlice("exporter-opt")) if err != nil { return errors.Wrap(err, "invalid exporter-opt") } + solveOpt.ExporterOutput, solveOpt.ExporterOutputDir, err = resolveExporterOutput(solveOpt.Exporter, solveOpt.ExporterAttrs["output"]) + if err != nil { + return errors.Wrap(err, "invalid exporter-opt: output") + } + if solveOpt.ExporterOutput != nil || solveOpt.ExporterOutputDir != "" { + delete(solveOpt.ExporterAttrs, "output") + } - frontendAttrs, err := attrMap(clicontext.StringSlice("frontend-opt")) + solveOpt.FrontendAttrs, err = attrMap(clicontext.StringSlice("frontend-opt")) if err != nil { return errors.Wrap(err, "invalid frontend-opt") } - localDirs, err := attrMap(clicontext.StringSlice("local")) + solveOpt.LocalDirs, err = attrMap(clicontext.StringSlice("local")) if err != nil { return errors.Wrap(err, "invalid local") } @@ -145,15 +161,7 @@ func build(clicontext *cli.Context) error { } eg.Go(func() error { - return c.Solve(ctx, def, client.SolveOpt{ - Exporter: clicontext.String("exporter"), - ExporterAttrs: exporterAttrs, - LocalDirs: localDirs, - Frontend: clicontext.String("frontend"), - FrontendAttrs: frontendAttrs, - ExportCache: clicontext.String("export-cache"), - ImportCache: clicontext.String("import-cache"), - }, ch) + return c.Solve(ctx, def, solveOpt, ch) }) eg.Go(func() error { @@ -203,3 +211,34 @@ func attrMap(sl []string) (map[string]string, error) { } return m, nil } + +// resolveExporterOutput returns at most either one of io.WriteCloser (single file) or a string (directory path). +func resolveExporterOutput(exporter, output string) (io.WriteCloser, string, error) { + switch exporter { + case client.ExporterLocal: + // it is ok to have empty output dir (just ignored) + // FIXME(AkihiroSuda): maybe disallow empty output dir? (breaks integration tests) + return nil, output, nil + case client.ExporterOCI, client.ExporterDocker: + if output != "" { + fi, err := os.Stat(output) + if err != nil && !os.IsNotExist(err) { + return nil, "", errors.Wrapf(err, "invalid destination file: %s", output) + } + if err == nil && fi.IsDir() { + return nil, "", errors.Errorf("destination file is a directory") + } + w, err := os.Create(output) + return w, "", err + } + if _, err := console.ConsoleFromFile(os.Stdout); err == nil { + return nil, "", errors.Errorf("output file is required for %s exporter. refusing to write to console", exporter) + } + return os.Stdout, "", nil + default: // e.g. client.ExporterImage + if output != "" { + logrus.Warnf("output %s is ignored for %s exporter", output, exporter) + } + return nil, "", nil + } +} diff --git a/examples/build-using-dockerfile/main.go b/examples/build-using-dockerfile/main.go index 2b29bfe09..8f173dd60 100644 --- a/examples/build-using-dockerfile/main.go +++ b/examples/build-using-dockerfile/main.go @@ -3,7 +3,7 @@ package main import ( "context" "fmt" - "io/ioutil" + "io" "os" "os/exec" "path/filepath" @@ -73,12 +73,8 @@ func action(clicontext *cli.Context) error { if err != nil { return err } - tmpTar, err := ioutil.TempFile("", "buldkit-build-using-dockerfile") - if err != nil { - return err - } - defer os.Remove(tmpTar.Name()) - solveOpt, err := newSolveOpt(clicontext, tmpTar.Name()) + pipeR, pipeW := io.Pipe() + solveOpt, err := newSolveOpt(clicontext, pipeW) if err != nil { return err } @@ -94,18 +90,20 @@ func action(clicontext *cli.Context) error { } return nil }) + eg.Go(func() error { + if err := loadDockerTar(pipeR); err != nil { + return err + } + return pipeR.Close() + }) if err := eg.Wait(); err != nil { return err } - logrus.Infof("Loading the image to Docker as %q. This may take a while.", clicontext.String("tag")) - if err := loadDockerTar(tmpTar.Name()); err != nil { - return err - } - logrus.Info("Done") + logrus.Infof("Loaded the image %q to Docker.", clicontext.String("tag")) return nil } -func newSolveOpt(clicontext *cli.Context, tmpTar string) (*client.SolveOpt, error) { +func newSolveOpt(clicontext *cli.Context, w io.WriteCloser) (*client.SolveOpt, error) { buildCtx := clicontext.Args().First() if buildCtx == "" { return nil, errors.New("please specify build context (e.g. \".\" for the current directory)") @@ -139,18 +137,19 @@ func newSolveOpt(clicontext *cli.Context, tmpTar string) (*client.SolveOpt, erro return &client.SolveOpt{ Exporter: "docker", // TODO: use containerd image store when it is integrated to Docker ExporterAttrs: map[string]string{ - "name": clicontext.String("tag"), - "output": tmpTar, + "name": clicontext.String("tag"), }, - LocalDirs: localDirs, - Frontend: "dockerfile.v0", // TODO: use gateway - FrontendAttrs: frontendAttrs, + ExporterOutput: w, + LocalDirs: localDirs, + Frontend: "dockerfile.v0", // TODO: use gateway + FrontendAttrs: frontendAttrs, }, nil } -func loadDockerTar(tar string) error { +func loadDockerTar(r io.Reader) error { // no need to use moby/moby/client here - cmd := exec.Command("docker", "load", "-i", tar) + cmd := exec.Command("docker", "load") + cmd.Stdin = r cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr return cmd.Run() diff --git a/frontend/dockerfile/dockerfile_test.go b/frontend/dockerfile/dockerfile_test.go index bf06480af..4198fbab9 100644 --- a/frontend/dockerfile/dockerfile_test.go +++ b/frontend/dockerfile/dockerfile_test.go @@ -547,11 +547,9 @@ Dockerfile defer os.RemoveAll(destDir) err = c.Solve(context.TODO(), nil, client.SolveOpt{ - Frontend: "dockerfile.v0", - Exporter: client.ExporterLocal, - ExporterAttrs: map[string]string{ - "output": destDir, - }, + Frontend: "dockerfile.v0", + Exporter: client.ExporterLocal, + ExporterOutputDir: destDir, LocalDirs: map[string]string{ builder.LocalNameDockerfile: dir, builder.LocalNameContext: dir, @@ -687,11 +685,9 @@ USER nobody defer os.RemoveAll(destDir) err = c.Solve(context.TODO(), nil, client.SolveOpt{ - Frontend: "dockerfile.v0", - Exporter: client.ExporterLocal, - ExporterAttrs: map[string]string{ - "output": destDir, - }, + Frontend: "dockerfile.v0", + Exporter: client.ExporterLocal, + ExporterOutputDir: destDir, LocalDirs: map[string]string{ builder.LocalNameDockerfile: dir, builder.LocalNameContext: dir, @@ -785,11 +781,9 @@ COPY --from=base /out / defer os.RemoveAll(destDir) err = c.Solve(context.TODO(), nil, client.SolveOpt{ - Frontend: "dockerfile.v0", - Exporter: client.ExporterLocal, - ExporterAttrs: map[string]string{ - "output": destDir, - }, + Frontend: "dockerfile.v0", + Exporter: client.ExporterLocal, + ExporterOutputDir: destDir, LocalDirs: map[string]string{ builder.LocalNameDockerfile: dir, builder.LocalNameContext: dir, @@ -839,11 +833,9 @@ COPY files dest defer os.RemoveAll(destDir) err = c.Solve(context.TODO(), nil, client.SolveOpt{ - Frontend: "dockerfile.v0", - Exporter: client.ExporterLocal, - ExporterAttrs: map[string]string{ - "output": destDir, - }, + Frontend: "dockerfile.v0", + Exporter: client.ExporterLocal, + ExporterOutputDir: destDir, LocalDirs: map[string]string{ builder.LocalNameDockerfile: dir, builder.LocalNameContext: dir, @@ -897,11 +889,9 @@ COPY sub/dir1 subdest6 defer os.RemoveAll(destDir) err = c.Solve(context.TODO(), nil, client.SolveOpt{ - Frontend: "dockerfile.v0", - Exporter: client.ExporterLocal, - ExporterAttrs: map[string]string{ - "output": destDir, - }, + Frontend: "dockerfile.v0", + Exporter: client.ExporterLocal, + ExporterOutputDir: destDir, LocalDirs: map[string]string{ builder.LocalNameDockerfile: dir, builder.LocalNameContext: dir, @@ -1007,10 +997,8 @@ COPY --from=build foo bar2 FrontendAttrs: map[string]string{ "context": "git://" + server.URL + "/#first", }, - Exporter: client.ExporterLocal, - ExporterAttrs: map[string]string{ - "output": destDir, - }, + Exporter: client.ExporterLocal, + ExporterOutputDir: destDir, }, nil) require.NoError(t, err) @@ -1032,10 +1020,8 @@ COPY --from=build foo bar2 FrontendAttrs: map[string]string{ "context": "git://" + server.URL + "/", }, - Exporter: client.ExporterLocal, - ExporterAttrs: map[string]string{ - "output": destDir, - }, + Exporter: client.ExporterLocal, + ExporterOutputDir: destDir, }, nil) require.NoError(t, err) @@ -1071,11 +1057,9 @@ COPY --from=busybox /etc/passwd test defer os.RemoveAll(destDir) err = c.Solve(context.TODO(), nil, client.SolveOpt{ - Frontend: "dockerfile.v0", - Exporter: client.ExporterLocal, - ExporterAttrs: map[string]string{ - "output": destDir, - }, + Frontend: "dockerfile.v0", + Exporter: client.ExporterLocal, + ExporterOutputDir: destDir, LocalDirs: map[string]string{ builder.LocalNameDockerfile: dir, builder.LocalNameContext: dir, @@ -1108,11 +1092,9 @@ COPY --from=golang /usr/bin/go go defer os.RemoveAll(destDir) err = c.Solve(context.TODO(), nil, client.SolveOpt{ - Frontend: "dockerfile.v0", - Exporter: client.ExporterLocal, - ExporterAttrs: map[string]string{ - "output": destDir, - }, + Frontend: "dockerfile.v0", + Exporter: client.ExporterLocal, + ExporterOutputDir: destDir, LocalDirs: map[string]string{ builder.LocalNameDockerfile: dir, builder.LocalNameContext: dir, diff --git a/session/filesync/filesync.go b/session/filesync/filesync.go index b9ce4d636..232a696d7 100644 --- a/session/filesync/filesync.go +++ b/session/filesync/filesync.go @@ -212,25 +212,25 @@ func FSSync(ctx context.Context, c session.Caller, opt FSSendRequestOpt) error { return pr.recvFn(stream, opt.DestDir, opt.CacheUpdater, opt.ProgressCb) } -// NewFSSyncTarget allows writing into a directory -func NewFSSyncTarget(outdir string) session.Attachable { +// NewFSSyncTargetDir allows writing into a directory +func NewFSSyncTargetDir(outdir string) session.Attachable { p := &fsSyncTarget{ outdir: outdir, } return p } -// NewFSSyncTargetFile allows writing into a file. Empty file means stdout -func NewFSSyncTargetFile(outfile string) session.Attachable { +// NewFSSyncTarget allows writing into an io.WriteCloser +func NewFSSyncTarget(w io.WriteCloser) session.Attachable { p := &fsSyncTarget{ - outfile: outfile, + outfile: w, } return p } type fsSyncTarget struct { outdir string - outfile string + outfile io.WriteCloser } func (sp *fsSyncTarget) Register(server *grpc.Server) { @@ -241,18 +241,11 @@ func (sp *fsSyncTarget) DiffCopy(stream FileSend_DiffCopyServer) error { if sp.outdir != "" { return syncTargetDiffCopy(stream, sp.outdir) } - var f *os.File - if sp.outfile == "" { - f = os.Stdout - } else { - var err error - f, err = os.Create(sp.outfile) - if err != nil { - return err - } + if sp.outfile == nil { + return errors.New("empty outfile and outdir") } - defer f.Close() - return writeTargetFile(stream, f) + defer sp.outfile.Close() + return writeTargetFile(stream, sp.outfile) } func CopyToCaller(ctx context.Context, srcPath string, c session.Caller, progress func(int, bool)) error {