Use sets.String replace sets.Set[string] (istio#47841)

* Use sets.String replace sets.Set[string] * Use %s print set * Address the problems from CR
WillardHu · Nov 16, 2023 · 7e24bf9 · 7e24bf9
1 parent b39c066
commit 7e24bf9
Show file tree

Hide file tree

Showing 17 changed files with 36 additions and 21 deletions.
diff --git a/cni/pkg/ambient/informers.go b/cni/pkg/ambient/informers.go
@@ -59,7 +59,7 @@ func (s *Server) Run(stop <-chan struct{}) {
 	<-stop
 }
 
-func (s *Server) ReconcileNamespaces() sets.Set[string] {
+func (s *Server) ReconcileNamespaces() sets.String {
 	processed := sets.New[string]()
 	for _, ns := range s.namespaces.List(metav1.NamespaceAll, klabels.Everything()) {
 		processed.Merge(s.enqueueNamespace(ns))
@@ -73,7 +73,7 @@ func (s *Server) EnqueueNamespace(o controllers.Object) {
 	s.enqueueNamespace(o)
 }
 
-func (s *Server) enqueueNamespace(o controllers.Object) sets.Set[string] {
+func (s *Server) enqueueNamespace(o controllers.Object) sets.String {
 	namespace := o.GetName()
 	matchAmbient := o.GetLabels()[constants.DataplaneMode] == constants.DataplaneModeAmbient
 	processed := sets.New[string]()

diff --git a/cni/pkg/ambient/net.go b/cni/pkg/ambient/net.go
@@ -281,7 +281,7 @@ func SetProc(path string, value string) error {
 	return os.WriteFile(path, []byte(value), 0o644)
 }
 
-func (s *Server) cleanStaleIPs(stales sets.Set[string]) {
+func (s *Server) cleanStaleIPs(stales sets.String) {
 	log.Infof("Ambient stale Pod IPs to be cleaned: %s", stales)
 	switch s.redirectMode {
 	case IptablesMode:

diff --git a/cni/pkg/ambient/net_linux.go b/cni/pkg/ambient/net_linux.go
@@ -1309,7 +1309,7 @@ func (s *Server) cleanupNode() {
 	}
 }
 
-func (s *Server) getEnrolledIPSets() sets.Set[string] {
+func (s *Server) getEnrolledIPSets() sets.String {
 	pods := sets.New[string]()
 	switch s.redirectMode {
 	case IptablesMode:

diff --git a/cni/pkg/ambient/server.go b/cni/pkg/ambient/server.go
@@ -91,7 +91,7 @@ func NewServer(ctx context.Context, args AmbientArgs) (*Server, error) {
 		s.ebpfServer.Start(ctx.Done())
 	}
 
-	log.Infof("Ambient enrolled IPs before reconciling: %+v", s.getEnrolledIPSets())
+	log.Infof("Ambient enrolled IPs before reconciling: %s", s.getEnrolledIPSets())
 
 	s.setupHandlers()
 

diff --git a/cni/pkg/ebpf/server/redirectionServer_linux.go b/cni/pkg/ebpf/server/redirectionServer_linux.go
@@ -674,7 +674,7 @@ func (r *RedirectServer) dumpZtunnelInfo() (*mapInfo, error) {
 	return &info, nil
 }
 
-func (r *RedirectServer) DumpAppIPs() sets.Set[string] {
+func (r *RedirectServer) DumpAppIPs() sets.String {
 	var keyOut [4]byte
 	var valueOut mapInfo
 	m := sets.New[string]()

diff --git a/cni/pkg/install/binaries.go b/cni/pkg/install/binaries.go
@@ -24,8 +24,8 @@ import (
 
 // Copies/mirrors any files present in a single source dir to N number of target dirs
 // and returns a set of the filenames copied.
-func copyBinaries(srcDir string, targetDirs []string) (sets.Set[string], error) {
-	copiedFilenames := sets.Set[string]{}
+func copyBinaries(srcDir string, targetDirs []string) (sets.String, error) {
+	copiedFilenames := sets.String{}
 	srcFiles, err := os.ReadDir(srcDir)
 	if err != nil {
 		return copiedFilenames, err

diff --git a/cni/pkg/install/install.go b/cni/pkg/install/install.go
@@ -47,7 +47,7 @@ func NewInstaller(cfg *config.InstallConfig, isReady *atomic.Value) *Installer {
 	}
 }
 
-func (in *Installer) installAll(ctx context.Context) (sets.Set[string], error) {
+func (in *Installer) installAll(ctx context.Context) (sets.String, error) {
 	// Install binaries
 	// Currently we _always_ do this, since the binaries do not live in a shared location
 	// and we harm no one by doing so.
@@ -179,7 +179,7 @@ func (in *Installer) Cleanup() error {
 // sleepWatchInstall  blocks until any file change for the binaries or config are detected.
 // At that point, the func yields so the caller can recheck the validity of the install.
 // If an error occurs or context is canceled, the function will return an error.
-func (in *Installer) sleepWatchInstall(ctx context.Context, installedBinFiles sets.Set[string]) error {
+func (in *Installer) sleepWatchInstall(ctx context.Context, installedBinFiles sets.String) error {
 	// Watch our specific binaries, in each configured binary dir.
 	// We may or may not be the only CNI plugin in play, and if we are not
 	// we shouldn't fire events for binaries that are not ours.

diff --git a/cni/pkg/install/install_test.go b/cni/pkg/install/install_test.go
@@ -173,7 +173,7 @@ func TestSleepCheckInstall(t *testing.T) {
 			}
 
 			t.Log("Expecting an invalid configuration log:")
-			err := in.sleepWatchInstall(ctx, sets.Set[string]{})
+			err := in.sleepWatchInstall(ctx, sets.String{})
 			if err != nil {
 				t.Fatalf("error should be nil due to invalid config, got: %v", err)
 			}
@@ -211,7 +211,7 @@ func TestSleepCheckInstall(t *testing.T) {
 			// Should detect a valid configuration and wait indefinitely for a file modification
 			errChan := make(chan error)
 			go func(ctx context.Context) {
-				errChan <- in.sleepWatchInstall(ctx, sets.Set[string]{})
+				errChan <- in.sleepWatchInstall(ctx, sets.String{})
 			}(ctx)
 
 			select {
@@ -251,7 +251,7 @@ func TestSleepCheckInstall(t *testing.T) {
 
 				// Run sleepWatchInstall
 				go func(ctx context.Context, in *Installer) {
-					errChan <- in.sleepWatchInstall(ctx, sets.Set[string]{})
+					errChan <- in.sleepWatchInstall(ctx, sets.String{})
 				}(ctx, in)
 			}
 

diff --git a/pilot/pkg/config/kube/gateway/deploymentcontroller.go b/pilot/pkg/config/kube/gateway/deploymentcontroller.go
@@ -502,7 +502,7 @@ func (d *DeploymentController) apply(controller string, yml string) error {
 	return nil
 }
 
-func (d *DeploymentController) HandleTagChange(newTags sets.Set[string]) {
+func (d *DeploymentController) HandleTagChange(newTags sets.String) {
 	for _, gw := range d.gateways.List(metav1.NamespaceAll, klabels.Everything()) {
 		d.queue.AddObject(gw)
 	}

diff --git a/pilot/pkg/model/network_test.go b/pilot/pkg/model/network_test.go
@@ -115,7 +115,7 @@ func TestGatewayHostnames(t *testing.T) {
 		xdsUpdater.WaitOrFail(t, "xds full")
 	})
 
-	workingDNSServer.setHosts(make(sets.Set[string]))
+	workingDNSServer.setHosts(make(sets.String))
 	t.Run("no answer", func(t *testing.T) {
 		assert.EventuallyEqual(t, func() int {
 			return len(env.NetworkManager.AllGateways())

diff --git a/pilot/pkg/model/push_context.go b/pilot/pkg/model/push_context.go
@@ -1710,7 +1710,7 @@ func (ps *PushContext) initVirtualServices(env *Environment) {
 					sets.InsertOrNew(ps.virtualServiceIndex.destinationsByGateway, gw, host)
 				}
 				if _, exists := ps.virtualServiceIndex.destinationsByGateway[gw]; !exists {
-					ps.virtualServiceIndex.destinationsByGateway[gw] = sets.Set[string]{}
+					ps.virtualServiceIndex.destinationsByGateway[gw] = sets.String{}
 				}
 				addHostsFromMeshConfig(ps, ps.virtualServiceIndex.destinationsByGateway[gw])
 			}

diff --git a/pilot/pkg/model/service.go b/pilot/pkg/model/service.go
@@ -862,7 +862,7 @@ type AmbientIndexes interface {
 		proxy *Proxy,
 		allAddresses sets.String,
 		currentSubs sets.String,
-	) sets.Set[string]
+	) sets.String
 	Policies(requested sets.Set[ConfigKey]) []*security.Authorization
 	Waypoint(scope WaypointScope) []netip.Addr
 	WorkloadsForWaypoint(scope WaypointScope) []*WorkloadInfo

diff --git a/pkg/revisions/tag_watcher.go b/pkg/revisions/tag_watcher.go
@@ -32,11 +32,11 @@ type TagWatcher interface {
 	Run(stopCh <-chan struct{})
 	HasSynced() bool
 	AddHandler(handler TagHandler)
-	GetMyTags() sets.Set[string]
+	GetMyTags() sets.String
 }
 
 // TagHandler is a callback for when the tags revision change.
-type TagHandler func(sets.Set[string])
+type TagHandler func(sets.String)
 
 type tagWatcher struct {
 	revision string

diff --git a/pkg/revisions/tag_watcher_test.go b/pkg/revisions/tag_watcher_test.go
@@ -36,7 +36,7 @@ func TestTagWatcher(t *testing.T) {
 	stop := test.NewStop(t)
 	c.RunAndWait(stop)
 	track := assert.NewTracker[string](t)
-	tw.AddHandler(func(s sets.Set[string]) {
+	tw.AddHandler(func(s sets.String) {
 		track.Record(strings.Join(sets.SortedList(s), ","))
 	})
 	go tw.Run(stop)

diff --git a/pkg/util/sets/set.go b/pkg/util/sets/set.go
@@ -15,6 +15,8 @@
 package sets
 
 import (
+	"fmt"
+
 	"golang.org/x/exp/constraints"
 
 	"istio.io/istio/pkg/slices"
@@ -233,6 +235,13 @@ func (s Set[T]) IsEmpty() bool {
 	return len(s) == 0
 }
 
+// String returns a string representation of the set.
+// Be aware that the order of elements is random so the string representation may vary.
+// Use it only for debugging and logging.
+func (s Set[T]) String() string {
+	return fmt.Sprintf("%v", s.UnsortedList())
+}
+
 // InsertOrNew inserts t into the set if the set exists, or returns a new set with t if not.
 // Works well with DeleteCleanupLast.
 // Example:

diff --git a/pkg/util/sets/set_test.go b/pkg/util/sets/set_test.go
@@ -338,3 +338,9 @@ func TestMapOfSet(t *testing.T) {
 	DeleteCleanupLast(m, 1, "not found")
 	assert.Equal(t, m, map[int]String{2: New("c")})
 }
+
+func TestSetString(t *testing.T) {
+	elements := []string{"a"}
+	set := New(elements...)
+	assert.Equal(t, "[a]", set.String())
+}
diff --git a/tools/bug-report/pkg/kubectlcmd/kubectlcmd.go b/tools/bug-report/pkg/kubectlcmd/kubectlcmd.go
@@ -45,7 +45,7 @@ type Runner struct {
 	taskSem chan struct{}
 
 	// runningTasks tracks the in-flight fetch operations for user feedback.
-	runningTasks   sets.Set[string]
+	runningTasks   sets.String
 	runningTasksMu sync.RWMutex
 
 	// runningTasksTicker is the report interval for running tasks.