Skip to content

Commit e826141

Browse files
jedevcjedevc
committed
controller: refactor progress api
Refactor the progress printer creation to the caller-side of the controller api. Then, instead of passing around status channels (and progressMode strings), we can simply pass around the higher level interface progress.Writer. This has a couple of benefits: - A simplified interface to the controller - Allows us to correctly extract warnings out of the controller, so that they can be displayed correctly from the client side. Some extra work is required to make sure that we can pass a progress.Printer into the debug monitor. If we want to keep it persistent, then we need a way to temporarily suspend output from it, otherwise it will continue printing as the monitor is prompting for input from the user, and forwarding output from debug containers. To handle this, we add two methods to the printer, `Pause` and `Unpause`. `Pause` acts similarly to `Wait`, closing the printer, and cleanly shutting down the display - however, the printer does not terminate, and can later be resumed by a call to `Unpause`. This provides a neater interface to the caller, instead of needing to continually reconstruct printers for every single time we want to produce progress output. Signed-off-by: Justin Chadwell <[email protected]>
1 parent 0c1fd31 commit e826141

11 files changed

Lines changed: 190 additions & 122 deletions

File tree

‎commands/build.go‎

Lines changed: 78 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package commands
22

33
import (
4+
"bytes"
45
"context"
56
"encoding/base64"
67
"encoding/csv"
@@ -15,6 +16,7 @@ import (
1516

1617
"github.com/containerd/console"
1718
"github.com/docker/buildx/build"
19+
"github.com/docker/buildx/builder"
1820
"github.com/docker/buildx/controller"
1921
cbuild "github.com/docker/buildx/controller/build"
2022
"github.com/docker/buildx/controller/control"
@@ -35,8 +37,11 @@ import (
3537
"github.com/docker/docker/pkg/ioutils"
3638
"github.com/moby/buildkit/client"
3739
"github.com/moby/buildkit/exporter/containerimage/exptypes"
40+
"github.com/moby/buildkit/solver/errdefs"
3841
"github.com/moby/buildkit/util/appcontext"
3942
"github.com/moby/buildkit/util/grpcerrors"
43+
"github.com/moby/buildkit/util/progress/progressui"
44+
"github.com/morikuni/aec"
4045
"github.com/pkg/errors"
4146
"github.com/sirupsen/logrus"
4247
"github.com/spf13/cobra"
@@ -202,18 +207,44 @@ func runBuild(dockerCli command.Cli, options buildOptions) (err error) {
202207
}
203208
}
204209

210+
contextPathHash := options.contextPath
211+
if absContextPath, err := filepath.Abs(contextPathHash); err == nil {
212+
contextPathHash = absContextPath
213+
}
214+
b, err := builder.New(dockerCli,
215+
builder.WithName(options.builder),
216+
builder.WithContextPathHash(contextPathHash),
217+
)
218+
if err != nil {
219+
return err
220+
}
221+
222+
ctx2, cancel := context.WithCancel(context.TODO())
223+
defer cancel()
205224
progressMode, err := options.toProgress()
206225
if err != nil {
207226
return err
208227
}
228+
printer, err := progress.NewPrinter(ctx2, os.Stderr, os.Stderr, progressMode, progressui.WithDesc(
229+
fmt.Sprintf("building with %q instance using %s driver", b.Name, b.Driver),
230+
fmt.Sprintf("%s:%s", b.Driver, b.Name),
231+
))
232+
if err != nil {
233+
return err
234+
}
209235

210236
var resp *client.SolveResponse
211237
var retErr error
212238
if isExperimental() {
213-
resp, retErr = runControllerBuild(ctx, dockerCli, options, progressMode)
239+
resp, retErr = runControllerBuild(ctx, dockerCli, options, printer)
214240
} else {
215-
resp, retErr = runBasicBuild(ctx, dockerCli, options, progressMode)
241+
resp, retErr = runBasicBuild(ctx, dockerCli, options, printer)
242+
}
243+
244+
if err := printer.Wait(); retErr == nil {
245+
retErr = err
216246
}
247+
printWarnings(os.Stderr, printer.Warnings(), progressMode)
217248
if retErr != nil {
218249
return retErr
219250
}
@@ -232,17 +263,17 @@ func runBuild(dockerCli command.Cli, options buildOptions) (err error) {
232263
return nil
233264
}
234265

235-
func runBasicBuild(ctx context.Context, dockerCli command.Cli, options buildOptions, progressMode string) (*client.SolveResponse, error) {
266+
func runBasicBuild(ctx context.Context, dockerCli command.Cli, options buildOptions, printer *progress.Printer) (*client.SolveResponse, error) {
236267
opts, err := options.toControllerOptions()
237268
if err != nil {
238269
return nil, err
239270
}
240271

241-
resp, _, err := cbuild.RunBuild(ctx, dockerCli, *opts, os.Stdin, progressMode, nil, false)
272+
resp, _, err := cbuild.RunBuild(ctx, dockerCli, *opts, os.Stdin, printer, false)
242273
return resp, err
243274
}
244275

245-
func runControllerBuild(ctx context.Context, dockerCli command.Cli, options buildOptions, progressMode string) (*client.SolveResponse, error) {
276+
func runControllerBuild(ctx context.Context, dockerCli command.Cli, options buildOptions, printer *progress.Printer) (*client.SolveResponse, error) {
246277
if options.invoke != nil && (options.dockerfileName == "-" || options.contextPath == "-") {
247278
// stdin must be usable for monitor
248279
return nil, errors.Errorf("Dockerfile or context from stdin is not supported with invoke")
@@ -284,7 +315,7 @@ func runControllerBuild(ctx context.Context, dockerCli command.Cli, options buil
284315
return nil
285316
})
286317

287-
ref, resp, err = c.Build(ctx, *opts, pr, os.Stdout, os.Stderr, progressMode)
318+
ref, resp, err = c.Build(ctx, *opts, pr, printer)
288319
if err != nil {
289320
var be *controllererrors.BuildError
290321
if errors.As(err, &be) {
@@ -318,7 +349,7 @@ func runControllerBuild(ctx context.Context, dockerCli command.Cli, options buil
318349
}
319350
return nil, errors.Errorf("failed to configure terminal: %v", err)
320351
}
321-
err = monitor.RunMonitor(ctx, ref, opts, options.invoke.InvokeConfig, c, progressMode, pr2, os.Stdout, os.Stderr)
352+
err = monitor.RunMonitor(ctx, ref, opts, options.invoke.InvokeConfig, c, pr2, os.Stdout, os.Stderr, printer)
322353
con.Reset()
323354
if err := pw2.Close(); err != nil {
324355
logrus.Debug("failed to close monitor stdin pipe reader")
@@ -869,3 +900,43 @@ func resolvePaths(options *controllerapi.BuildOptions) (_ *controllerapi.BuildOp
869900

870901
return options, nil
871902
}
903+
904+
func printWarnings(w io.Writer, warnings []client.VertexWarning, mode string) {
905+
if len(warnings) == 0 || mode == progress.PrinterModeQuiet {
906+
return
907+
}
908+
fmt.Fprintf(w, "\n ")
909+
sb := &bytes.Buffer{}
910+
if len(warnings) == 1 {
911+
fmt.Fprintf(sb, "1 warning found")
912+
} else {
913+
fmt.Fprintf(sb, "%d warnings found", len(warnings))
914+
}
915+
if logrus.GetLevel() < logrus.DebugLevel {
916+
fmt.Fprintf(sb, " (use --debug to expand)")
917+
}
918+
fmt.Fprintf(sb, ":\n")
919+
fmt.Fprint(w, aec.Apply(sb.String(), aec.YellowF))
920+
921+
for _, warn := range warnings {
922+
fmt.Fprintf(w, " - %s\n", warn.Short)
923+
if logrus.GetLevel() < logrus.DebugLevel {
924+
continue
925+
}
926+
for _, d := range warn.Detail {
927+
fmt.Fprintf(w, "%s\n", d)
928+
}
929+
if warn.URL != "" {
930+
fmt.Fprintf(w, "More info: %s\n", warn.URL)
931+
}
932+
if warn.SourceInfo != nil && warn.Range != nil {
933+
src := errdefs.Source{
934+
Info: warn.SourceInfo,
935+
Ranges: warn.Range,
936+
}
937+
src.Print(w)
938+
}
939+
fmt.Fprintf(w, "\n")
940+
941+
}
942+
}

‎commands/debug-shell.go‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/docker/buildx/controller/control"
1111
controllerapi "github.com/docker/buildx/controller/pb"
1212
"github.com/docker/buildx/monitor"
13+
"github.com/docker/buildx/util/progress"
1314
"github.com/docker/cli/cli/command"
1415
"github.com/pkg/errors"
1516
"github.com/sirupsen/logrus"
@@ -18,7 +19,7 @@ import (
1819

1920
func debugShellCmd(dockerCli command.Cli) *cobra.Command {
2021
var options control.ControlOptions
21-
var progress string
22+
var progressMode string
2223

2324
cmd := &cobra.Command{
2425
Use: "debug-shell",
@@ -38,9 +39,15 @@ func debugShellCmd(dockerCli command.Cli) *cobra.Command {
3839
if err := con.SetRaw(); err != nil {
3940
return errors.Errorf("failed to configure terminal: %v", err)
4041
}
42+
43+
printer, err := progress.NewPrinter(context.TODO(), os.Stderr, os.Stderr, progressMode)
44+
if err != nil {
45+
return err
46+
}
47+
4148
err = monitor.RunMonitor(ctx, "", nil, controllerapi.InvokeConfig{
4249
Tty: true,
43-
}, c, progress, os.Stdin, os.Stdout, os.Stderr)
50+
}, c, os.Stdin, os.Stdout, os.Stderr, printer)
4451
con.Reset()
4552
return err
4653
},
@@ -51,7 +58,7 @@ func debugShellCmd(dockerCli command.Cli) *cobra.Command {
5158
flags.StringVar(&options.Root, "root", "", "Specify root directory of server to connect [experimental]")
5259
flags.BoolVar(&options.Detach, "detach", runtime.GOOS == "linux", "Detach buildx server (supported only on linux) [experimental]")
5360
flags.StringVar(&options.ServerConfig, "server-config", "", "Specify buildx server config file (used only when launching new server) [experimental]")
54-
flags.StringVar(&progress, "progress", "auto", `Set type of progress output ("auto", "plain", "tty"). Use plain to show container output`)
61+
flags.StringVar(&progressMode, "progress", "auto", `Set type of progress output ("auto", "plain", "tty"). Use plain to show container output`)
5562

5663
return cmd
5764
}

‎controller/build/build.go‎

Lines changed: 7 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
11
package build
22

33
import (
4-
"bytes"
54
"context"
65
"encoding/base64"
76
"encoding/csv"
87
"encoding/json"
9-
"fmt"
108
"io"
119
"os"
1210
"path/filepath"
@@ -30,12 +28,8 @@ import (
3028
"github.com/docker/go-units"
3129
"github.com/moby/buildkit/client"
3230
"github.com/moby/buildkit/session/auth/authprovider"
33-
"github.com/moby/buildkit/solver/errdefs"
3431
"github.com/moby/buildkit/util/grpcerrors"
35-
"github.com/moby/buildkit/util/progress/progressui"
36-
"github.com/morikuni/aec"
3732
"github.com/pkg/errors"
38-
"github.com/sirupsen/logrus"
3933
"google.golang.org/grpc/codes"
4034
)
4135

@@ -46,7 +40,7 @@ const defaultTargetName = "default"
4640
// NOTE: When an error happens during the build and this function acquires the debuggable *build.ResultContext,
4741
// this function returns it in addition to the error (i.e. it does "return nil, res, err"). The caller can
4842
// inspect the result and debug the cause of that error.
49-
func RunBuild(ctx context.Context, dockerCli command.Cli, in controllerapi.BuildOptions, inStream io.Reader, progressMode string, statusChan chan *client.SolveStatus, generateResult bool) (*client.SolveResponse, *build.ResultContext, error) {
43+
func RunBuild(ctx context.Context, dockerCli command.Cli, in controllerapi.BuildOptions, inStream io.Reader, progress progress.Writer, generateResult bool) (*client.SolveResponse, *build.ResultContext, error) {
5044
if in.NoCache && len(in.NoCacheFilter) > 0 {
5145
return nil, nil, errors.Errorf("--no-cache and --no-cache-filter cannot currently be used together")
5246
}
@@ -164,6 +158,7 @@ func RunBuild(ctx context.Context, dockerCli command.Cli, in controllerapi.Build
164158
contextPathHash = in.ContextPath
165159
}
166160

161+
// TODO: this should not be loaded this side of the controller api
167162
b, err := builder.New(dockerCli,
168163
builder.WithName(in.Builder),
169164
builder.WithContextPathHash(contextPathHash),
@@ -179,7 +174,7 @@ func RunBuild(ctx context.Context, dockerCli command.Cli, in controllerapi.Build
179174
return nil, nil, err
180175
}
181176

182-
resp, res, err := buildTargets(ctx, dockerCli, b.NodeGroup, nodes, map[string]build.Options{defaultTargetName: opts}, progressMode, in.MetadataFile, statusChan, generateResult)
177+
resp, res, err := buildTargets(ctx, dockerCli, b.NodeGroup, nodes, map[string]build.Options{defaultTargetName: opts}, progress, in.MetadataFile, generateResult)
183178
err = wrapBuildError(err, false)
184179
if err != nil {
185180
// NOTE: buildTargets can return *build.ResultContext even on error.
@@ -193,36 +188,22 @@ func RunBuild(ctx context.Context, dockerCli command.Cli, in controllerapi.Build
193188
// NOTE: When an error happens during the build and this function acquires the debuggable *build.ResultContext,
194189
// this function returns it in addition to the error (i.e. it does "return nil, res, err"). The caller can
195190
// inspect the result and debug the cause of that error.
196-
func buildTargets(ctx context.Context, dockerCli command.Cli, ng *store.NodeGroup, nodes []builder.Node, opts map[string]build.Options, progressMode string, metadataFile string, statusChan chan *client.SolveStatus, generateResult bool) (*client.SolveResponse, *build.ResultContext, error) {
197-
ctx2, cancel := context.WithCancel(context.TODO())
198-
defer cancel()
199-
200-
printer, err := progress.NewPrinter(ctx2, os.Stderr, os.Stderr, progressMode, progressui.WithDesc(
201-
fmt.Sprintf("building with %q instance using %s driver", ng.Name, ng.Driver),
202-
fmt.Sprintf("%s:%s", ng.Driver, ng.Name),
203-
))
204-
if err != nil {
205-
return nil, nil, err
206-
}
207-
191+
func buildTargets(ctx context.Context, dockerCli command.Cli, ng *store.NodeGroup, nodes []builder.Node, opts map[string]build.Options, progress progress.Writer, metadataFile string, generateResult bool) (*client.SolveResponse, *build.ResultContext, error) {
208192
var res *build.ResultContext
209193
var resp map[string]*client.SolveResponse
194+
var err error
210195
if generateResult {
211196
var mu sync.Mutex
212197
var idx int
213-
resp, err = build.BuildWithResultHandler(ctx, nodes, opts, dockerutil.NewClient(dockerCli), confutil.ConfigDir(dockerCli), progress.Tee(printer, statusChan), func(driverIndex int, gotRes *build.ResultContext) {
198+
resp, err = build.BuildWithResultHandler(ctx, nodes, opts, dockerutil.NewClient(dockerCli), confutil.ConfigDir(dockerCli), progress, func(driverIndex int, gotRes *build.ResultContext) {
214199
mu.Lock()
215200
defer mu.Unlock()
216201
if res == nil || driverIndex < idx {
217202
idx, res = driverIndex, gotRes
218203
}
219204
})
220205
} else {
221-
resp, err = build.Build(ctx, nodes, opts, dockerutil.NewClient(dockerCli), confutil.ConfigDir(dockerCli), progress.Tee(printer, statusChan))
222-
}
223-
err1 := printer.Wait()
224-
if err == nil {
225-
err = err1
206+
resp, err = build.Build(ctx, nodes, opts, dockerutil.NewClient(dockerCli), confutil.ConfigDir(dockerCli), progress)
226207
}
227208
if err != nil {
228209
return nil, res, err
@@ -234,8 +215,6 @@ func buildTargets(ctx context.Context, dockerCli command.Cli, ng *store.NodeGrou
234215
}
235216
}
236217

237-
printWarnings(os.Stderr, printer.Warnings(), progressMode)
238-
239218
for k := range resp {
240219
if opts[k].PrintFunc != nil {
241220
if err := printResult(opts[k].PrintFunc, resp[k].ExporterResponse); err != nil {
@@ -247,46 +226,6 @@ func buildTargets(ctx context.Context, dockerCli command.Cli, ng *store.NodeGrou
247226
return resp[defaultTargetName], res, err
248227
}
249228

250-
func printWarnings(w io.Writer, warnings []client.VertexWarning, mode string) {
251-
if len(warnings) == 0 || mode == progress.PrinterModeQuiet {
252-
return
253-
}
254-
fmt.Fprintf(w, "\n ")
255-
sb := &bytes.Buffer{}
256-
if len(warnings) == 1 {
257-
fmt.Fprintf(sb, "1 warning found")
258-
} else {
259-
fmt.Fprintf(sb, "%d warnings found", len(warnings))
260-
}
261-
if logrus.GetLevel() < logrus.DebugLevel {
262-
fmt.Fprintf(sb, " (use --debug to expand)")
263-
}
264-
fmt.Fprintf(sb, ":\n")
265-
fmt.Fprint(w, aec.Apply(sb.String(), aec.YellowF))
266-
267-
for _, warn := range warnings {
268-
fmt.Fprintf(w, " - %s\n", warn.Short)
269-
if logrus.GetLevel() < logrus.DebugLevel {
270-
continue
271-
}
272-
for _, d := range warn.Detail {
273-
fmt.Fprintf(w, "%s\n", d)
274-
}
275-
if warn.URL != "" {
276-
fmt.Fprintf(w, "More info: %s\n", warn.URL)
277-
}
278-
if warn.SourceInfo != nil && warn.Range != nil {
279-
src := errdefs.Source{
280-
Info: warn.SourceInfo,
281-
Ranges: warn.Range,
282-
}
283-
src.Print(w)
284-
}
285-
fmt.Fprintf(w, "\n")
286-
287-
}
288-
}
289-
290229
func parsePrintFunc(str string) (*build.PrintFunc, error) {
291230
if str == "" {
292231
return nil, nil

‎controller/control/controller.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,13 @@ import (
44
"context"
55
"io"
66

7-
"github.com/containerd/console"
87
controllerapi "github.com/docker/buildx/controller/pb"
8+
"github.com/docker/buildx/util/progress"
99
"github.com/moby/buildkit/client"
1010
)
1111

1212
type BuildxController interface {
13-
Build(ctx context.Context, options controllerapi.BuildOptions, in io.ReadCloser, w io.Writer, out console.File, progressMode string) (ref string, resp *client.SolveResponse, err error)
13+
Build(ctx context.Context, options controllerapi.BuildOptions, in io.ReadCloser, progress progress.Writer) (ref string, resp *client.SolveResponse, err error)
1414
// Invoke starts an IO session into the specified process.
1515
// If pid doesn't matche to any running processes, it starts a new process with the specified config.
1616
// If there is no container running or InvokeConfig.Rollback is speicfied, the process will start in a newly created container.

‎controller/local/controller.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,14 @@ import (
55
"io"
66
"sync/atomic"
77

8-
"github.com/containerd/console"
98
"github.com/docker/buildx/build"
109
cbuild "github.com/docker/buildx/controller/build"
1110
"github.com/docker/buildx/controller/control"
1211
controllererrors "github.com/docker/buildx/controller/errdefs"
1312
controllerapi "github.com/docker/buildx/controller/pb"
1413
"github.com/docker/buildx/controller/processes"
1514
"github.com/docker/buildx/util/ioset"
15+
"github.com/docker/buildx/util/progress"
1616
"github.com/docker/cli/cli/command"
1717
"github.com/moby/buildkit/client"
1818
"github.com/pkg/errors"
@@ -42,13 +42,13 @@ type localController struct {
4242
buildOnGoing atomic.Bool
4343
}
4444

45-
func (b *localController) Build(ctx context.Context, options controllerapi.BuildOptions, in io.ReadCloser, w io.Writer, out console.File, progressMode string) (string, *client.SolveResponse, error) {
45+
func (b *localController) Build(ctx context.Context, options controllerapi.BuildOptions, in io.ReadCloser, progress progress.Writer) (string, *client.SolveResponse, error) {
4646
if !b.buildOnGoing.CompareAndSwap(false, true) {
4747
return "", nil, errors.New("build ongoing")
4848
}
4949
defer b.buildOnGoing.Store(false)
5050

51-
resp, res, buildErr := cbuild.RunBuild(ctx, b.dockerCli, options, in, progressMode, nil, true)
51+
resp, res, buildErr := cbuild.RunBuild(ctx, b.dockerCli, options, in, progress, true)
5252
// NOTE: RunBuild can return *build.ResultContext even on error.
5353
if res != nil {
5454
b.buildConfig = buildConfig{

0 commit comments

Comments
 (0)