From e0f3d7a89ffc44efcb766d53761710949b6c7549 Mon Sep 17 00:00:00 2001 From: Simon Bauer Date: Thu, 24 Oct 2024 12:44:53 +0200 Subject: [PATCH] fix, Cleanup Docker volume in any case Fixes #366 Not sure worth doing an actual test case cause it would be quite complex (i.e. starting a separate binary and then simulating SIGTERM, ensuring that the volume is then removed). Verified that it works. --- cmd/eval-dev-quality/cmd/cleanup/cleanup.go | 81 +++++++++++++++++++ .../cmd/cleanup/cleanup_test.go | 74 +++++++++++++++++ cmd/eval-dev-quality/cmd/evaluate.go | 34 +++++--- cmd/eval-dev-quality/main.go | 4 + 4 files changed, 180 insertions(+), 13 deletions(-) create mode 100644 cmd/eval-dev-quality/cmd/cleanup/cleanup.go create mode 100644 cmd/eval-dev-quality/cmd/cleanup/cleanup_test.go diff --git a/cmd/eval-dev-quality/cmd/cleanup/cleanup.go b/cmd/eval-dev-quality/cmd/cleanup/cleanup.go new file mode 100644 index 00000000..3140a46c --- /dev/null +++ b/cmd/eval-dev-quality/cmd/cleanup/cleanup.go @@ -0,0 +1,81 @@ +package cleanup + +import ( + "fmt" + "os" + "os/signal" + "sync" + "syscall" +) + +var ( + // functions holds the cleanup functions to execute on program exit. + functions []func() + // lockFunctions is the lock for accessing the cleanup functions. + lockFunctions sync.Mutex +) + +// Register adds a function for cleanup. +// REMARK: The function may be called from a different Goroutine so the scope of the function needs to be accessed in a thread-safe manner. +func Register(f func()) { + ensureInitialized() + + lockFunctions.Lock() + defer lockFunctions.Unlock() + + functions = append(functions, f) +} + +var ( + // signalChannel holds the channel for notifying os signals. + signalChannel chan os.Signal + // handler synchronizes the cleanup handler. + handler sync.WaitGroup +) + +func ensureInitialized() { + if signalChannel == nil { + panic("cleanup was never initialized") + } +} + +// Init sets up cleanup. +// If already initialized, subsequent calls to "Init" block until cleanup was triggered. +func Init() { + handler.Wait() + + lockFunctions.Lock() + defer lockFunctions.Unlock() + + functions = nil + + if signalChannel == nil { // Assume that we already set everything up. + signalChannel = make(chan os.Signal, 1) + signal.Notify(signalChannel, os.Interrupt, syscall.SIGTERM) + } + + handler.Add(1) + go func() { + defer handler.Done() + <-signalChannel + + lockFunctions.Lock() + defer lockFunctions.Unlock() + + if len(functions) > 0 { + fmt.Println("Graceful shutdown. Cleaning up...") + } + for _, f := range functions { + f() + } + functions = nil + }() +} + +// Trigger executes the cleanup manually. +func Trigger() { + ensureInitialized() + + signalChannel <- os.Interrupt // We react to all signals coming through the channel, so any would do. + handler.Wait() +} diff --git a/cmd/eval-dev-quality/cmd/cleanup/cleanup_test.go b/cmd/eval-dev-quality/cmd/cleanup/cleanup_test.go new file mode 100644 index 00000000..ee6d390d --- /dev/null +++ b/cmd/eval-dev-quality/cmd/cleanup/cleanup_test.go @@ -0,0 +1,74 @@ +package cleanup + +import ( + "sync/atomic" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestCleanup(t *testing.T) { + type testCase struct { + Name string + + Functions []func() + + Validate func(t *testing.T) + } + + validate := func(t *testing.T, tc *testCase) { + t.Run(tc.Name, func(t *testing.T) { + Init() + + for _, f := range tc.Functions { + Register(f) + } + Trigger() + + if tc.Validate != nil { + tc.Validate(t) + } + }) + } + + validate(t, &testCase{ + Name: "None", + + Functions: []func(){}, + }) + { + value := &atomic.Int32{} + validate(t, &testCase{ + Name: "Single", + + Functions: []func(){ + func() { + value.Add(1) + }, + }, + + Validate: func(t *testing.T) { + assert.EqualValues(t, 1, value.Load()) + }, + }) + } + { + value := &atomic.Int32{} + validate(t, &testCase{ + Name: "Multiple", + + Functions: []func(){ + func() { + value.Add(1) + }, + func() { + value.Add(1) + }, + }, + + Validate: func(t *testing.T) { + assert.EqualValues(t, 2, value.Load()) + }, + }) + } +} diff --git a/cmd/eval-dev-quality/cmd/evaluate.go b/cmd/eval-dev-quality/cmd/evaluate.go index 8fdcfd93..1a1956c5 100644 --- a/cmd/eval-dev-quality/cmd/evaluate.go +++ b/cmd/eval-dev-quality/cmd/evaluate.go @@ -16,10 +16,12 @@ import ( "text/template" "time" + "github.com/avast/retry-go" pkgerrors "github.com/pkg/errors" "github.com/zimmski/osutil" "golang.org/x/exp/maps" + "github.com/symflower/eval-dev-quality/cmd/eval-dev-quality/cmd/cleanup" "github.com/symflower/eval-dev-quality/evaluate" "github.com/symflower/eval-dev-quality/evaluate/metrics" "github.com/symflower/eval-dev-quality/evaluate/report" @@ -580,20 +582,26 @@ func (command *Evaluate) evaluateDocker(ctx *evaluate.Context) (err error) { return pkgerrors.WithMessage(pkgerrors.WithStack(err), output) } - // Cleanup volume. - defer func() { - output, deferErr := util.CommandWithResult(context.Background(), command.logger, &util.Command{ - Command: []string{ - "docker", - "volume", - "rm", - volumeName, - }, - }) - if deferErr != nil { - err = errors.Join(err, pkgerrors.WithMessage(pkgerrors.WithStack(deferErr), output)) + // Remove volume in a global cleanup to ensure it happens even if the user aborts using "Ctrl+C". + cleanup.Register(func() { + err := retry.Do(func() error { + _, err := util.CommandWithResult(context.Background(), command.logger, &util.Command{ + Command: []string{ + "docker", + "volume", + "rm", + volumeName, + }, + }) + + return err + }, retry.RetryIf(func(err error) bool { + return strings.Contains(err.Error(), "volume is in use") + }), retry.Attempts(5), retry.Delay(time.Second)) + if err != nil { + command.logger.Error(fmt.Sprintf("could not cleanup Docker volume: %s", err.Error())) } - }() + }) } // Pull the image to ensure using the latest version diff --git a/cmd/eval-dev-quality/main.go b/cmd/eval-dev-quality/main.go index 346854d7..842cb118 100644 --- a/cmd/eval-dev-quality/main.go +++ b/cmd/eval-dev-quality/main.go @@ -4,9 +4,13 @@ import ( "os" "github.com/symflower/eval-dev-quality/cmd/eval-dev-quality/cmd" + "github.com/symflower/eval-dev-quality/cmd/eval-dev-quality/cmd/cleanup" "github.com/symflower/eval-dev-quality/log" ) func main() { + cleanup.Init() + defer cleanup.Trigger() + cmd.Execute(log.STDOUT(), os.Args[1:]) }