Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

krt: initial debug interface #53597

Merged
merged 6 commits into from
Nov 5, 2024
Merged

krt: initial debug interface #53597

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3074fd0
krt: initial debug interface
howardjohn Oct 21, 2024
e319633
make it more explicit
howardjohn Oct 30, 2024
7605d11
fixes
howardjohn Oct 30, 2024
85013e2
banner
howardjohn Oct 30, 2024
93bda6a
revert api change
howardjohn Oct 30, 2024
88c4623
lint
howardjohn Nov 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fixes
  • Loading branch information
@howardjohn
howardjohn committed Nov 5, 2024
commit 7605d11cc8a338d7b1261113864fa2004d7f18ba
5 changes: 3 additions & 2 deletions pilot/pkg/serviceregistry/kube/controller/ambient/ambientindex.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ type Options struct {
LookupNetwork LookupNetwork
LookupNetworkGateways LookupNetworkGateways
StatusNotifier *activenotifier.ActiveNotifier

Debugger *krt.DebugHandler
}

func New(options Options) Index {
Expand All @@ -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,
Expand Down
22 changes: 13 additions & 9 deletions pilot/pkg/serviceregistry/kube/controller/ambient/ambientindex_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion pilot/pkg/serviceregistry/kube/controller/ambient/authorization_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
2 changes: 2 additions & 0 deletions pilot/pkg/serviceregistry/kube/controller/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pilot/pkg/xds/debug.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
35 changes: 16 additions & 19 deletions pkg/kube/krt/debug.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it could be nice to have an env like KRT_GLOBAL_DEBUG that allows enabling debug on all collections without modifying code to apss in GlobalDebugHandler

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 {
Expand Down
5 changes: 0 additions & 5 deletions pkg/kube/krt/testing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down