From d5ac3958a4543ca7ee94f5c39bf2147d4a8089c4 Mon Sep 17 00:00:00 2001 From: p4u1 Date: Mon, 11 Mar 2024 15:24:41 +0100 Subject: [PATCH] feat: add retries to app volume remove --- cli/app/remove.go | 45 ++++--------------- cli/app/volume.go | 7 +-- pkg/client/volumes.go | 33 +++++++++++--- .../client/volumes_test.go | 2 +- tests/integration/app_remove.bats | 3 -- tests/integration/app_volume.bats | 6 --- 6 files changed, 40 insertions(+), 56 deletions(-) rename cli/app/remove_test.go => pkg/client/volumes_test.go (96%) diff --git a/cli/app/remove.go b/cli/app/remove.go index 82499fd1..f9167d82 100644 --- a/cli/app/remove.go +++ b/cli/app/remove.go @@ -5,7 +5,6 @@ import ( "fmt" "log" "os" - "time" "coopcloud.tech/abra/cli/internal" "coopcloud.tech/abra/pkg/autocomplete" @@ -13,7 +12,6 @@ import ( stack "coopcloud.tech/abra/pkg/upstream/stack" "github.com/AlecAivazis/survey/v2" "github.com/docker/docker/api/types" - "github.com/docker/docker/api/types/volume" "github.com/sirupsen/logrus" "github.com/urfave/cli" ) @@ -112,28 +110,19 @@ flag. logrus.Fatal(err) } - volumeListOptions := volume.ListOptions{fs} - volumeListOKBody, err := cl.VolumeList(context.Background(), volumeListOptions) - volumeList := volumeListOKBody.Volumes + volumeList, err := client.GetVolumes(cl, context.Background(), app.Server, fs) if err != nil { logrus.Fatal(err) } + volumeNames := client.GetVolumeNames(volumeList) - var vols []string - for _, vol := range volumeList { - vols = append(vols, vol.Name) - } - - if len(vols) > 0 { - for _, vol := range vols { - err = retryFunc(5, func() error { - return cl.VolumeRemove(context.Background(), vol, internal.Force) // last argument is for force removing - }) - if err != nil { - log.Fatalf("removing volumes failed: %s", err) - } - logrus.Info(fmt.Sprintf("volume %s removed", vol)) + if len(volumeNames) > 0 { + err := client.RemoveVolumes(cl, context.Background(), volumeNames, internal.Force, 5) + if err != nil { + log.Fatalf("removing volumes failed: %s", err) } + + logrus.Infof("%d volumes removed successfully", len(volumeNames)) } else { logrus.Info("no volumes to remove") } @@ -147,21 +136,3 @@ flag. return nil }, } - -// retryFunc retries the given function for the given retries. After the nth -// retry it waits (n + 1)^2 seconds before the next retry (starting with n=0). -// It returns an error if the function still failed after the last retry. -func retryFunc(retries int, fn func() error) error { - for i := 0; i < retries; i++ { - err := fn() - if err == nil { - return nil - } - if i+1 < retries { - sleep := time.Duration(i+1) * time.Duration(i+1) - logrus.Infof("%s: waiting %d seconds before next retry", err, sleep) - time.Sleep(sleep * time.Second) - } - } - return fmt.Errorf("%d retries failed", retries) -} diff --git a/cli/app/volume.go b/cli/app/volume.go index b64ce69c..4fb72c6b 100644 --- a/cli/app/volume.go +++ b/cli/app/volume.go @@ -2,6 +2,7 @@ package app import ( "context" + "log" "coopcloud.tech/abra/cli/internal" "coopcloud.tech/abra/pkg/autocomplete" @@ -131,12 +132,12 @@ Passing "--force/-f" will select all volumes for removal. Be careful. } if len(volumesToRemove) > 0 { - err = client.RemoveVolumes(cl, context.Background(), app.Server, volumesToRemove, internal.Force) + err := client.RemoveVolumes(cl, context.Background(), volumesToRemove, internal.Force, 5) if err != nil { - logrus.Fatal(err) + log.Fatalf("removing volumes failed: %s", err) } - logrus.Info("volumes removed successfully") + logrus.Infof("%d volumes removed successfully", len(volumesToRemove)) } else { logrus.Info("no volumes removed") } diff --git a/pkg/client/volumes.go b/pkg/client/volumes.go index 4afd60c2..f295b081 100644 --- a/pkg/client/volumes.go +++ b/pkg/client/volumes.go @@ -2,15 +2,17 @@ package client import ( "context" + "fmt" + "time" "github.com/docker/docker/api/types/filters" "github.com/docker/docker/api/types/volume" "github.com/docker/docker/client" + "github.com/sirupsen/logrus" ) func GetVolumes(cl *client.Client, ctx context.Context, server string, fs filters.Args) ([]*volume.Volume, error) { - volumeListOptions := volume.ListOptions{fs} - volumeListOKBody, err := cl.VolumeList(ctx, volumeListOptions) + volumeListOKBody, err := cl.VolumeList(ctx, volume.ListOptions{Filters: fs}) volumeList := volumeListOKBody.Volumes if err != nil { return volumeList, err @@ -29,13 +31,32 @@ func GetVolumeNames(volumes []*volume.Volume) []string { return volumeNames } -func RemoveVolumes(cl *client.Client, ctx context.Context, server string, volumeNames []string, force bool) error { +func RemoveVolumes(cl *client.Client, ctx context.Context, volumeNames []string, force bool, retries int) error { for _, volName := range volumeNames { - err := cl.VolumeRemove(ctx, volName, force) + err := retryFunc(5, func() error { + return cl.VolumeRemove(context.Background(), volName, force) + }) if err != nil { - return err + return fmt.Errorf("volume %s: %s", volName, err) } } - return nil } + +// retryFunc retries the given function for the given retries. After the nth +// retry it waits (n + 1)^2 seconds before the next retry (starting with n=0). +// It returns an error if the function still failed after the last retry. +func retryFunc(retries int, fn func() error) error { + for i := 0; i < retries; i++ { + err := fn() + if err == nil { + return nil + } + if i+1 < retries { + sleep := time.Duration(i+1) * time.Duration(i+1) + logrus.Infof("%s: waiting %d seconds before next retry", err, sleep) + time.Sleep(sleep * time.Second) + } + } + return fmt.Errorf("%d retries failed", retries) +} diff --git a/cli/app/remove_test.go b/pkg/client/volumes_test.go similarity index 96% rename from cli/app/remove_test.go rename to pkg/client/volumes_test.go index c3c9f8a0..792af002 100644 --- a/cli/app/remove_test.go +++ b/pkg/client/volumes_test.go @@ -1,4 +1,4 @@ -package app +package client import ( "fmt" diff --git a/tests/integration/app_remove.bats b/tests/integration/app_remove.bats index 4cf6f07a..c0be90da 100644 --- a/tests/integration/app_remove.bats +++ b/tests/integration/app_remove.bats @@ -104,9 +104,6 @@ teardown(){ _undeploy_app - # TODO: should wait as long as volume is no longer in use - sleep 10 - run $ABRA app volume rm "$TEST_APP_DOMAIN" --no-input assert_success diff --git a/tests/integration/app_volume.bats b/tests/integration/app_volume.bats index 99acc6c9..6327821c 100644 --- a/tests/integration/app_volume.bats +++ b/tests/integration/app_volume.bats @@ -78,9 +78,6 @@ teardown(){ _undeploy_app - # NOTE(d1): to let the stack come down before nuking volumes - sleep 10 - run $ABRA app volume rm "$TEST_APP_DOMAIN" --force assert_success assert_output --partial 'volumes removed successfully' @@ -92,9 +89,6 @@ teardown(){ _undeploy_app - # NOTE(d1): to let the stack come down before nuking volumes - sleep 10 - run $ABRA app volume rm "$TEST_APP_DOMAIN" --force assert_success assert_output --partial 'volumes removed successfully'