From 2dcd791a4589b36047b2ef54f751617cd571fc47 Mon Sep 17 00:00:00 2001 From: dongjiang Date: Wed, 29 Oct 2025 22:05:59 +0800 Subject: [PATCH] change sort to slices package Signed-off-by: dongjiang --- pkg/cache/cache.go | 4 ++-- pkg/cache/cache_test.go | 14 +++++++------- pkg/client/apiutil/errors.go | 4 ++-- pkg/healthz/healthz.go | 4 ++-- pkg/internal/testing/certs/tinyca_test.go | 9 +++++---- pkg/internal/testing/process/arguments.go | 4 ++-- tools/setup-envtest/env/env.go | 9 +++++---- tools/setup-envtest/env/helpers.go | 7 ++++--- tools/setup-envtest/remote/http_client.go | 7 +++---- tools/setup-envtest/store/store.go | 17 +++++++++-------- tools/setup-envtest/versions/misc_test.go | 6 +++--- tools/setup-envtest/versions/version.go | 19 ++++++++++++++----- .../setup-envtest/workflows/workflows_test.go | 4 ++-- 13 files changed, 60 insertions(+), 48 deletions(-) diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index a7e491855a..107a7f1cda 100644 --- a/pkg/cache/cache.go +++ b/pkg/cache/cache.go @@ -22,7 +22,6 @@ import ( "maps" "net/http" "slices" - "sort" "time" corev1 "k8s.io/api/core/v1" @@ -583,7 +582,8 @@ func defaultConfig(toDefault, defaultFrom Config) Config { func namespaceAllSelector(namespaces []string) []fields.Selector { selectors := make([]fields.Selector, 0, len(namespaces)-1) - sort.Strings(namespaces) + slices.Sort(namespaces) + for _, namespace := range namespaces { if namespace != metav1.NamespaceAll { selectors = append(selectors, fields.OneTermNotEqualSelector("metadata.namespace", namespace)) diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go index 7748e2e317..c2dae0978f 100644 --- a/pkg/cache/cache_test.go +++ b/pkg/cache/cache_test.go @@ -21,7 +21,7 @@ import ( "errors" "fmt" "reflect" - "sort" + "slices" "strconv" "strings" "time" @@ -808,8 +808,8 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca By("verifying the pointer fields in pod have the same addresses") Expect(outList1.Items).To(HaveLen(len(outList2.Items))) - sort.SliceStable(outList1.Items, func(i, j int) bool { return outList1.Items[i].Name <= outList1.Items[j].Name }) - sort.SliceStable(outList2.Items, func(i, j int) bool { return outList2.Items[i].Name <= outList2.Items[j].Name }) + slices.SortStableFunc(outList1.Items, func(i, j corev1.Pod) int { return strings.Compare(i.Name, j.Name) }) + slices.SortStableFunc(outList2.Items, func(i, j corev1.Pod) int { return strings.Compare(i.Name, j.Name) }) for i := range outList1.Items { a := &outList1.Items[i] b := &outList2.Items[i] @@ -1134,8 +1134,8 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca By("verifying the pointer fields in pod have the same addresses") Expect(outList1.Items).To(HaveLen(len(outList2.Items))) - sort.SliceStable(outList1.Items, func(i, j int) bool { return outList1.Items[i].GetName() <= outList1.Items[j].GetName() }) - sort.SliceStable(outList2.Items, func(i, j int) bool { return outList2.Items[i].GetName() <= outList2.Items[j].GetName() }) + slices.SortStableFunc(outList1.Items, func(i, j unstructured.Unstructured) int { return strings.Compare(i.GetName(), j.GetName()) }) + slices.SortStableFunc(outList2.Items, func(i, j unstructured.Unstructured) int { return strings.Compare(i.GetName(), j.GetName()) }) for i := range outList1.Items { a := &outList1.Items[i] b := &outList2.Items[i] @@ -1519,8 +1519,8 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca By("verifying the pointer fields in pod have the same addresses") Expect(outList1.Items).To(HaveLen(len(outList2.Items))) - sort.SliceStable(outList1.Items, func(i, j int) bool { return outList1.Items[i].Name <= outList1.Items[j].Name }) - sort.SliceStable(outList2.Items, func(i, j int) bool { return outList2.Items[i].Name <= outList2.Items[j].Name }) + slices.SortStableFunc(outList1.Items, func(i, j metav1.PartialObjectMetadata) int { return strings.Compare(i.Name, j.Name) }) + slices.SortStableFunc(outList2.Items, func(i, j metav1.PartialObjectMetadata) int { return strings.Compare(i.Name, j.Name) }) for i := range outList1.Items { a := &outList1.Items[i] b := &outList2.Items[i] diff --git a/pkg/client/apiutil/errors.go b/pkg/client/apiutil/errors.go index c216c49d2a..b00e071232 100644 --- a/pkg/client/apiutil/errors.go +++ b/pkg/client/apiutil/errors.go @@ -18,7 +18,7 @@ package apiutil import ( "fmt" - "sort" + "slices" "strings" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -38,7 +38,7 @@ func (e *ErrResourceDiscoveryFailed) Error() string { for k, v := range *e { subErrors = append(subErrors, fmt.Sprintf("%s: %v", k, v)) } - sort.Strings(subErrors) + slices.Sort(subErrors) return fmt.Sprintf("unable to retrieve the complete list of server APIs: %s", strings.Join(subErrors, ", ")) } diff --git a/pkg/healthz/healthz.go b/pkg/healthz/healthz.go index cfb5dc8d02..149b02ec98 100644 --- a/pkg/healthz/healthz.go +++ b/pkg/healthz/healthz.go @@ -20,7 +20,7 @@ import ( "fmt" "net/http" "path" - "sort" + "slices" "strings" "k8s.io/apimachinery/pkg/util/sets" @@ -75,7 +75,7 @@ func (h *Handler) serveAggregated(resp http.ResponseWriter, req *http.Request) { } // ...sort to be consistent... - sort.Slice(parts, func(i, j int) bool { return parts[i].name < parts[j].name }) + slices.SortStableFunc(parts, func(i, j checkStatus) int { return strings.Compare(i.name, j.name) }) // ...and write out the result // TODO(directxman12): this should also accept a request for JSON content (via a accept header) diff --git a/pkg/internal/testing/certs/tinyca_test.go b/pkg/internal/testing/certs/tinyca_test.go index 5d84de56fb..9542975565 100644 --- a/pkg/internal/testing/certs/tinyca_test.go +++ b/pkg/internal/testing/certs/tinyca_test.go @@ -21,7 +21,7 @@ import ( "encoding/pem" "math/big" "net" - "sort" + "slices" "time" . "github.com/onsi/ginkgo/v2" @@ -67,10 +67,11 @@ var _ = Describe("TinyCA", func() { secondCerts.Cert.SerialNumber, thirdCerts.Cert.SerialNumber, } - // quick uniqueness check of numbers: sort, then you only have to compare sequential entries - sort.Slice(serials, func(i, j int) bool { - return serials[i].Cmp(serials[j]) == -1 + // quick uniqueness check of numbers: slices sort, then you only have to compare sequential entries + slices.SortStableFunc(serials, func(i, j *big.Int) int { + return i.Cmp(j) }) + Expect(serials[1].Cmp(serials[0])).NotTo(Equal(0), "serials shouldn't be equal") Expect(serials[2].Cmp(serials[1])).NotTo(Equal(0), "serials shouldn't be equal") }) diff --git a/pkg/internal/testing/process/arguments.go b/pkg/internal/testing/process/arguments.go index 391eec1fac..1e556e9980 100644 --- a/pkg/internal/testing/process/arguments.go +++ b/pkg/internal/testing/process/arguments.go @@ -19,7 +19,7 @@ package process import ( "bytes" "html/template" - "sort" + "slices" "strings" ) @@ -230,7 +230,7 @@ func (a *Arguments) AsStrings(defaults map[string][]string) []string { for key := range a.values { keysInOrder = append(keysInOrder, key) } - sort.Strings(keysInOrder) + slices.Sort(keysInOrder) var res []string for _, key := range keysInOrder { diff --git a/tools/setup-envtest/env/env.go b/tools/setup-envtest/env/env.go index 6168739eb6..cfcad3505b 100644 --- a/tools/setup-envtest/env/env.go +++ b/tools/setup-envtest/env/env.go @@ -10,7 +10,7 @@ import ( "io" "io/fs" "path/filepath" - "sort" + "slices" "strings" "text/tabwriter" @@ -121,9 +121,10 @@ func (e *Env) ListVersions(ctx context.Context) { if !e.Version.Matches(set.Version) { continue } - sort.Slice(set.Platforms, func(i, j int) bool { - return orderPlatforms(set.Platforms[i].Platform, set.Platforms[j].Platform) + slices.SortStableFunc(set.Platforms, func(i, j versions.PlatformItem) int { + return orderPlatforms(i.Platform, j.Platform) }) + for _, plat := range set.Platforms { if e.Platform.Matches(plat.Platform) { fmt.Fprintf(out, "(available)\tv%s\t%s\n", set.Version, plat) @@ -246,7 +247,7 @@ func (e *Env) EnsureVersionIsSet(ctx context.Context) { serverVer, platform := e.LatestVersion(ctx) // if we're not forcing a download, and we have a newer local version, just use that - if !e.ForceDownload && localVer != nil && localVer.NewerThan(serverVer) { + if !e.ForceDownload && localVer != nil && localVer.Compare(serverVer) > 0 { e.Platform.Platform = localPlat // update our data with hash e.Version.MakeConcrete(*localVer) return diff --git a/tools/setup-envtest/env/helpers.go b/tools/setup-envtest/env/helpers.go index 2c98c88d95..8572088517 100644 --- a/tools/setup-envtest/env/helpers.go +++ b/tools/setup-envtest/env/helpers.go @@ -5,17 +5,18 @@ package env import ( "fmt" + "strings" "sigs.k8s.io/controller-runtime/tools/setup-envtest/versions" ) // orderPlatforms orders platforms by OS then arch. -func orderPlatforms(first, second versions.Platform) bool { +func orderPlatforms(first, second versions.Platform) int { // sort by OS, then arch if first.OS != second.OS { - return first.OS < second.OS + return strings.Compare(first.OS, second.OS) } - return first.Arch < second.Arch + return strings.Compare(first.Arch, second.Arch) } // PrintFormat indicates how to print out fetch and switch results. diff --git a/tools/setup-envtest/remote/http_client.go b/tools/setup-envtest/remote/http_client.go index 0339654a82..a87ef1f105 100644 --- a/tools/setup-envtest/remote/http_client.go +++ b/tools/setup-envtest/remote/http_client.go @@ -9,7 +9,7 @@ import ( "io" "net/http" "net/url" - "sort" + "slices" "github.com/go-logr/logr" "sigs.k8s.io/controller-runtime/tools/setup-envtest/versions" @@ -87,9 +87,8 @@ func (c *HTTPClient) ListVersions(ctx context.Context) ([]versions.Set, error) { res = append(res, versions.Set{Version: ver, Platforms: details}) } // sort in inverse order so that the newest one is first - sort.Slice(res, func(i, j int) bool { - first, second := res[i].Version, res[j].Version - return first.NewerThan(second) + slices.SortStableFunc(res, func(i, j versions.Set) int { + return i.Version.Compare(j.Version) }) return res, nil diff --git a/tools/setup-envtest/store/store.go b/tools/setup-envtest/store/store.go index bb5a1f7bcd..1e1d4beb3c 100644 --- a/tools/setup-envtest/store/store.go +++ b/tools/setup-envtest/store/store.go @@ -12,7 +12,8 @@ import ( "io" "os" "path/filepath" - "sort" + "slices" + "strings" "github.com/go-logr/logr" "github.com/spf13/afero" @@ -102,11 +103,11 @@ func (s *Store) List(ctx context.Context, matching Filter) ([]Item, error) { return nil, fmt.Errorf("unable to list version-platform pairs in store: %w", err) } - sort.Slice(res, func(i, j int) bool { - if !res[i].Version.Matches(res[j].Version) { - return res[i].Version.NewerThan(res[j].Version) + slices.SortStableFunc(res, func(i, j Item) int { + if !i.Version.Matches(j.Version) { + return i.Version.Compare(j.Version) } - return orderPlatforms(res[i].Platform, res[j].Platform) + return orderPlatforms(i.Platform, j.Platform) }) return res, nil @@ -296,10 +297,10 @@ func (s *Store) removeItem(itemDir afero.Fs) error { } // orderPlatforms orders platforms by OS then arch. -func orderPlatforms(first, second versions.Platform) bool { +func orderPlatforms(first, second versions.Platform) int { // sort by OS, then arch if first.OS != second.OS { - return first.OS < second.OS + return strings.Compare(first.OS, second.OS) } - return first.Arch < second.Arch + return strings.Compare(first.Arch, second.Arch) } diff --git a/tools/setup-envtest/versions/misc_test.go b/tools/setup-envtest/versions/misc_test.go index a609f4dc60..1c0dbb68db 100644 --- a/tools/setup-envtest/versions/misc_test.go +++ b/tools/setup-envtest/versions/misc_test.go @@ -36,13 +36,13 @@ var _ = Describe("Concrete", func() { Describe("when ordering relative to other versions", func() { ver1163 := Concrete{Major: 1, Minor: 16, Patch: 3} Specify("newer patch should be newer", func() { - Expect(ver1163.NewerThan(Concrete{Major: 1, Minor: 16})).To(BeTrue()) + Expect(ver1163.Compare(Concrete{Major: 1, Minor: 16})).To(Equal(1)) }) Specify("newer minor should be newer", func() { - Expect(ver1163.NewerThan(Concrete{Major: 1, Minor: 15, Patch: 3})).To(BeTrue()) + Expect(ver1163.Compare(Concrete{Major: 1, Minor: 15, Patch: 3})).To(Equal(1)) }) Specify("newer major should be newer", func() { - Expect(ver1163.NewerThan(Concrete{Major: 0, Minor: 16, Patch: 3})).To(BeTrue()) + Expect(ver1163.Compare(Concrete{Major: 0, Minor: 16, Patch: 3})).To(Equal(1)) }) }) }) diff --git a/tools/setup-envtest/versions/version.go b/tools/setup-envtest/versions/version.go index 945a95006f..bad05c3218 100644 --- a/tools/setup-envtest/versions/version.go +++ b/tools/setup-envtest/versions/version.go @@ -27,15 +27,24 @@ func (c Concrete) AsConcrete() *Concrete { return &c } -// NewerThan checks if the given other version is newer than this one. -func (c Concrete) NewerThan(other Concrete) bool { +// Compare checks if the given other version is newer than this one. +func (c Concrete) Compare(other Concrete) int { + IntCompare := func(a, b int) int { + if a > b { + return 1 + } else if a < b { + return -1 + } + return 0 + } + if c.Major != other.Major { - return c.Major > other.Major + return IntCompare(c.Major, other.Major) } if c.Minor != other.Minor { - return c.Minor > other.Minor + return IntCompare(c.Minor, other.Minor) } - return c.Patch > other.Patch + return IntCompare(c.Patch, other.Patch) } // Matches checks if this version is equal to the other one. diff --git a/tools/setup-envtest/workflows/workflows_test.go b/tools/setup-envtest/workflows/workflows_test.go index 435ae24285..eaa1c8c0a3 100644 --- a/tools/setup-envtest/workflows/workflows_test.go +++ b/tools/setup-envtest/workflows/workflows_test.go @@ -9,7 +9,7 @@ import ( "io/fs" "path/filepath" "runtime/debug" - "sort" + "slices" "strings" . "github.com/onsi/ginkgo/v2" @@ -333,7 +333,7 @@ var _ = Describe("Workflows", func() { archiveNames = append(archiveNames, archiveName) } } - sort.Strings(archiveNames) + slices.Sort(archiveNames) archiveNamesSet := sets.Set[string]{}.Insert(archiveNames[:7]...) // Delete all other archives for _, release := range remoteHTTPItems.index.Releases {