Skip to content

Commit ddc1763

Browse files
author
Tibor Vass
committed
builder: fix lint issues + refactor
Signed-off-by: Tibor Vass <[email protected]>
1 parent 88c8fdd commit ddc1763

File tree

4 files changed

+114
-144
lines changed

4 files changed

+114
-144
lines changed

cli/command/image/build.go

Lines changed: 37 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -372,38 +372,11 @@ func runBuild(dockerCli command.Cli, options buildOptions) error {
372372

373373
configFile := dockerCli.ConfigFile()
374374
authConfigs, _ := configFile.GetAllCredentials()
375-
buildOptions := types.ImageBuildOptions{
376-
Memory: options.memory.Value(),
377-
MemorySwap: options.memorySwap.Value(),
378-
Tags: options.tags.GetAll(),
379-
SuppressOutput: options.quiet,
380-
NoCache: options.noCache,
381-
Remove: options.rm,
382-
ForceRemove: options.forceRm,
383-
PullParent: options.pull,
384-
Isolation: container.Isolation(options.isolation),
385-
CPUSetCPUs: options.cpuSetCpus,
386-
CPUSetMems: options.cpuSetMems,
387-
CPUShares: options.cpuShares,
388-
CPUQuota: options.cpuQuota,
389-
CPUPeriod: options.cpuPeriod,
390-
CgroupParent: options.cgroupParent,
391-
Dockerfile: relDockerfile,
392-
ShmSize: options.shmSize.Value(),
393-
Ulimits: options.ulimits.GetList(),
394-
BuildArgs: configFile.ParseProxyConfig(dockerCli.Client().DaemonHost(), options.buildArgs.GetAll()),
395-
AuthConfigs: authConfigs,
396-
Labels: opts.ConvertKVStringsToMap(options.labels.GetAll()),
397-
CacheFrom: options.cacheFrom,
398-
SecurityOpt: options.securityOpt,
399-
NetworkMode: options.networkMode,
400-
Squash: options.squash,
401-
ExtraHosts: options.extraHosts.GetAll(),
402-
Target: options.target,
403-
RemoteContext: remote,
404-
Platform: options.platform,
405-
Version: types.BuilderV1,
406-
}
375+
buildOptions := imageBuildOptions(dockerCli, options)
376+
buildOptions.Version = types.BuilderV1
377+
buildOptions.Dockerfile = relDockerfile
378+
buildOptions.AuthConfigs = authConfigs
379+
buildOptions.RemoteContext = remote
407380

408381
if s != nil {
409382
go func() {
@@ -613,3 +586,35 @@ func replaceDockerfileForContentTrust(ctx context.Context, inputTarStream io.Rea
613586

614587
return pipeReader
615588
}
589+
590+
func imageBuildOptions(dockerCli command.Cli, options buildOptions) types.ImageBuildOptions {
591+
configFile := dockerCli.ConfigFile()
592+
return types.ImageBuildOptions{
593+
Memory: options.memory.Value(),
594+
MemorySwap: options.memorySwap.Value(),
595+
Tags: options.tags.GetAll(),
596+
SuppressOutput: options.quiet,
597+
NoCache: options.noCache,
598+
Remove: options.rm,
599+
ForceRemove: options.forceRm,
600+
PullParent: options.pull,
601+
Isolation: container.Isolation(options.isolation),
602+
CPUSetCPUs: options.cpuSetCpus,
603+
CPUSetMems: options.cpuSetMems,
604+
CPUShares: options.cpuShares,
605+
CPUQuota: options.cpuQuota,
606+
CPUPeriod: options.cpuPeriod,
607+
CgroupParent: options.cgroupParent,
608+
ShmSize: options.shmSize.Value(),
609+
Ulimits: options.ulimits.GetList(),
610+
BuildArgs: configFile.ParseProxyConfig(dockerCli.Client().DaemonHost(), options.buildArgs.GetAll()),
611+
Labels: opts.ConvertKVStringsToMap(options.labels.GetAll()),
612+
CacheFrom: options.cacheFrom,
613+
SecurityOpt: options.securityOpt,
614+
NetworkMode: options.networkMode,
615+
Squash: options.squash,
616+
ExtraHosts: options.extraHosts.GetAll(),
617+
Target: options.target,
618+
Platform: options.platform,
619+
}
620+
}

cli/command/image/build/context.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,6 @@ func DetectArchiveReader(input io.ReadCloser) (rc io.ReadCloser, isArchive bool,
9797
return ioutils.NewReadCloserWrapper(buf, func() error { return input.Close() }), IsArchive(magic), nil
9898
}
9999

100-
101100
// WriteTempDockerfile writes a Dockerfile stream to a temporary file with a
102101
// name specified by DefaultDockerfileName and returns the path to the
103102
// temporary directory containing the Dockerfile.

cli/command/image/build_buildkit.go

Lines changed: 77 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,7 @@ import (
1111
"github.com/docker/cli/cli"
1212
"github.com/docker/cli/cli/command"
1313
"github.com/docker/cli/cli/command/image/build"
14-
"github.com/docker/cli/opts"
1514
"github.com/docker/docker/api/types"
16-
"github.com/docker/docker/api/types/container"
1715
"github.com/docker/docker/pkg/jsonmessage"
1816
"github.com/docker/docker/pkg/stringid"
1917
"github.com/docker/docker/pkg/urlutil"
@@ -33,6 +31,7 @@ const uploadRequestRemote = "upload-request"
3331

3432
var errDockerfileConflict = errors.New("ambiguous Dockerfile source: both stdin and flag correspond to Dockerfiles")
3533

34+
//nolint: gocyclo
3635
func runBuildBuildKit(dockerCli command.Cli, options buildOptions) error {
3736
ctx := appcontext.Context()
3837

@@ -151,122 +150,97 @@ func runBuildBuildKit(dockerCli command.Cli, options buildOptions) error {
151150
})
152151
}
153152

154-
eg.Go(func() (finalErr error) {
153+
eg.Go(func() error {
155154
defer func() { // make sure the Status ends cleanly on build errors
156155
s.Close()
157156
}()
158157

159-
configFile := dockerCli.ConfigFile()
160-
buildOptions := types.ImageBuildOptions{
161-
Memory: options.memory.Value(),
162-
MemorySwap: options.memorySwap.Value(),
163-
Tags: options.tags.GetAll(),
164-
SuppressOutput: options.quiet,
165-
NoCache: options.noCache,
166-
Remove: options.rm,
167-
ForceRemove: options.forceRm,
168-
PullParent: options.pull,
169-
Isolation: container.Isolation(options.isolation),
170-
CPUSetCPUs: options.cpuSetCpus,
171-
CPUSetMems: options.cpuSetMems,
172-
CPUShares: options.cpuShares,
173-
CPUQuota: options.cpuQuota,
174-
CPUPeriod: options.cpuPeriod,
175-
CgroupParent: options.cgroupParent,
176-
Dockerfile: dockerfileName,
177-
ShmSize: options.shmSize.Value(),
178-
Ulimits: options.ulimits.GetList(),
179-
BuildArgs: configFile.ParseProxyConfig(dockerCli.Client().DaemonHost(), options.buildArgs.GetAll()),
180-
// AuthConfigs: authConfigs, // handled by session
181-
Labels: opts.ConvertKVStringsToMap(options.labels.GetAll()),
182-
CacheFrom: options.cacheFrom,
183-
SecurityOpt: options.securityOpt,
184-
NetworkMode: options.networkMode,
185-
Squash: options.squash,
186-
ExtraHosts: options.extraHosts.GetAll(),
187-
Target: options.target,
188-
RemoteContext: remote,
189-
Platform: options.platform,
190-
SessionID: s.ID(),
191-
Version: types.BuilderBuildKit,
192-
BuildID: buildID,
158+
buildOptions := imageBuildOptions(dockerCli, options)
159+
buildOptions.Version = types.BuilderBuildKit
160+
buildOptions.Dockerfile = dockerfileName
161+
//buildOptions.AuthConfigs = authConfigs // handled by session
162+
buildOptions.RemoteContext = remote
163+
buildOptions.SessionID = s.ID()
164+
buildOptions.BuildID = buildID
165+
return doBuild(ctx, eg, dockerCli, options, buildOptions)
166+
})
167+
168+
return eg.Wait()
169+
}
170+
171+
func doBuild(ctx context.Context, eg *errgroup.Group, dockerCli command.Cli, options buildOptions, buildOptions types.ImageBuildOptions) (finalErr error) {
172+
response, err := dockerCli.Client().ImageBuild(context.Background(), nil, buildOptions)
173+
if err != nil {
174+
return err
175+
}
176+
defer response.Body.Close()
177+
178+
done := make(chan struct{})
179+
defer close(done)
180+
eg.Go(func() error {
181+
select {
182+
case <-ctx.Done():
183+
return dockerCli.Client().BuildCancel(context.TODO(), buildOptions.BuildID)
184+
case <-done:
193185
}
186+
return nil
187+
})
194188

195-
response, err := dockerCli.Client().ImageBuild(context.Background(), nil, buildOptions)
196-
if err != nil {
197-
return err
189+
t := newTracer()
190+
ssArr := []*client.SolveStatus{}
191+
192+
displayStatus := func(displayCh chan *client.SolveStatus) {
193+
var c console.Console
194+
out := os.Stderr
195+
if cons, err := console.ConsoleFromFile(out); err == nil && !options.noConsole {
196+
c = cons
198197
}
199-
defer response.Body.Close()
198+
// not using shared context to not disrupt display but let is finish reporting errors
199+
eg.Go(func() error {
200+
return progressui.DisplaySolveStatus(context.TODO(), c, out, displayCh)
201+
})
202+
}
200203

201-
done := make(chan struct{})
202-
defer close(done)
204+
if options.quiet {
203205
eg.Go(func() error {
204-
select {
205-
case <-ctx.Done():
206-
return dockerCli.Client().BuildCancel(context.TODO(), buildID)
207-
case <-done:
206+
// TODO: make sure t.displayCh closes
207+
for ss := range t.displayCh {
208+
ssArr = append(ssArr, ss)
209+
}
210+
<-done
211+
// TODO: verify that finalErr is indeed set when error occurs
212+
if finalErr != nil {
213+
displayCh := make(chan *client.SolveStatus)
214+
go func() {
215+
for _, ss := range ssArr {
216+
displayCh <- ss
217+
}
218+
close(displayCh)
219+
}()
220+
displayStatus(displayCh)
208221
}
209222
return nil
210223
})
211-
212-
t := newTracer()
213-
ssArr := []*client.SolveStatus{}
214-
215-
displayStatus := func(displayCh chan *client.SolveStatus) {
216-
var c console.Console
217-
out := os.Stderr
218-
if cons, err := console.ConsoleFromFile(out); err == nil && !options.noConsole {
219-
c = cons
220-
}
221-
// not using shared context to not disrupt display but let is finish reporting errors
222-
eg.Go(func() error {
223-
return progressui.DisplaySolveStatus(context.TODO(), c, out, displayCh)
224-
})
225-
}
226-
227-
if options.quiet {
228-
eg.Go(func() error {
229-
// TODO: make sure t.displayCh closes
230-
for ss := range t.displayCh {
231-
ssArr = append(ssArr, ss)
232-
}
233-
<-done
234-
// TODO: verify that finalErr is indeed set when error occurs
235-
if finalErr != nil {
236-
displayCh := make(chan *client.SolveStatus)
237-
go func() {
238-
for _, ss := range ssArr {
239-
displayCh <- ss
240-
}
241-
close(displayCh)
242-
}()
243-
displayStatus(displayCh)
244-
}
245-
return nil
246-
})
247-
} else {
248-
displayStatus(t.displayCh)
249-
}
250-
defer close(t.displayCh)
251-
err = jsonmessage.DisplayJSONMessagesStream(response.Body, os.Stdout, dockerCli.Out().FD(), dockerCli.Out().IsTerminal(), t.write)
252-
if err != nil {
253-
if jerr, ok := err.(*jsonmessage.JSONError); ok {
254-
// If no error code is set, default to 1
255-
if jerr.Code == 0 {
256-
jerr.Code = 1
257-
}
258-
// if options.quiet {
259-
// fmt.Fprintf(dockerCli.Err(), "%s%s", progBuff, buildBuff)
260-
// }
261-
return cli.StatusError{Status: jerr.Message, StatusCode: jerr.Code}
224+
} else {
225+
displayStatus(t.displayCh)
226+
}
227+
defer close(t.displayCh)
228+
err = jsonmessage.DisplayJSONMessagesStream(response.Body, os.Stdout, dockerCli.Out().FD(), dockerCli.Out().IsTerminal(), t.write)
229+
if err != nil {
230+
if jerr, ok := err.(*jsonmessage.JSONError); ok {
231+
// If no error code is set, default to 1
232+
if jerr.Code == 0 {
233+
jerr.Code = 1
262234
}
263-
return err
235+
// if options.quiet {
236+
// fmt.Fprintf(dockerCli.Err(), "%s%s", progBuff, buildBuff)
237+
// }
238+
return cli.StatusError{Status: jerr.Message, StatusCode: jerr.Code}
264239
}
240+
return err
241+
}
265242

266-
return nil
267-
})
268-
269-
return eg.Wait()
243+
return nil
270244
}
271245

272246
func resetUIDAndGID(s *fsutil.Stat) bool {

cli/command/image/build_session.go

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -156,11 +156,3 @@ func tryNodeIdentifier() string {
156156
}
157157
return out
158158
}
159-
160-
func defaultSessionName() string {
161-
wd, err := os.Getwd()
162-
if err != nil {
163-
return "unknown"
164-
}
165-
return filepath.Base(wd)
166-
}

0 commit comments

Comments
 (0)