Skip to content

Commit 1d69f4a

Browse files
thaJeztahndeloof
authored andcommitted
pkg/compose: composeService.Up: rewrite without go-multierror
- Use a errgroup.Group and add a appendErr utility to not fail-fast, but collect errors. - replace doneCh for a global context to cancel goroutines - Commented out attachCtx code, as it didn't appear to be functional (as it wouldn't be cancelled). Signed-off-by: Sebastiaan van Stijn <[email protected]>
1 parent 6078b4d commit 1d69f4a

File tree

2 files changed

+60
-33
lines changed

2 files changed

+60
-33
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ require (
2323
github.com/eiannone/keyboard v0.0.0-20220611211555-0d226195f203
2424
github.com/fsnotify/fsevents v0.2.0
2525
github.com/google/go-cmp v0.7.0
26-
github.com/hashicorp/go-multierror v1.1.1
2726
github.com/hashicorp/go-version v1.7.0
2827
github.com/jonboulle/clockwork v0.5.0
2928
github.com/mattn/go-shellwords v1.0.12
@@ -117,6 +116,7 @@ require (
117116
github.com/grpc-ecosystem/grpc-gateway/v2 v2.26.1 // indirect
118117
github.com/hashicorp/errwrap v1.1.0 // indirect
119118
github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
119+
github.com/hashicorp/go-multierror v1.1.1 // indirect
120120
github.com/in-toto/in-toto-golang v0.9.0 // indirect
121121
github.com/inconshreveable/mousetrap v1.1.0 // indirect
122122
github.com/inhies/go-bytesize v0.0.0-20220417184213-4913239db9cf // indirect

pkg/compose/up.go

Lines changed: 59 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,12 @@ package compose
1818

1919
import (
2020
"context"
21+
"errors"
2122
"fmt"
2223
"os"
2324
"os/signal"
2425
"slices"
26+
"sync"
2527
"sync/atomic"
2628
"syscall"
2729

@@ -33,7 +35,6 @@ import (
3335
"github.com/docker/compose/v2/pkg/api"
3436
"github.com/docker/compose/v2/pkg/progress"
3537
"github.com/eiannone/keyboard"
36-
"github.com/hashicorp/go-multierror"
3738
"github.com/sirupsen/logrus"
3839
"golang.org/x/sync/errgroup"
3940
)
@@ -61,14 +62,11 @@ func (s *composeService) Up(ctx context.Context, project *types.Project, options
6162
return err
6263
}
6364

64-
var eg multierror.Group
65-
6665
// if we get a second signal during shutdown, we kill the services
6766
// immediately, so the channel needs to have sufficient capacity or
6867
// we might miss a signal while setting up the second channel read
6968
// (this is also why signal.Notify is used vs signal.NotifyContext)
7069
signalChan := make(chan os.Signal, 2)
71-
defer close(signalChan)
7270
signal.Notify(signalChan, syscall.SIGINT, syscall.SIGTERM)
7371
defer signal.Stop(signalChan)
7472
var isTerminated atomic.Bool
@@ -103,26 +101,45 @@ func (s *composeService) Up(ctx context.Context, project *types.Project, options
103101

104102
printer := newLogPrinter(logConsumer)
105103

106-
doneCh := make(chan bool)
104+
// global context to handle canceling goroutines
105+
globalCtx, cancel := context.WithCancel(ctx)
106+
defer cancel()
107+
108+
var (
109+
eg errgroup.Group
110+
mu sync.Mutex
111+
errs []error
112+
)
113+
114+
appendErr := func(err error) {
115+
if err != nil {
116+
mu.Lock()
117+
errs = append(errs, err)
118+
mu.Unlock()
119+
}
120+
}
121+
107122
eg.Go(func() error {
108123
first := true
109124
gracefulTeardown := func() {
110125
first = false
111126
fmt.Println("Gracefully Stopping... press Ctrl+C again to force")
112127
eg.Go(func() error {
113-
return progress.RunWithLog(context.WithoutCancel(ctx), func(ctx context.Context) error {
114-
return s.stop(ctx, project.Name, api.StopOptions{
128+
err := progress.RunWithLog(context.WithoutCancel(globalCtx), func(c context.Context) error {
129+
return s.stop(c, project.Name, api.StopOptions{
115130
Services: options.Create.Services,
116131
Project: project,
117132
}, printer.HandleEvent)
118133
}, s.stdinfo(), logConsumer)
134+
appendErr(err)
135+
return nil
119136
})
120137
isTerminated.Store(true)
121138
}
122139

123140
for {
124141
select {
125-
case <-doneCh:
142+
case <-globalCtx.Done():
126143
if watcher != nil {
127144
return watcher.Stop()
128145
}
@@ -133,12 +150,12 @@ func (s *composeService) Up(ctx context.Context, project *types.Project, options
133150
}
134151
case <-signalChan:
135152
if first {
136-
keyboard.Close() //nolint:errcheck
153+
_ = keyboard.Close()
137154
gracefulTeardown()
138155
break
139156
}
140157
eg.Go(func() error {
141-
err := s.kill(context.WithoutCancel(ctx), project.Name, api.KillOptions{
158+
err := s.kill(context.WithoutCancel(globalCtx), project.Name, api.KillOptions{
142159
Services: options.Create.Services,
143160
Project: project,
144161
All: true,
@@ -148,18 +165,21 @@ func (s *composeService) Up(ctx context.Context, project *types.Project, options
148165
return nil
149166
}
150167

151-
return err
168+
appendErr(err)
169+
return nil
152170
})
153171
return nil
154172
case event := <-kEvents:
155-
navigationMenu.HandleKeyEvents(ctx, event, project, options)
173+
navigationMenu.HandleKeyEvents(globalCtx, event, project, options)
156174
}
157175
}
158176
})
159177

160178
if options.Start.Watch && watcher != nil {
161-
err = watcher.Start(ctx)
162-
if err != nil {
179+
if err := watcher.Start(globalCtx); err != nil {
180+
// cancel the global context to terminate background goroutines
181+
cancel()
182+
_ = eg.Wait()
163183
return err
164184
}
165185
}
@@ -186,12 +206,14 @@ func (s *composeService) Up(ctx context.Context, project *types.Project, options
186206
exitCode = event.ExitCode
187207
_, _ = fmt.Fprintln(s.stdinfo(), progress.ErrorColor("Aborting on container exit..."))
188208
eg.Go(func() error {
189-
return progress.RunWithLog(context.WithoutCancel(ctx), func(ctx context.Context) error {
190-
return s.stop(ctx, project.Name, api.StopOptions{
209+
err := progress.RunWithLog(context.WithoutCancel(globalCtx), func(c context.Context) error {
210+
return s.stop(c, project.Name, api.StopOptions{
191211
Services: options.Create.Services,
192212
Project: project,
193213
}, printer.HandleEvent)
194214
}, s.stdinfo(), logConsumer)
215+
appendErr(err)
216+
return nil
195217
})
196218
}
197219
})
@@ -208,13 +230,10 @@ func (s *composeService) Up(ctx context.Context, project *types.Project, options
208230
})
209231
}
210232

211-
// use an independent context tied to the errgroup for background attach operations
212-
// the primary context is still used for other operations
213-
// this means that once any attach operation fails, all other attaches are cancelled,
214-
// but an attach failing won't interfere with the rest of the start
215-
_, attachCtx := errgroup.WithContext(ctx)
216-
containers, err := s.attach(attachCtx, project, printer.HandleEvent, options.Start.AttachTo)
233+
containers, err := s.attach(globalCtx, project, printer.HandleEvent, options.Start.AttachTo)
217234
if err != nil {
235+
cancel()
236+
_ = eg.Wait()
218237
return err
219238
}
220239
attached := make([]string, len(containers))
@@ -230,38 +249,46 @@ func (s *composeService) Up(ctx context.Context, project *types.Project, options
230249
return
231250
}
232251
eg.Go(func() error {
233-
ctr, err := s.apiClient().ContainerInspect(ctx, event.ID)
252+
ctr, err := s.apiClient().ContainerInspect(globalCtx, event.ID)
234253
if err != nil {
235-
return err
254+
appendErr(err)
255+
return nil
236256
}
237257

238-
err = s.doLogContainer(ctx, options.Start.Attach, event.Source, ctr, api.LogOptions{
258+
err = s.doLogContainer(globalCtx, options.Start.Attach, event.Source, ctr, api.LogOptions{
239259
Follow: true,
240260
Since: ctr.State.StartedAt,
241261
})
242262
if errdefs.IsNotImplemented(err) {
243263
// container may be configured with logging_driver: none
244264
// as container already started, we might miss the very first logs. But still better than none
245-
return s.doAttachContainer(ctx, event.Service, event.ID, event.Source, printer.HandleEvent)
265+
err := s.doAttachContainer(globalCtx, event.Service, event.ID, event.Source, printer.HandleEvent)
266+
appendErr(err)
267+
return nil
246268
}
247-
return err
269+
appendErr(err)
270+
return nil
248271
})
249272
})
250273

251274
eg.Go(func() error {
252-
err := monitor.Start(context.Background())
253-
// Signal for the signal-handler goroutines to stop
254-
close(doneCh)
255-
return err
275+
err := monitor.Start(globalCtx)
276+
// cancel the global context to terminate signal-handler goroutines
277+
cancel()
278+
appendErr(err)
279+
return nil
256280
})
257281

258282
// We use the parent context without cancellation as we manage sigterm to stop the stack
259283
err = s.start(context.WithoutCancel(ctx), project.Name, options.Start, printer.HandleEvent)
260284
if err != nil && !isTerminated.Load() { // Ignore error if the process is terminated
285+
cancel()
286+
_ = eg.Wait()
261287
return err
262288
}
263289

264-
err = eg.Wait().ErrorOrNil()
290+
_ = eg.Wait()
291+
err = errors.Join(errs...)
265292
if exitCode != 0 {
266293
errMsg := ""
267294
if err != nil {

0 commit comments

Comments
 (0)