fixes

istio · Oct 30, 2024 · a2f9b9f · a2f9b9f
1 parent 79a7441
commit a2f9b9f
Show file tree

Hide file tree

Showing 7 changed files with 36 additions and 37 deletions.
diff --git a/pilot/pkg/serviceregistry/kube/controller/ambient/ambientindex.go b/pilot/pkg/serviceregistry/kube/controller/ambient/ambientindex.go
@@ -123,6 +123,8 @@ type Options struct {
 	LookupNetwork         LookupNetwork
 	LookupNetworkGateways LookupNetworkGateways
 	StatusNotifier        *activenotifier.ActiveNotifier
+
+	Debugger *krt.DebugHandler
 }
 
 func New(options Options) Index {
@@ -140,8 +142,7 @@ func New(options Options) Index {
 	filter := kclient.Filter{
 		ObjectFilter: options.Client.ObjectFilter(),
 	}
-	debugger := &krt.DebugHandler{}
-	withDebug := krt.WithDebugging(debugger)
+	withDebug := krt.WithDebugging(options.Debugger)
 	ConfigMaps := krt.NewInformerFiltered[*v1.ConfigMap](options.Client, filter, krt.WithName("ConfigMaps"), withDebug)
 
 	authzPolicies := kclient.NewDelayedInformer[*securityclient.AuthorizationPolicy](options.Client,

diff --git a/pilot/pkg/serviceregistry/kube/controller/ambient/ambientindex_test.go b/pilot/pkg/serviceregistry/kube/controller/ambient/ambientindex_test.go
@@ -27,6 +27,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	k8sv1 "sigs.k8s.io/gateway-api/apis/v1"
 	k8sbeta "sigs.k8s.io/gateway-api/apis/v1beta1"
+	"sigs.k8s.io/yaml"
 
 	"istio.io/api/annotation"
 	"istio.io/api/label"
@@ -1486,6 +1487,7 @@ func newAmbientTestServer(t *testing.T, clusterID cluster.ID, networkID network.
 	} {
 		clienttest.MakeCRD(t, cl, crd)
 	}
+	debugger := &krt.DebugHandler{}
 	idx := New(Options{
 		Client:          cl,
 		SystemNamespace: systemNS,
@@ -1499,19 +1501,12 @@ func newAmbientTestServer(t *testing.T, clusterID cluster.ID, networkID network.
 			return nil
 		},
 		StatusNotifier: activenotifier.New(true),
+		Debugger:       debugger,
 	})
 	idx.NetworksSynced()
 	cl.RunAndWait(test.NewStop(t))
 
-	t.Cleanup(func() {
-		if t.Failed() {
-			idx := idx.(*index)
-			krt.Dump(idx.authorizationPolicies)
-			krt.Dump(idx.workloads.Collection)
-			krt.Dump(idx.services.Collection)
-			krt.Dump(idx.waypoints.Collection)
-		}
-	})
+	dumpOnFailure(t, debugger)
 	a := &ambientTestServer{
 		t:         t,
 		clusterID: clusterID,
@@ -1551,6 +1546,15 @@ func newAmbientTestServer(t *testing.T, clusterID cluster.ID, networkID network.
 	return a
 }
 
+func dumpOnFailure(t *testing.T, debugger *krt.DebugHandler) {
+	t.Cleanup(func() {
+		if t.Failed() {
+			b, _ := yaml.Marshal(debugger)
+			t.Log(string(b))
+		}
+	})
+}
+
 func (s *ambientTestServer) addWaypoint(t *testing.T, ip, name, trafficType string, ready bool) {
 	s.addWaypointSpecificAddress(t, ip, fmt.Sprintf("%s.%s.svc.%s", name, testNS, s.DomainSuffix), name, trafficType, ready)
 }

diff --git a/pilot/pkg/serviceregistry/kube/controller/ambient/authorization_test.go b/pilot/pkg/serviceregistry/kube/controller/ambient/authorization_test.go
@@ -257,7 +257,7 @@ func TestWaypointPolicyStatusCollection(t *testing.T) {
 		}
 	})
 
-	wpsCollection := WaypointPolicyStatusCollection(authzPolCol, waypointCol, svcCol, seCol, nsCol, nil)
+	wpsCollection := WaypointPolicyStatusCollection(authzPolCol, waypointCol, svcCol, seCol, nsCol, krt.WithDebugging(&krt.DebugHandler{}))
 	c.RunAndWait(ctx.Done())
 
 	_, err := clientNs.Create(&v1.Namespace{

diff --git a/pilot/pkg/serviceregistry/kube/controller/controller.go b/pilot/pkg/serviceregistry/kube/controller/controller.go
@@ -49,6 +49,7 @@ import (
 	kubelib "istio.io/istio/pkg/kube"
 	"istio.io/istio/pkg/kube/controllers"
 	"istio.io/istio/pkg/kube/kclient"
+	"istio.io/istio/pkg/kube/krt"
 	istiolog "istio.io/istio/pkg/log"
 	"istio.io/istio/pkg/maps"
 	"istio.io/istio/pkg/monitoring"
@@ -290,6 +291,7 @@ func NewController(kubeClient kubelib.Client, options Options) *Controller {
 			LookupNetwork:         c.Network,
 			LookupNetworkGateways: c.NetworkGateways,
 			StatusNotifier:        options.StatusWritingEnabled,
+			Debugger:              krt.GlobalDebugHandler,
 		})
 	}
 	c.exports = newServiceExportCache(c)

diff --git a/pilot/pkg/xds/debug.go b/pilot/pkg/xds/debug.go
@@ -1083,7 +1083,7 @@ func (s *DiscoveryServer) ambientz(w http.ResponseWriter, req *http.Request) {
 }
 
 func (s *DiscoveryServer) krtz(w http.ResponseWriter, req *http.Request) {
-	writeJSON(w, krt.DebugCollections, req)
+	writeJSON(w, krt.GlobalDebugHandler, req)
 }
 
 func (s *DiscoveryServer) networkz(w http.ResponseWriter, req *http.Request) {

diff --git a/pkg/kube/krt/debug.go b/pkg/kube/krt/debug.go
@@ -2,14 +2,22 @@ package krt
 
 import (
 	"encoding/json"
-
-	"istio.io/istio/pkg/slices"
+	"sync"
 )
 
 // DebugHandler allows attaching a variety of collections to it and then dumping them
-type DebugHandler struct{}
+type DebugHandler struct {
+	debugCollections []DebugCollection
+	mu               sync.RWMutex
+}
+
+func (p *DebugHandler) MarshalJSON() ([]byte, error) {
+	p.mu.RLock()
+	defer p.mu.RUnlock()
+	return json.Marshal(p.debugCollections)
+}
 
-var DebugCollections = []DebugCollection{}
+var GlobalDebugHandler = new(DebugHandler)
 
 type CollectionDump struct {
 	// Map of output key -> output
@@ -35,31 +43,20 @@ func (p DebugCollection) MarshalJSON() ([]byte, error) {
 	})
 }
 
-func RegisterCollectionForDebugging[T any](c Collection[T]) {
-	cc := c.(internalCollection[T])
-	DebugCollections = append(DebugCollections, DebugCollection{
-		name: cc.name(),
-		dump: cc.dump,
-	})
-}
-
+// maybeRegisterCollectionForDebugging registers the collection in the debugger, if one is enabled
 func maybeRegisterCollectionForDebugging[T any](c Collection[T], handler *DebugHandler) {
 	if handler == nil {
 		return
 	}
 	cc := c.(internalCollection[T])
-	DebugCollections = append(DebugCollections, DebugCollection{
+	handler.mu.Lock()
+	defer handler.mu.Unlock()
+	handler.debugCollections = append(handler.debugCollections, DebugCollection{
 		name: cc.name(),
 		dump: cc.dump,
 	})
 }
 
-func eraseSlice[T any](l []T) []any {
-	return slices.Map(l, func(e T) any {
-		return any(e)
-	})
-}
-
 func eraseMap[T any](l map[Key[T]]T) map[string]any {
 	nm := make(map[string]any, len(l))
 	for k, v := range l {

diff --git a/pkg/kube/krt/testing.go b/pkg/kube/krt/testing.go
@@ -14,11 +14,6 @@
 
 package krt
 
-// Dump is a *testing* helper to dump the state of a collection, if possible, into logs.
-func Dump[O any](c Collection[O]) {
-	c.(internalCollection[O]).dump()
-}
-
 type TestingDummyContext struct{}
 
 func (t TestingDummyContext) _internalHandler() {