Skip to content

Support wide output format for get commands (-o wide) #3129

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

Merged
merged 5 commits into from
Jun 6, 2024
Merged

Support wide output format for get commands (-o wide) #3129

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6409eb6
Support wide output format for get commands (-o wide)
deszhou Jun 4, 2024
53ea74f
Add a case statement for OutputFormatWide type
deszhou Jun 4, 2024
bcd6bc5
Move REFERRED BY ROUTES to wide format
deszhou Jun 6, 2024
c825f15
Merge branch 'main' into support-wide-output-format
deszhou Jun 6, 2024
d0fe13b
Move REFERRED BY ROUTES to wide format
deszhou Jun 6, 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
2 changes: 1 addition & 1 deletion gwctl/cmd/subcommand.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ func runGetOrDescribeBackends(f cmdutils.Factory, o *getOrDescribeOptions) {
realClock := clock.RealClock{}
backendsPrinter := &printer.BackendsPrinter{Writer: o.out, Clock: realClock}
if o.cmdName == commandNameGet {
backendsPrinter.Print(resourceModel)
printer.Print(backendsPrinter, resourceModel, o.outputFormat)
} else {
backendsPrinter.PrintDescribeView(resourceModel)
}
Expand Down
21 changes: 17 additions & 4 deletions gwctl/pkg/printer/backends.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,20 @@ type BackendsPrinter struct {
Clock clock.Clock
}

func (bp *BackendsPrinter) Print(resourceModel *resourcediscovery.ResourceModel) {
func (bp *BackendsPrinter) GetPrintableNodes(resourceModel *resourcediscovery.ResourceModel) []NodeResource {
return NodeResources(maps.Values(resourceModel.GatewayClasses))
}

func (bp *BackendsPrinter) PrintTable(resourceModel *resourcediscovery.ResourceModel, wide bool) {
var columnNames []string
if wide {
columnNames = []string{"NAMESPACE", "NAME", "TYPE", "REFERRED BY ROUTES", "AGE", "POLICIES"}
} else {
columnNames = []string{"NAMESPACE", "NAME", "TYPE", "REFERRED BY ROUTES", "AGE"}
Copy link
Member

Choose a reason for hiding this comment

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

I know this is a bit different from what's documented but I've come to realize that we should avoid any parameter from being displayed which needs additional calculations.

Calculating things like "REFERRED BY ROUTES" and "POLICIES" will take additional time (which becomes significant when there's lot's of resources). Hence it's preferrable to only include them in the -o wide format and not as a default.

tl;dr: Can you please move REFERRED BY ROUTES to wide format as well?

(In case you are wondering, we'll separately work on not calculating the additional values by default int the resourceModel -- as of now, that distinction doesn't exist`

Copy link
Member Author

Choose a reason for hiding this comment

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

Of course, done!

}

table := &Table{
ColumnNames: []string{"NAMESPACE", "NAME", "TYPE", "REFERRED BY ROUTES", "AGE", "POLICIES"},
ColumnNames: columnNames,
UseSeparator: false,
}

Expand Down Expand Up @@ -73,15 +84,17 @@ func (bp *BackendsPrinter) Print(resourceModel *resourcediscovery.ResourceModel)
name := backend.GetName()
backendType := backend.GetKind()
age := duration.HumanDuration(bp.Clock.Since(backend.GetCreationTimestamp().Time))
policiesCount := fmt.Sprintf("%d", len(backendNode.Policies))

row := []string{
namespace,
name,
backendType,
referredByRoutes,
age,
policiesCount,
}
if wide {
policiesCount := fmt.Sprintf("%d", len(backendNode.Policies))
row = append(row, policiesCount)
}
table.Rows = append(table.Rows, row)
}
Expand Down
23 changes: 20 additions & 3 deletions gwctl/pkg/printer/backends_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,15 +196,32 @@ func TestBackendsPrinter_Print(t *testing.T) {
Clock: fakeClock,
}

bp.Print(resourceModel)
bp.PrintTable(resourceModel, false)

got := buff.String()
want := `
NAMESPACE NAME TYPE REFERRED BY ROUTES AGE
ns1 foo-svc-1 Service ns1/foo-httproute-1 3d
ns1 foo-svc-2 Service ns1/foo-httproute-2, ns1/foo-httproute-3 + 2 more 2d
`
if diff := cmp.Diff(common.YamlString(want), common.YamlString(got), common.YamlStringTransformer); diff != "" {
t.Errorf("Unexpected diff\ngot=\n%v\nwant=\n%v\ndiff (-want +got)=\n%v", got, want, diff)
}

buff.Reset()
nsp2 := &BackendsPrinter{
Writer: buff,
Clock: fakeClock,
}
nsp2.PrintTable(resourceModel, true)

got2 := buff.String()
want2 := `
NAMESPACE NAME TYPE REFERRED BY ROUTES AGE POLICIES
ns1 foo-svc-1 Service ns1/foo-httproute-1 3d 1
ns1 foo-svc-2 Service ns1/foo-httproute-2, ns1/foo-httproute-3 + 2 more 2d 0
`
if diff := cmp.Diff(common.YamlString(want), common.YamlString(got), common.YamlStringTransformer); diff != "" {
t.Errorf("Unexpected diff\ngot=\n%v\nwant=\n%v\ndiff (-want +got)=\n%v", got, want, diff)
if diff := cmp.Diff(common.YamlString(want2), common.YamlString(got2), common.YamlStringTransformer); diff != "" {
t.Errorf("Unexpected diff\ngot=\n%v\nwant=\n%v\ndiff (-want +got)=\n%v", got2, want2, diff)
}
}
14 changes: 12 additions & 2 deletions gwctl/pkg/printer/gatewayclasses.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,15 @@ func (gcp *GatewayClassesPrinter) GetPrintableNodes(resourceModel *resourcedisco
return NodeResources(maps.Values(resourceModel.GatewayClasses))
}

func (gcp *GatewayClassesPrinter) PrintTable(resourceModel *resourcediscovery.ResourceModel) {
func (gcp *GatewayClassesPrinter) PrintTable(resourceModel *resourcediscovery.ResourceModel, wide bool) {
var columnNames []string
if wide {
columnNames = []string{"NAME", "CONTROLLER", "ACCEPTED", "AGE", "GATEWAYS"}
} else {
columnNames = []string{"NAME", "CONTROLLER", "ACCEPTED", "AGE"}
}
table := &Table{
ColumnNames: []string{"NAME", "CONTROLLER", "ACCEPTED", "AGE"},
ColumnNames: columnNames,
UseSeparator: false,
}

Expand All @@ -62,6 +68,10 @@ func (gcp *GatewayClassesPrinter) PrintTable(resourceModel *resourcediscovery.Re
accepted,
age,
}
if wide {
gatewayCount := fmt.Sprintf("%d", len(gatewayClassNode.Gateways))
row = append(row, gatewayCount)
}
table.Rows = append(table.Rows, row)
}

Expand Down
50 changes: 49 additions & 1 deletion gwctl/pkg/printer/gatewayclasses_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,37 @@ func TestGatewayClassesPrinter_PrintTable(t *testing.T) {
},
},
},
&gatewayv1.Gateway{
ObjectMeta: metav1.ObjectMeta{
Name: "bar-gateway",
CreationTimestamp: metav1.Time{
Time: fakeClock.Now().Add(-3 * time.Second),
},
},
Spec: gatewayv1.GatewaySpec{
GatewayClassName: "bar-com-internal-gateway-class",
Listeners: []gatewayv1.Listener{
{
Name: gatewayv1.SectionName("http-8443"),
Protocol: gatewayv1.HTTPProtocolType,
Port: gatewayv1.PortNumber(8443),
},
},
},
Status: gatewayv1.GatewayStatus{
Addresses: []gatewayv1.GatewayStatusAddress{
{
Value: "10.11.12.13",
},
},
Conditions: []metav1.Condition{
{
Type: "Programmed",
Status: "Unknown",
},
},
},
},
}

k8sClients := common.MustClientsForTest(t, objects...)
Expand All @@ -117,7 +148,7 @@ func TestGatewayClassesPrinter_PrintTable(t *testing.T) {
Writer: buff,
Clock: fakeClock,
}
Print(gcp, resourceModel, utils.OutputFormatTable)
gcp.PrintTable(resourceModel, false)

got := buff.String()
want := `
Expand All @@ -129,6 +160,23 @@ foo-com-internal-gateway-class foo.com/internal-gateway-class Unknown 24m
if diff := cmp.Diff(common.YamlString(want), common.YamlString(got), common.YamlStringTransformer); diff != "" {
t.Errorf("Unexpected diff\ngot=\n%v\nwant=\n%v\ndiff (-want +got)=\n%v", got, want, diff)
}
buff.Reset()
gcp2 := &GatewayClassesPrinter{
Writer: buff,
Clock: fakeClock,
}
gcp2.PrintTable(resourceModel, true)

got2 := buff.String()
want2 := `
NAME CONTROLLER ACCEPTED AGE GATEWAYS
bar-com-internal-gateway-class bar.baz/internal-gateway-class True 365d 1
foo-com-external-gateway-class foo.com/external-gateway-class False 100d 0
foo-com-internal-gateway-class foo.com/internal-gateway-class Unknown 24m 0
`
if diff := cmp.Diff(common.YamlString(want2), common.YamlString(got2), common.YamlStringTransformer); diff != "" {
t.Errorf("Unexpected diff\ngot=\n%v\nwant=\n%v\ndiff (-want +got)=\n%v", got2, want2, diff)
}
}

func TestGatewayClassesPrinter_PrintDescribeView(t *testing.T) {
Expand Down
15 changes: 13 additions & 2 deletions gwctl/pkg/printer/gateways.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,15 @@ func (gp *GatewaysPrinter) GetPrintableNodes(resourceModel *resourcediscovery.Re
return NodeResources(maps.Values(resourceModel.Gateways))
}

func (gp *GatewaysPrinter) PrintTable(resourceModel *resourcediscovery.ResourceModel) {
func (gp *GatewaysPrinter) PrintTable(resourceModel *resourcediscovery.ResourceModel, wide bool) {
var columnNames []string
if wide {
columnNames = []string{"NAME", "CLASS", "ADDRESSES", "PORTS", "PROGRAMMED", "AGE", "POLICIES", "HTTPROUTES"}
} else {
columnNames = []string{"NAME", "CLASS", "ADDRESSES", "PORTS", "PROGRAMMED", "AGE"}
}
table := &Table{
ColumnNames: []string{"NAME", "CLASS", "ADDRESSES", "PORTS", "PROGRAMMED", "AGE"},
ColumnNames: columnNames,
UseSeparator: false,
}

Expand Down Expand Up @@ -81,6 +87,11 @@ func (gp *GatewaysPrinter) PrintTable(resourceModel *resourcediscovery.ResourceM
programmedStatus,
age,
}
if wide {
policiesCount := fmt.Sprintf("%d", len(gatewayNode.Policies))
httpRoutesCount := fmt.Sprintf("%d", len(gatewayNode.HTTPRoutes))
row = append(row, policiesCount, httpRoutesCount)
}
table.Rows = append(table.Rows, row)
}

Expand Down
104 changes: 103 additions & 1 deletion gwctl/pkg/printer/gateways_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,90 @@ func TestGatewaysPrinter_PrintTable(t *testing.T) {
},
},
},
&gatewayv1.HTTPRoute{
TypeMeta: metav1.TypeMeta{
Kind: "HTTPRoute",
},
ObjectMeta: metav1.ObjectMeta{
Name: "foo-httproute",
},
Spec: gatewayv1.HTTPRouteSpec{
CommonRouteSpec: gatewayv1.CommonRouteSpec{
ParentRefs: []gatewayv1.ParentReference{{
Kind: common.PtrTo(gatewayv1.Kind("Gateway")),
Group: common.PtrTo(gatewayv1.Group("gateway.networking.k8s.io")),
Name: "abc-gateway-12345",
}},
},
},
},

&apiextensionsv1.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{
Name: "healthcheckpolicies.foo.com",
Labels: map[string]string{
gatewayv1alpha2.PolicyLabelKey: "inherited",
},
},
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
Scope: apiextensionsv1.ClusterScoped,
Group: "foo.com",
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{{Name: "v1"}},
Names: apiextensionsv1.CustomResourceDefinitionNames{
Plural: "healthcheckpolicies",
Kind: "HealthCheckPolicy",
},
},
},
&unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "foo.com/v1",
"kind": "HealthCheckPolicy",
"metadata": map[string]interface{}{
"name": "health-check-gatewayclass",
},
"spec": map[string]interface{}{
"override": map[string]interface{}{
"key1": "value-parent-1",
"key3": "value-parent-3",
"key5": "value-parent-5",
},
"default": map[string]interface{}{
"key2": "value-parent-2",
"key4": "value-parent-4",
},
"targetRef": map[string]interface{}{
"group": "gateway.networking.k8s.io",
"kind": "GatewayClass",
"name": "regional-internal-class",
},
},
},
},
&unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "foo.com/v1",
"kind": "HealthCheckPolicy",
"metadata": map[string]interface{}{
"name": "health-check-gateway",
},
"spec": map[string]interface{}{
"override": map[string]interface{}{
"key1": "value-child-1",
},
"default": map[string]interface{}{
"key2": "value-child-2",
"key5": "value-child-5",
},
"targetRef": map[string]interface{}{
"group": "gateway.networking.k8s.io",
"kind": "Gateway",
"name": "random-gateway",
"namespace": "default",
},
},
},
},
}

k8sClients := common.MustClientsForTest(t, objects...)
Expand All @@ -191,7 +275,7 @@ func TestGatewaysPrinter_PrintTable(t *testing.T) {
Writer: buff,
Clock: fakeClock,
}
gp.PrintTable(resourceModel)
gp.PrintTable(resourceModel, false)

got := buff.String()
want := `
Expand All @@ -204,6 +288,24 @@ random-gateway regional-internal-class 10.11.12.13 8443
if diff := cmp.Diff(common.YamlString(want), common.YamlString(got), common.YamlStringTransformer); diff != "" {
t.Errorf("Unexpected diff\ngot=\n%v\nwant=\n%v\ndiff (-want +got)=\n%v", got, want, diff)
}

buff.Reset()
nsp2 := &GatewaysPrinter{
Writer: buff,
Clock: fakeClock,
}
nsp2.PrintTable(resourceModel, true)

got2 := buff.String()
want2 := `
NAME CLASS ADDRESSES PORTS PROGRAMMED AGE POLICIES HTTPROUTES
abc-gateway-12345 internal-class 192.168.100.5 443,8080 False 20d 0 1
demo-gateway-2 external-class 10.0.0.1,10.0.0.2 + 1 more 80 True 5d 0 0
random-gateway regional-internal-class 10.11.12.13 8443 Unknown 3s 1 0
`
if diff := cmp.Diff(common.YamlString(want2), common.YamlString(got2), common.YamlStringTransformer); diff != "" {
t.Errorf("Unexpected diff\ngot=\n%v\nwant=\n%v\ndiff (-want +got)=\n%v", got2, want2, diff)
}
}

func TestGatewaysPrinter_PrintDescribeView(t *testing.T) {
Expand Down
16 changes: 13 additions & 3 deletions gwctl/pkg/printer/httproutes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,18 @@ func (hp *HTTPRoutesPrinter) GetPrintableNodes(resourceModel *resourcediscovery.
return NodeResources(maps.Values(resourceModel.HTTPRoutes))
}

func (hp *HTTPRoutesPrinter) PrintTable(resourceModel *resourcediscovery.ResourceModel) {
func (hp *HTTPRoutesPrinter) PrintTable(resourceModel *resourcediscovery.ResourceModel, wide bool) {
var columnNames []string
if wide {
columnNames = []string{"NAMESPACE", "NAME", "HOSTNAMES", "PARENT REFS", "AGE", "POLICIES"}
} else {
columnNames = []string{"NAMESPACE", "NAME", "HOSTNAMES", "PARENT REFS", "AGE"}
}

table := &Table{
ColumnNames: []string{"NAMESPACE", "NAME", "HOSTNAMES", "PARENT REFS", "AGE"},
ColumnNames: columnNames,
UseSeparator: false,
}

httpRouteNodes := maps.Values(resourceModel.HTTPRoutes)

for _, httpRouteNode := range SortByString(httpRouteNodes) {
Expand Down Expand Up @@ -76,6 +82,10 @@ func (hp *HTTPRoutesPrinter) PrintTable(resourceModel *resourcediscovery.Resourc
parentRefsCount,
age,
}
if wide {
policiesCount := fmt.Sprintf("%d", len(httpRouteNode.Policies))
row = append(row, policiesCount)
}
table.Rows = append(table.Rows, row)
}
table.Write(hp, 0)
Expand Down
Loading