From b49e88e3178fc8183e5de7459ef6ae46985b0497 Mon Sep 17 00:00:00 2001 From: Alan Clucas Date: Thu, 24 Oct 2024 17:26:17 +0100 Subject: [PATCH] refactor: remove `util/slice` and use standard `slices` library (#13775) Signed-off-by: Alan Clucas --- pkg/apis/workflow/v1alpha1/workflow_types.go | 7 +- util/slice/slice.go | 21 ------ util/slice/slice_test.go | 69 -------------------- workflow/controller/artifact_gc.go | 7 +- workflow/controller/controller.go | 9 +-- workflow/controller/controller_test.go | 2 +- workflow/controller/operator.go | 5 +- workflow/executor/common/common.go | 4 +- 8 files changed, 19 insertions(+), 105 deletions(-) delete mode 100644 util/slice/slice.go delete mode 100644 util/slice/slice_test.go diff --git a/pkg/apis/workflow/v1alpha1/workflow_types.go b/pkg/apis/workflow/v1alpha1/workflow_types.go index 750f00a06fe4..e9b824139430 100644 --- a/pkg/apis/workflow/v1alpha1/workflow_types.go +++ b/pkg/apis/workflow/v1alpha1/workflow_types.go @@ -11,6 +11,7 @@ import ( "reflect" "regexp" "runtime" + "slices" "sort" "strings" "time" @@ -26,7 +27,6 @@ import ( log "github.com/sirupsen/logrus" argoerrs "github.com/argoproj/argo-workflows/v3/errors" - "github.com/argoproj/argo-workflows/v3/util/slice" ) // TemplateType is the type of a template @@ -3789,7 +3789,7 @@ func (ss *SemaphoreStatus) LockAcquired(holderKey, lockKey string, currentHolder if i < 0 { ss.Holding = append(ss.Holding, SemaphoreHolding{Semaphore: lockKey, Holders: []string{holdingName}}) return true - } else if !slice.ContainsString(semaphoreHolding.Holders, holdingName) { + } else if !slices.Contains(semaphoreHolding.Holders, holdingName) { semaphoreHolding.Holders = append(semaphoreHolding.Holders, holdingName) ss.Holding[i] = semaphoreHolding return true @@ -3802,7 +3802,8 @@ func (ss *SemaphoreStatus) LockReleased(holderKey, lockKey string) bool { holdingName := holderKey if i >= 0 { - semaphoreHolding.Holders = slice.RemoveString(semaphoreHolding.Holders, holdingName) + semaphoreHolding.Holders = slices.DeleteFunc(semaphoreHolding.Holders, + func(x string) bool { return x == holdingName }) ss.Holding[i] = semaphoreHolding return true } diff --git a/util/slice/slice.go b/util/slice/slice.go deleted file mode 100644 index 2787cce42dbc..000000000000 --- a/util/slice/slice.go +++ /dev/null @@ -1,21 +0,0 @@ -package slice - -func RemoveString(slice []string, element string) []string { - for i, v := range slice { - if element == v { - ret := make([]string, 0, len(slice)-1) - ret = append(ret, slice[:i]...) - return append(ret, slice[i+1:]...) - } - } - return slice -} - -func ContainsString(slice []string, element string) bool { - for _, item := range slice { - if item == element { - return true - } - } - return false -} diff --git a/util/slice/slice_test.go b/util/slice/slice_test.go deleted file mode 100644 index fa20b69e1a57..000000000000 --- a/util/slice/slice_test.go +++ /dev/null @@ -1,69 +0,0 @@ -package slice - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestRemoveString(t *testing.T) { - t.Run("RemoveEmpty", func(t *testing.T) { - slice := []string{} - newSlice := RemoveString(slice, "1") - assert.Empty(t, newSlice) - assert.NotContains(t, newSlice, "1") - }) - - t.Run("RemoveSingle", func(t *testing.T) { - slice := []string{"1"} - newSlice := RemoveString(slice, "3") - assert.Len(t, newSlice, 1) - assert.Contains(t, newSlice, "1") - }) - - t.Run("RemoveSingleWithMatch", func(t *testing.T) { - slice := []string{"1"} - newSlice := RemoveString(slice, "1") - assert.Empty(t, newSlice) - assert.NotContains(t, newSlice, "1") - }) - - t.Run("RemoveFirst", func(t *testing.T) { - slice := []string{"1", "2", "3", "4", "5", "6"} - newSlice := RemoveString(slice, "1") - assert.Len(t, newSlice, 5) - assert.NotContains(t, newSlice, "1") - }) - - t.Run("RemoveMiddle", func(t *testing.T) { - slice := []string{"1", "2", "3", "4", "5", "6"} - newSlice := RemoveString(slice, "3") - assert.Len(t, newSlice, 5) - assert.NotContains(t, newSlice, "3") - }) - - t.Run("RemoveLast", func(t *testing.T) { - slice := []string{"1", "2", "3", "4", "5", "6"} - newSlice := RemoveString(slice, "6") - assert.Len(t, newSlice, 5) - assert.NotContains(t, newSlice, "6") - }) -} - -func TestContainsString(t *testing.T) { - slice := []string{"1", "2", "3", "4", "5", "6"} - t.Run("FindFirst", func(t *testing.T) { - assert.True(t, ContainsString(slice, "1")) - }) - - t.Run("FindMiddle", func(t *testing.T) { - assert.True(t, ContainsString(slice, "4")) - }) - - t.Run("FindLast", func(t *testing.T) { - assert.True(t, ContainsString(slice, "6")) - }) - t.Run("NoFound", func(t *testing.T) { - assert.False(t, ContainsString(slice, "7")) - }) -} diff --git a/workflow/controller/artifact_gc.go b/workflow/controller/artifact_gc.go index eeb9547b9e52..5002cbb8fc57 100644 --- a/workflow/controller/artifact_gc.go +++ b/workflow/controller/artifact_gc.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "hash/fnv" + "slices" "sort" "golang.org/x/exp/maps" @@ -16,7 +17,6 @@ import ( "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow" wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1" - "github.com/argoproj/argo-workflows/v3/util/slice" "github.com/argoproj/argo-workflows/v3/workflow/common" "github.com/argoproj/argo-workflows/v3/workflow/controller/indexes" "github.com/argoproj/argo-workflows/v3/workflow/util" @@ -38,7 +38,7 @@ func (woc *wfOperationCtx) addArtifactGCFinalizer() { // only do Artifact GC if we have a Finalizer for it (i.e. Artifact GC is configured for this Workflow // and there's work left to do for it) - if !slice.ContainsString(woc.wf.Finalizers, common.FinalizerArtifactGC) { + if !slices.Contains(woc.wf.Finalizers, common.FinalizerArtifactGC) { if woc.wf.Status.ArtifactGCStatus.NotSpecified { return // we already verified it's not required for this workflow } @@ -549,7 +549,8 @@ func (woc *wfOperationCtx) processArtifactGCCompletion(ctx context.Context) erro } if removeFinalizer { woc.log.Infof("no remaining artifacts to GC, removing artifact GC finalizer (forceFinalizerRemoval=%v)", forceFinalizerRemoval) - woc.wf.Finalizers = slice.RemoveString(woc.wf.Finalizers, common.FinalizerArtifactGC) + woc.wf.Finalizers = slices.DeleteFunc(woc.wf.Finalizers, + func(x string) bool { return x == common.FinalizerArtifactGC }) woc.updated = true } return nil diff --git a/workflow/controller/controller.go b/workflow/controller/controller.go index c1837b0adc85..ccce9cd43107 100644 --- a/workflow/controller/controller.go +++ b/workflow/controller/controller.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "os" + "slices" "strconv" gosync "sync" "syscall" @@ -15,7 +16,6 @@ import ( "github.com/argoproj/pkg/errors" syncpkg "github.com/argoproj/pkg/sync" log "github.com/sirupsen/logrus" - "golang.org/x/exp/slices" "golang.org/x/time/rate" apiv1 "k8s.io/api/core/v1" apierr "k8s.io/apimachinery/pkg/api/errors" @@ -53,7 +53,6 @@ import ( "github.com/argoproj/argo-workflows/v3/util/diff" "github.com/argoproj/argo-workflows/v3/util/env" errorsutil "github.com/argoproj/argo-workflows/v3/util/errors" - "github.com/argoproj/argo-workflows/v3/util/slice" "github.com/argoproj/argo-workflows/v3/util/telemetry" "github.com/argoproj/argo-workflows/v3/workflow/artifactrepositories" "github.com/argoproj/argo-workflows/v3/workflow/common" @@ -546,7 +545,9 @@ func (wfc *WorkflowController) getPodCleanupPatch(pod *apiv1.Pod, labelPodComple finalizerEnabled := os.Getenv(common.EnvVarPodStatusCaptureFinalizer) == "true" if finalizerEnabled && pod.Finalizers != nil { - finalizers := slice.RemoveString(pod.Finalizers, common.FinalizerPodStatus) + finalizers := slices.Clone(pod.Finalizers) + finalizers = slices.DeleteFunc(finalizers, + func(s string) bool { return s == common.FinalizerPodStatus }) if len(finalizers) != len(pod.Finalizers) { un.SetFinalizers(finalizers) un.SetResourceVersion(pod.ObjectMeta.ResourceVersion) @@ -1108,7 +1109,7 @@ func (wfc *WorkflowController) addWorkflowInformerHandlers(ctx context.Context) log.WithError(err).Error("Failed to list pods") } for _, p := range podList.Items { - if slice.ContainsString(p.Finalizers, common.FinalizerPodStatus) { + if slices.Contains(p.Finalizers, common.FinalizerPodStatus) { wfc.queuePodForCleanup(p.Namespace, p.Name, removeFinalizer) } } diff --git a/workflow/controller/controller_test.go b/workflow/controller/controller_test.go index 859f1fa3f861..c7746f4b15fe 100644 --- a/workflow/controller/controller_test.go +++ b/workflow/controller/controller_test.go @@ -1191,7 +1191,7 @@ spec: assert.Empty(t, pods.Items) } -func TestPodCleaupPatch(t *testing.T) { +func TestPodCleanupPatch(t *testing.T) { wfc := &WorkflowController{} pod := &apiv1.Pod{ diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go index a4f5f4391ed0..16ddf218a2c8 100644 --- a/workflow/controller/operator.go +++ b/workflow/controller/operator.go @@ -9,6 +9,7 @@ import ( "reflect" "regexp" "runtime/debug" + "slices" "sort" "strconv" "strings" @@ -50,7 +51,6 @@ import ( "github.com/argoproj/argo-workflows/v3/util/retry" argoruntime "github.com/argoproj/argo-workflows/v3/util/runtime" "github.com/argoproj/argo-workflows/v3/util/secrets" - "github.com/argoproj/argo-workflows/v3/util/slice" "github.com/argoproj/argo-workflows/v3/util/template" waitutil "github.com/argoproj/argo-workflows/v3/util/wait" "github.com/argoproj/argo-workflows/v3/workflow/common" @@ -1769,7 +1769,8 @@ func (woc *wfOperationCtx) deletePVCs(ctx context.Context) error { if err != nil { return err } - x.Finalizers = slice.RemoveString(x.Finalizers, "kubernetes.io/pvc-protection") + x.Finalizers = slices.DeleteFunc(x.Finalizers, + func(s string) bool { return s == "kubernetes.io/pvc-protection" }) _, err = pvcClient.Update(ctx, x, metav1.UpdateOptions{}) if err != nil { return err diff --git a/workflow/executor/common/common.go b/workflow/executor/common/common.go index 1196ad1805ee..54c11ea17ac7 100644 --- a/workflow/executor/common/common.go +++ b/workflow/executor/common/common.go @@ -7,6 +7,7 @@ import ( "fmt" "os" "path/filepath" + "slices" "strings" "syscall" "time" @@ -15,7 +16,6 @@ import ( v1 "k8s.io/api/core/v1" envutil "github.com/argoproj/argo-workflows/v3/util/env" - "github.com/argoproj/argo-workflows/v3/util/slice" ) const ( @@ -96,7 +96,7 @@ func TerminatePodWithContainerNames(ctx context.Context, c KubernetesClientInter return err } for _, s := range containerStatuses { - if !slice.ContainsString(containerNames, s.Name) { + if !slices.Contains(containerNames, s.Name) { continue } if s.State.Terminated != nil {