Build: Support for printing outline/targets of the current build #1100
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,6 +39,7 @@ import ( | |
| gateway "github.com/moby/buildkit/frontend/gateway/client" | ||
| "github.com/moby/buildkit/session" | ||
| "github.com/moby/buildkit/session/upload/uploadprovider" | ||
| "github.com/moby/buildkit/solver/errdefs" | ||
| "github.com/moby/buildkit/solver/pb" | ||
| "github.com/moby/buildkit/util/apicaps" | ||
| "github.com/moby/buildkit/util/entitlements" | ||
|
|
@@ -57,6 +58,10 @@ var ( | |
| errDockerfileConflict = errors.New("ambiguous Dockerfile source: both stdin and flag correspond to Dockerfiles") | ||
| ) | ||
|
|
||
| const ( | ||
| printFallbackImage = "docker/dockerfile-upstream:1.4-outline@sha256:ccd574ab34a8875c64bb6a8fb3cfae2e6d62d31b28b9f688075cc14c9b669a59" | ||
| ) | ||
|
|
||
| type Options struct { | ||
| Inputs Inputs | ||
|
|
||
|
|
@@ -81,7 +86,13 @@ type Options struct { | |
| Ulimits *opts.UlimitOpt | ||
|
|
||
| // Linked marks this target as exclusively linked (not requested by the user). | ||
| Linked bool | ||
| PrintFunc *PrintFunc | ||
| } | ||
|
|
||
| type PrintFunc struct { | ||
| Name string | ||
| Format string | ||
| } | ||
|
|
||
| type Inputs struct { | ||
|
|
@@ -730,7 +741,7 @@ func BuildWithResultHandler(ctx context.Context, drivers []DriverInfo, opt map[s | |
| } | ||
| } | ||
|
|
||
| if noMobyDriver != nil && !noDefaultLoad() && noPrintFunc(opt) { | ||
| var noOutputTargets []string | ||
| for name, opt := range opt { | ||
| if !opt.Linked && len(opt.Exports) == 0 { | ||
|
|
@@ -1039,22 +1050,69 @@ func BuildWithResultHandler(ctx context.Context, drivers []DriverInfo, opt map[s | |
| defer func() { <-done }() | ||
|
|
||
| cc := c | ||
| var printRes map[string][]byte | ||
| rr, err := c.Build(ctx, so, "buildx", func(ctx context.Context, c gateway.Client) (*gateway.Result, error) { | ||
| var isFallback bool | ||
| var origErr error | ||
| for { | ||
| if opt.PrintFunc != nil { | ||
| if _, ok := req.FrontendOpt["frontend.caps"]; !ok { | ||
| req.FrontendOpt["frontend.caps"] = "moby.buildkit.frontend.subrequests+forward" | ||
|
There was a problem hiding this comment. Think it would be nice to have consts for the subrequest caps on the buildkit side for these caps similar to solver/pb/caps.go There was a problem hiding this comment. This should be defined in some package that defines keys for Dockerfile frontend compatibility interface. https://github.com/moby/buildkit/blob/master/frontend/dockerfile/builder/build.go#L43-L47 Unfortunately no such package exists atm |
||
| } else { | ||
| req.FrontendOpt["frontend.caps"] += ",moby.buildkit.frontend.subrequests+forward" | ||
| } | ||
| req.FrontendOpt["requestid"] = "frontend." + opt.PrintFunc.Name | ||
| if isFallback { | ||
| req.FrontendOpt["build-arg:BUILDKIT_SYNTAX"] = printFallbackImage | ||
| } | ||
| } | ||
| res, err := c.Solve(ctx, req) | ||
| if err != nil { | ||
| if origErr != nil { | ||
| return nil, err | ||
| } | ||
| var reqErr *errdefs.UnsupportedSubrequestError | ||
| if !isFallback { | ||
| if errors.As(err, &reqErr) { | ||
| switch reqErr.Name { | ||
| case "frontend.outline", "frontend.targets": | ||
| isFallback = true | ||
| origErr = err | ||
| continue | ||
| } | ||
| return nil, err | ||
| } | ||
| // buildkit v0.8 vendored in Docker 20.10 does not support typed errors | ||
| if strings.Contains(err.Error(), "unsupported request frontend.outline") || strings.Contains(err.Error(), "unsupported request frontend.targets") { | ||
| isFallback = true | ||
| origErr = err | ||
| continue | ||
| } | ||
| } | ||
| return nil, err | ||
| } | ||
| if opt.PrintFunc != nil { | ||
| printRes = res.Metadata | ||
| } | ||
| results.Set(resultKey(dp.driverIndex, k), res) | ||
| if resultHandleFunc != nil { | ||
| resultHandleFunc(dp.driverIndex, &ResultContext{cc, res}) | ||
| } | ||
| return res, nil | ||
| } | ||
| }, ch) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| res[i] = rr | ||
|
|
||
| if rr.ExporterResponse == nil { | ||
| rr.ExporterResponse = map[string]string{} | ||
| } | ||
| for k, v := range printRes { | ||
| rr.ExporterResponse[k] = string(v) | ||
|
There was a problem hiding this comment. Because we store the values in here, are they going to be output to the metadata file as well? I think that makes sense. I wonder if we need to do something here to make sure we don't write to any known keys here, if the frontend's returned Metadata contains a duplicate key for whatever reason, we might potentially corrupt some data here. Maybe it's worth keeping this info separate with a composite struct? There was a problem hiding this comment. Actually, I wonder if we could do something similar to annotations, where we have the prefix There was a problem hiding this comment.
Yes.
How can it be duplicated if it is a map?
I don't think that would be right. The point of subrequests is to take a different code path and return results for it, so it is not "do build and also return extra keys that would need to be namespace". Oth, subrequest build does not mean that it needs to return json/text. A subrequest could for example do a build with a different configuration and return a different image. In that case, it does want to return the correct image keys so they get exported correctly, not namespaced special keys. There was a problem hiding this comment. Sorry, I meant duplicated values between Agreed though, thanks for the clarification on subrequests. |
||
| } | ||
|
|
||
| d := drivers[dp.driverIndex].Driver | ||
| if d.IsMobyDriver() { | ||
| for _, e := range so.Exports { | ||
|
|
@@ -1630,3 +1688,12 @@ func tryNodeIdentifier(configDir string) (out string) { | |
| } | ||
| return | ||
| } | ||
|
|
||
| func noPrintFunc(opt map[string]Options) bool { | ||
| for _, v := range opt { | ||
| if v.PrintFunc != nil { | ||
| return false | ||
| } | ||
| } | ||
| return true | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,6 +48,7 @@ const defaultTargetName = "default" | |
| type buildOptions struct { | ||
| contextPath string | ||
| dockerfileName string | ||
| printFunc string | ||
|
|
||
| allow []string | ||
| buildArgs []string | ||
|
|
@@ -122,6 +123,11 @@ func runBuild(dockerCli command.Cli, in buildOptions) (err error) { | |
| return err | ||
| } | ||
|
|
||
| printFunc, err := parsePrintFunc(in.printFunc) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| opts := build.Options{ | ||
| Inputs: build.Inputs{ | ||
| ContextPath: in.contextPath, | ||
|
|
@@ -141,6 +147,7 @@ func runBuild(dockerCli command.Cli, in buildOptions) (err error) { | |
| Tags: in.tags, | ||
| Target: in.target, | ||
| Ulimits: in.ulimits, | ||
| PrintFunc: printFunc, | ||
| } | ||
|
|
||
| platforms, err := platformutil.Parse(in.platforms) | ||
|
|
@@ -307,6 +314,14 @@ func buildTargets(ctx context.Context, dockerCli command.Cli, opts map[string]bu | |
|
|
||
| printWarnings(os.Stderr, printer.Warnings(), progressMode) | ||
|
|
||
| for k := range resp { | ||
| if opts[k].PrintFunc != nil { | ||
| if err := printResult(opts[k].PrintFunc, resp[k].ExporterResponse); err != nil { | ||
| return "", nil, err | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return resp[defaultTargetName].ExporterResponse["containerimage.digest"], res, err | ||
| } | ||
|
|
||
|
|
@@ -463,6 +478,10 @@ func buildCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { | |
|
|
||
| flags.StringArrayVar(&options.platforms, "platform", platformsDefault, "Set target platform for build") | ||
|
|
||
| if isExperimental() { | ||
| flags.StringVar(&options.printFunc, "print", "", "Print result of information request (outline, targets)") | ||
tonistiigi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| flags.BoolVar(&options.exportPush, "push", false, `Shorthand for "--output=type=registry"`) | ||
|
|
||
| flags.BoolVarP(&options.quiet, "quiet", "q", false, "Suppress the build output and print image ID on success") | ||
|
|
@@ -481,7 +500,7 @@ func buildCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { | |
|
|
||
| flags.Var(options.ulimits, "ulimit", "Ulimit options") | ||
|
|
||
| if isExperimental() { | ||
| flags.StringVar(&options.invoke, "invoke", "", "Invoke a command after the build. BUILDX_EXPERIMENTAL=1 is required.") | ||
| } | ||
|
|
||
|
|
@@ -596,6 +615,34 @@ func parseContextNames(values []string) (map[string]build.NamedContext, error) { | |
| return result, nil | ||
| } | ||
|
|
||
| func parsePrintFunc(str string) (*build.PrintFunc, error) { | ||
| if str == "" { | ||
| return nil, nil | ||
| } | ||
| csvReader := csv.NewReader(strings.NewReader(str)) | ||
| fields, err := csvReader.Read() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| f := &build.PrintFunc{} | ||
| for _, field := range fields { | ||
| parts := strings.SplitN(field, "=", 2) | ||
| if len(parts) == 2 { | ||
| if parts[0] == "format" { | ||
|
There was a problem hiding this comment. This feels a bit confusing to me since we already have the There was a problem hiding this comment. This was made to match #1100 (comment) |
||
| f.Format = parts[1] | ||
| } else { | ||
| return nil, errors.Errorf("invalid print field: %s", field) | ||
| } | ||
| } else { | ||
| if f.Name != "" { | ||
| return nil, errors.Errorf("invalid print value: %s", str) | ||
| } | ||
| f.Name = field | ||
| } | ||
| } | ||
| return f, nil | ||
| } | ||
|
|
||
| func writeMetadataFile(filename string, dt interface{}) error { | ||
| b, err := json.MarshalIndent(dt, "", " ") | ||
| if err != nil { | ||
|
|
@@ -652,3 +699,11 @@ func (w *wrapped) Error() string { | |
| func (w *wrapped) Unwrap() error { | ||
| return w.err | ||
| } | ||
|
|
||
| func isExperimental() bool { | ||
| if v, ok := os.LookupEnv("BUILDKIT_EXPERIMENTAL"); ok { | ||
| vv, _ := strconv.ParseBool(v) | ||
| return vv | ||
| } | ||
| return false | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| package commands | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "io" | ||
| "log" | ||
| "os" | ||
|
|
||
| "github.com/docker/buildx/build" | ||
| "github.com/docker/docker/api/types/versions" | ||
| "github.com/moby/buildkit/frontend/subrequests" | ||
| "github.com/moby/buildkit/frontend/subrequests/outline" | ||
| "github.com/moby/buildkit/frontend/subrequests/targets" | ||
| ) | ||
|
|
||
| func printResult(f *build.PrintFunc, res map[string]string) error { | ||
| switch f.Name { | ||
| case "outline": | ||
| return printValue(outline.PrintOutline, outline.SubrequestsOutlineDefinition.Version, f.Format, res) | ||
| case "targets": | ||
| return printValue(targets.PrintTargets, targets.SubrequestsTargetsDefinition.Version, f.Format, res) | ||
| case "subrequests.describe": | ||
| return printValue(subrequests.PrintDescribe, subrequests.SubrequestsDescribeDefinition.Version, f.Format, res) | ||
| default: | ||
| if dt, ok := res["result.txt"]; ok { | ||
| fmt.Print(dt) | ||
| } else { | ||
| log.Printf("%s %+v", f, res) | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| type printFunc func([]byte, io.Writer) error | ||
|
|
||
| func printValue(printer printFunc, version string, format string, res map[string]string) error { | ||
| if format == "json" { | ||
| fmt.Fprintln(os.Stdout, res["result.json"]) | ||
|
There was a problem hiding this comment. I think we should have There was a problem hiding this comment. This is not hardcoded anywhere but defined in the subrequest definition that frontend shows. If handling a specific one, client should look what outputs it has from the description. This is a generic "best-effort" because we let the user specify any function name/ There was a problem hiding this comment.
Could you use the Lines 607 to 623 in 98f9f80 There was a problem hiding this comment. A source is the original source file that we don't control. It can be any kind of bytes. I think we shouldn't make assumptions about this. Even if it is a Dockerfile, showing some encoded string in here for the whole big file would not make it clearer and the user would need to do some decoding from a string to try to get the original form. There was a problem hiding this comment. This is not encoded JSON so |
||
| return nil | ||
| } | ||
|
|
||
| if res["version"] != "" && versions.LessThan(version, res["version"]) && res["result.txt"] != "" { | ||
| // structure is too new and we don't know how to print it | ||
| fmt.Fprint(os.Stdout, res["result.txt"]) | ||
| return nil | ||
| } | ||
| return printer([]byte(res["result.json"]), os.Stdout) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if we should use a loop here to do the retries? As I understand it, it's so that we can have a second try with a fallback, but that might be easier to read in-serial, instead of with the loop/continue which is hard to reason about control flow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I understand. Without the loop, a bunch of this code would need to be duplicated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking we could try and move the logic into a separate function/method, as a generic
makeRequestor something function, then we could use a bool return to check if it was successful, and if not, we could make a fallback call to the same function, which would keep the logic needed inBuildWithResultHandlerto a minimum.