gwctl performance improvements (kubernetes-sigs#3145)

* Fetch resources in bulk instead of single API calls as the default. Eg. When we have a set of HTTPRoutes and we want to fetch all Backends referenced by this HTTPRoute, it's slightly better to fetch all the Backends with one API call and then filter these out based on which are being referenced. This is in contrast with fetching each individual backend through a separate API call that is referenced by the HTTPRoutes. (Even at smaller scales, multiple API calls starts to seem inefficient) * Lazily fetch events instead of making them part of the ResourceModel. Events associated with a resource need to be fetch independently for each resource. This means the number of API calls grows linearly with the number of resources that we have. - Pre-fetching events for all resources BEFORE PRINTING ANYTHING gives the impression of the CLI being unresponsive. - Lazily fetching events allows us to print the describe-view for a single resource fully before fetching events for the next one, thereby giving the user an impression of the CLI making progress. Also, it makes sense for Events to not be part of the ResourceModel given events are just tied with a single resource and have no interdependence (so don't need some pre-calculations)
BobyMCbobs · Jul 10, 2024 · 6646f09 · 6646f09
1 parent 8d33a32
commit 6646f09
Show file tree

Hide file tree

Showing 11 changed files with 85 additions and 164 deletions.
diff --git a/gwctl/cmd/subcommand.go b/gwctl/cmd/subcommand.go
@@ -217,7 +217,7 @@ func runGetOrDescribeNamespaces(f cmdutils.Factory, o *getOrDescribeOptions) {
 	handleErrOrExitWithMsg(err, "failed to discover Namespace resources")
 
 	realClock := clock.RealClock{}
-	nsPrinter := &printer.NamespacesPrinter{Writer: o.out, Clock: realClock}
+	nsPrinter := &printer.NamespacesPrinter{Writer: o.out, Clock: realClock, EventFetcher: discoverer}
 	if o.cmdName == commandNameGet {
 		printer.Print(nsPrinter, resourceModel, o.outputFormat)
 	} else {
@@ -248,7 +248,7 @@ func runGetOrDescribeGatewayClasses(f cmdutils.Factory, o *getOrDescribeOptions)
 	handleErrOrExitWithMsg(err, "failed to discover GatewayClass resources")
 
 	realClock := clock.RealClock{}
-	gwcPrinter := &printer.GatewayClassesPrinter{Writer: o.out, Clock: realClock}
+	gwcPrinter := &printer.GatewayClassesPrinter{Writer: o.out, Clock: realClock, EventFetcher: discoverer}
 	if o.cmdName == commandNameGet {
 		printer.Print(gwcPrinter, resourceModel, o.outputFormat)
 	} else {
@@ -281,7 +281,7 @@ func runGetOrDescribeGateways(f cmdutils.Factory, o *getOrDescribeOptions) {
 	handleErrOrExitWithMsg(err, "failed to discover Gateway resources")
 
 	realClock := clock.RealClock{}
-	gwPrinter := &printer.GatewaysPrinter{Writer: o.out, Clock: realClock}
+	gwPrinter := &printer.GatewaysPrinter{Writer: o.out, Clock: realClock, EventFetcher: discoverer}
 	if o.cmdName == commandNameGet {
 		printer.Print(gwPrinter, resourceModel, o.outputFormat)
 	} else {
@@ -347,7 +347,7 @@ func runGetOrDescribeBackends(f cmdutils.Factory, o *getOrDescribeOptions) {
 	handleErrOrExitWithMsg(err, "failed to discover Backend resources")
 
 	realClock := clock.RealClock{}
-	backendsPrinter := &printer.BackendsPrinter{Writer: o.out, Clock: realClock}
+	backendsPrinter := &printer.BackendsPrinter{Writer: o.out, Clock: realClock, EventFetcher: discoverer}
 	if o.cmdName == commandNameGet {
 		printer.Print(backendsPrinter, resourceModel, o.outputFormat)
 	} else {

diff --git a/gwctl/pkg/printer/backends.go b/gwctl/pkg/printer/backends.go
@@ -17,6 +17,7 @@ limitations under the License.
 package printer
 
 import (
+	"context"
 	"fmt"
 	"io"
 	"strings"
@@ -31,7 +32,8 @@ import (
 
 type BackendsPrinter struct {
 	io.Writer
-	Clock clock.Clock
+	Clock        clock.Clock
+	EventFetcher eventFetcher
 }
 
 func (bp *BackendsPrinter) GetPrintableNodes(resourceModel *resourcediscovery.ResourceModel) []NodeResource {
@@ -155,7 +157,8 @@ func (bp *BackendsPrinter) PrintDescribeView(resourceModel *resourcediscovery.Re
 		}
 
 		// Events
-		pairs = append(pairs, &DescriberKV{Key: "Events", Value: convertEventsSliceToTable(backendNode.Events, bp.Clock)})
+		eventList := bp.EventFetcher.FetchEventsFor(context.Background(), backendNode.Backend)
+		pairs = append(pairs, &DescriberKV{Key: "Events", Value: convertEventsSliceToTable(eventList.Items, bp.Clock)})
 
 		Describe(bp, pairs)
 

diff --git a/gwctl/pkg/printer/common.go b/gwctl/pkg/printer/common.go
@@ -17,6 +17,7 @@ limitations under the License.
 package printer
 
 import (
+	"context"
 	"fmt"
 	"io"
 	"os"
@@ -208,3 +209,7 @@ func NodeResources[K NodeResource](items []K) []NodeResource {
 	}
 	return output
 }
+
+type eventFetcher interface {
+	FetchEventsFor(context.Context, client.Object) *corev1.EventList
+}
diff --git a/gwctl/pkg/printer/gatewayclasses.go b/gwctl/pkg/printer/gatewayclasses.go
@@ -17,6 +17,7 @@ limitations under the License.
 package printer
 
 import (
+	"context"
 	"fmt"
 	"io"
 
@@ -31,7 +32,8 @@ var _ Printer = (*GatewayClassesPrinter)(nil)
 
 type GatewayClassesPrinter struct {
 	io.Writer
-	Clock clock.Clock
+	Clock        clock.Clock
+	EventFetcher eventFetcher
 }
 
 func (gcp *GatewayClassesPrinter) GetPrintableNodes(resourceModel *resourcediscovery.ResourceModel) []NodeResource {
@@ -106,7 +108,8 @@ func (gcp *GatewayClassesPrinter) PrintDescribeView(resourceModel *resourcedisco
 		pairs = append(pairs, &DescriberKV{Key: "DirectlyAttachedPolicies", Value: convertPolicyRefsToTable(policyRefs)})
 
 		// Events
-		pairs = append(pairs, &DescriberKV{Key: "Events", Value: convertEventsSliceToTable(gatewayClassNode.Events, gcp.Clock)})
+		eventList := gcp.EventFetcher.FetchEventsFor(context.Background(), gatewayClassNode.GatewayClass)
+		pairs = append(pairs, &DescriberKV{Key: "Events", Value: convertEventsSliceToTable(eventList.Items, gcp.Clock)})
 
 		Describe(gcp, pairs)
 

diff --git a/gwctl/pkg/printer/gatewayclasses_test.go b/gwctl/pkg/printer/gatewayclasses_test.go
@@ -331,8 +331,9 @@ Events: <none>
 			}
 
 			gcp := &GatewayClassesPrinter{
-				Writer: buff,
-				Clock:  fakeClock,
+				Writer:       buff,
+				Clock:        fakeClock,
+				EventFetcher: discoverer,
 			}
 			gcp.PrintDescribeView(resourceModel)
 

diff --git a/gwctl/pkg/printer/gateways.go b/gwctl/pkg/printer/gateways.go
@@ -17,6 +17,7 @@ limitations under the License.
 package printer
 
 import (
+	"context"
 	"fmt"
 	"io"
 	"strings"
@@ -32,7 +33,8 @@ var _ Printer = (*GatewaysPrinter)(nil)
 
 type GatewaysPrinter struct {
 	io.Writer
-	Clock clock.Clock
+	Clock        clock.Clock
+	EventFetcher eventFetcher
 }
 
 func (gp *GatewaysPrinter) GetPrintableNodes(resourceModel *resourcediscovery.ResourceModel) []NodeResource {
@@ -152,7 +154,8 @@ func (gp *GatewaysPrinter) PrintDescribeView(resourceModel *resourcediscovery.Re
 		}
 
 		// Events
-		pairs = append(pairs, &DescriberKV{Key: "Events", Value: convertEventsSliceToTable(gatewayNode.Events, gp.Clock)})
+		eventList := gp.EventFetcher.FetchEventsFor(context.Background(), gatewayNode.Gateway)
+		pairs = append(pairs, &DescriberKV{Key: "Events", Value: convertEventsSliceToTable(eventList.Items, gp.Clock)})
 
 		Describe(gp, pairs)
 

diff --git a/gwctl/pkg/printer/gateways_test.go b/gwctl/pkg/printer/gateways_test.go
@@ -485,8 +485,9 @@ func TestGatewaysPrinter_PrintDescribeView(t *testing.T) {
 	}
 
 	gp := &GatewaysPrinter{
-		Writer: buff,
-		Clock:  fakeClock,
+		Writer:       buff,
+		Clock:        fakeClock,
+		EventFetcher: discoverer,
 	}
 	gp.PrintDescribeView(resourceModel)
 

diff --git a/gwctl/pkg/printer/namespace.go b/gwctl/pkg/printer/namespace.go
@@ -17,6 +17,7 @@ limitations under the License.
 package printer
 
 import (
+	"context"
 	"fmt"
 	"io"
 
@@ -31,7 +32,8 @@ var _ Printer = (*NamespacesPrinter)(nil)
 
 type NamespacesPrinter struct {
 	io.Writer
-	Clock clock.Clock
+	Clock        clock.Clock
+	EventFetcher eventFetcher
 }
 
 func (nsp *NamespacesPrinter) GetPrintableNodes(resourceModel *resourcediscovery.ResourceModel) []NodeResource {
@@ -94,7 +96,8 @@ func (nsp *NamespacesPrinter) PrintDescribeView(resourceModel *resourcediscovery
 		pairs = append(pairs, &DescriberKV{Key: "DirectlyAttachedPolicies", Value: convertPolicyRefsToTable(policyRefs)})
 
 		// Events
-		pairs = append(pairs, &DescriberKV{Key: "Events", Value: convertEventsSliceToTable(namespaceNode.Events, nsp.Clock)})
+		eventList := nsp.EventFetcher.FetchEventsFor(context.Background(), namespaceNode.Namespace)
+		pairs = append(pairs, &DescriberKV{Key: "Events", Value: convertEventsSliceToTable(eventList.Items, nsp.Clock)})
 
 		Describe(nsp, pairs)
 

diff --git a/gwctl/pkg/printer/namespace_test.go b/gwctl/pkg/printer/namespace_test.go
@@ -282,8 +282,9 @@ func TestNamespacePrinter_PrintDescribeView(t *testing.T) {
 	}
 
 	nsp := &NamespacesPrinter{
-		Writer: buff,
-		Clock:  fakeClock,
+		Writer:       buff,
+		Clock:        fakeClock,
+		EventFetcher: discoverer,
 	}
 	nsp.PrintDescribeView(resourceModel)