From c86bcdadf89630f01618ef37747f535b9f055fce Mon Sep 17 00:00:00 2001 From: Nicolas De loof Date: Mon, 27 Apr 2026 11:28:42 +0000 Subject: [PATCH] refactor: miscellaneous small cleanups (forEach, filters, Set) Assisted-By: docker-agent Signed-off-by: Nicolas De loof --- pkg/compose/containers.go | 9 ++------- pkg/compose/convergence_test.go | 4 ++-- pkg/compose/filters.go | 17 +++++++++-------- pkg/compose/kill_test.go | 6 +++--- pkg/compose/logs_test.go | 4 ++-- pkg/compose/monitor.go | 2 +- pkg/compose/ps_test.go | 2 +- pkg/compose/remove.go | 5 ++--- pkg/utils/set.go | 22 +++------------------- pkg/utils/set_test.go | 14 -------------- 10 files changed, 25 insertions(+), 60 deletions(-) diff --git a/pkg/compose/containers.go b/pkg/compose/containers.go index 19e96a6ef..b2f6b9748 100644 --- a/pkg/compose/containers.go +++ b/pkg/compose/containers.go @@ -62,7 +62,7 @@ func getDefaultFilters(projectName string, oneOff oneOff, selectedServices ...st if len(selectedServices) == 1 { f.Add("label", serviceFilter(selectedServices[0])) } - f.Add("label", hasConfigHashLabel()) + f.Add("label", api.ConfigHashLabel) switch oneOff { case oneOffOnly: f.Add("label", oneOffFilter(true)) @@ -167,12 +167,6 @@ func (containers Containers) names() []string { return names } -func (containers Containers) forEach(fn func(container.Summary)) { - for _, c := range containers { - fn(c) - } -} - // forEachContainerConcurrent runs fn for every container concurrently and waits for all goroutines. func forEachContainerConcurrent(ctx context.Context, containers Containers, fn func(context.Context, container.Summary) error) error { eg, ctx := errgroup.WithContext(ctx) @@ -184,6 +178,7 @@ func forEachContainerConcurrent(ctx context.Context, containers Containers, fn f return eg.Wait() } +// sorted sorts containers in place by canonical name and returns the (same) slice. func (containers Containers) sorted() Containers { sort.Slice(containers, func(i, j int) bool { return getCanonicalContainerName(containers[i]) < getCanonicalContainerName(containers[j]) diff --git a/pkg/compose/convergence_test.go b/pkg/compose/convergence_test.go index 2f7c31cf8..441356959 100644 --- a/pkg/compose/convergence_test.go +++ b/pkg/compose/convergence_test.go @@ -73,7 +73,7 @@ func TestServiceLinks(t *testing.T) { Filters: projectFilter(testProject).Add("label", serviceFilter("db"), oneOffFilter(false), - hasConfigHashLabel(), + api.ConfigHashLabel, ), All: true, } @@ -201,7 +201,7 @@ func TestServiceLinks(t *testing.T) { Filters: projectFilter(testProject).Add("label", serviceFilter("web"), oneOffFilter(false), - hasConfigHashLabel(), + api.ConfigHashLabel, ), All: true, } diff --git a/pkg/compose/filters.go b/pkg/compose/filters.go index ebac9eb82..27ecd0311 100644 --- a/pkg/compose/filters.go +++ b/pkg/compose/filters.go @@ -24,16 +24,21 @@ import ( "github.com/docker/compose/v5/pkg/api" ) +// labelFilter returns a label filter string of the form "key=value". +func labelFilter(key, value string) string { + return fmt.Sprintf("%s=%s", key, value) +} + func projectFilter(projectName string) client.Filters { - return make(client.Filters).Add("label", fmt.Sprintf("%s=%s", api.ProjectLabel, projectName)) + return make(client.Filters).Add("label", labelFilter(api.ProjectLabel, projectName)) } func serviceFilter(serviceName string) string { - return fmt.Sprintf("%s=%s", api.ServiceLabel, serviceName) + return labelFilter(api.ServiceLabel, serviceName) } func networkFilter(name string) string { - return fmt.Sprintf("%s=%s", api.NetworkLabel, name) + return labelFilter(api.NetworkLabel, name) } func oneOffFilter(b bool) string { @@ -45,9 +50,5 @@ func oneOffFilter(b bool) string { } func containerNumberFilter(index int) string { - return fmt.Sprintf("%s=%d", api.ContainerNumberLabel, index) -} - -func hasConfigHashLabel() string { - return api.ConfigHashLabel + return labelFilter(api.ContainerNumberLabel, fmt.Sprintf("%d", index)) } diff --git a/pkg/compose/kill_test.go b/pkg/compose/kill_test.go index cc71ed7cc..9b9a0e078 100644 --- a/pkg/compose/kill_test.go +++ b/pkg/compose/kill_test.go @@ -44,7 +44,7 @@ func TestKillAll(t *testing.T) { name := strings.ToLower(testProject) api.EXPECT().ContainerList(t.Context(), client.ContainerListOptions{ - Filters: projectFilter(name).Add("label", hasConfigHashLabel()), + Filters: projectFilter(name).Add("label", api.ConfigHashLabel), }).Return(client.ContainerListResult{ Items: []container.Summary{ testContainer("service1", "123", false), @@ -83,7 +83,7 @@ func TestKillSignal(t *testing.T) { name := strings.ToLower(testProject) listOptions := client.ContainerListOptions{ - Filters: projectFilter(name).Add("label", serviceFilter(serviceName), hasConfigHashLabel()), + Filters: projectFilter(name).Add("label", serviceFilter(serviceName), api.ConfigHashLabel), } api.EXPECT().ContainerList(t.Context(), listOptions).Return(client.ContainerListResult{ @@ -145,7 +145,7 @@ func anyCancellableContext() gomock.Matcher { } func projectFilterListOpt(withOneOff bool) client.ContainerListOptions { - filter := projectFilter(strings.ToLower(testProject)).Add("label", hasConfigHashLabel()) + filter := projectFilter(strings.ToLower(testProject)).Add("label", api.ConfigHashLabel) if !withOneOff { filter.Add("label", oneOffFilter(false)) } diff --git a/pkg/compose/logs_test.go b/pkg/compose/logs_test.go index 3b02ef965..fe9d4238c 100644 --- a/pkg/compose/logs_test.go +++ b/pkg/compose/logs_test.go @@ -98,7 +98,7 @@ func TestComposeService_Logs_Demux(t *testing.T) { api.EXPECT().ContainerList(t.Context(), client.ContainerListOptions{ All: true, - Filters: projectFilter(name).Add("label", oneOffFilter(false), hasConfigHashLabel()), + Filters: projectFilter(name).Add("label", oneOffFilter(false), api.ConfigHashLabel), }).Return( client.ContainerListResult{ Items: []containerType.Summary{ @@ -166,7 +166,7 @@ func TestComposeService_Logs_ServiceFiltering(t *testing.T) { api.EXPECT().ContainerList(t.Context(), client.ContainerListOptions{ All: true, - Filters: projectFilter(name).Add("label", oneOffFilter(false), hasConfigHashLabel()), + Filters: projectFilter(name).Add("label", oneOffFilter(false), api.ConfigHashLabel), }).Return( client.ContainerListResult{ Items: []containerType.Summary{ diff --git a/pkg/compose/monitor.go b/pkg/compose/monitor.go index d544d5620..f405a5d5b 100644 --- a/pkg/compose/monitor.go +++ b/pkg/compose/monitor.go @@ -60,7 +60,7 @@ func (c *monitor) Start(ctx context.Context) error { All: true, Filters: projectFilter(c.project).Add("label", oneOffFilter(false), - hasConfigHashLabel(), + api.ConfigHashLabel, ), }) if err != nil { diff --git a/pkg/compose/ps_test.go b/pkg/compose/ps_test.go index 1bca7bcc6..3027fc51c 100644 --- a/pkg/compose/ps_test.go +++ b/pkg/compose/ps_test.go @@ -38,7 +38,7 @@ func TestPs(t *testing.T) { assert.NilError(t, err) listOpts := client.ContainerListOptions{ - Filters: projectFilter(strings.ToLower(testProject)).Add("label", hasConfigHashLabel(), oneOffFilter(false)), + Filters: projectFilter(strings.ToLower(testProject)).Add("label", api.ConfigHashLabel, oneOffFilter(false)), All: false, } c1, inspect1 := containerDetails("service1", "123", containerType.StateRunning, containerType.Healthy, 0) diff --git a/pkg/compose/remove.go b/pkg/compose/remove.go index 8b81a61f8..b9bf0f336 100644 --- a/pkg/compose/remove.go +++ b/pkg/compose/remove.go @@ -21,7 +21,6 @@ import ( "fmt" "strings" - "github.com/moby/moby/api/types/container" "github.com/moby/moby/client" "golang.org/x/sync/errgroup" @@ -71,9 +70,9 @@ func (s *composeService) Remove(ctx context.Context, projectName string, options } var names []string - stoppedContainers.forEach(func(c container.Summary) { + for _, c := range stoppedContainers { names = append(names, getCanonicalContainerName(c)) - }) + } if len(names) == 0 { return api.ErrNoResources diff --git a/pkg/utils/set.go b/pkg/utils/set.go index 5a092d7c2..1086176a0 100644 --- a/pkg/utils/set.go +++ b/pkg/utils/set.go @@ -51,9 +51,9 @@ func (s Set[T]) Remove(v T) bool { return ok } -func (s Set[T]) Clear() { - for v := range s { - delete(s, v) +func (s Set[T]) RemoveAll(elements ...T) { + for _, e := range elements { + s.Remove(e) } } @@ -65,12 +65,6 @@ func (s Set[T]) Elements() []T { return elements } -func (s Set[T]) RemoveAll(elements ...T) { - for _, e := range elements { - s.Remove(e) - } -} - func (s Set[T]) Diff(other Set[T]) Set[T] { out := make(Set[T]) for k := range s { @@ -81,13 +75,3 @@ func (s Set[T]) Diff(other Set[T]) Set[T] { return out } -func (s Set[T]) Union(other Set[T]) Set[T] { - out := make(Set[T]) - for k := range s { - out[k] = struct{}{} - } - for k := range other { - out[k] = struct{}{} - } - return out -} diff --git a/pkg/utils/set_test.go b/pkg/utils/set_test.go index 452bacc1c..3a1896197 100644 --- a/pkg/utils/set_test.go +++ b/pkg/utils/set_test.go @@ -15,7 +15,6 @@ package utils import ( - "slices" "testing" "gotest.tools/v3/assert" @@ -33,16 +32,3 @@ func TestSet_Diff(t *testing.T) { assert.DeepEqual(t, []int{1}, a.Diff(b).Elements()) assert.DeepEqual(t, []int{3}, b.Diff(a).Elements()) } - -func TestSet_Union(t *testing.T) { - a := NewSet[int](1, 2) - b := NewSet[int](2, 3) - - actual := a.Union(b).Elements() - slices.Sort(actual) - assert.DeepEqual(t, []int{1, 2, 3}, actual) - - actual = b.Union(a).Elements() - slices.Sort(actual) - assert.DeepEqual(t, []int{1, 2, 3}, actual) -}