From b752f15474bdbce4636d33efdb415dd6539d7e25 Mon Sep 17 00:00:00 2001 From: toversus Date: Thu, 15 Aug 2019 11:00:55 +0900 Subject: [PATCH 01/10] Make `kn service list --all-namespace` print namespace column `kn service list` should print NAMESPACE column when specifying `--all-namespace` option, which follows the kubectl convention. --- pkg/kn/commands/human_readable_flags.go | 11 ++- .../commands/service/human_readable_flags.go | 34 +++++-- pkg/kn/commands/service/service_list.go | 3 + pkg/kn/commands/service/service_list_flags.go | 8 +- .../service/service_list_mock_test.go | 88 +++++++++++++++++++ pkg/kn/commands/service/service_list_test.go | 35 ++++++-- pkg/printers/interface.go | 1 + 7 files changed, 160 insertions(+), 20 deletions(-) create mode 100644 pkg/kn/commands/service/service_list_mock_test.go diff --git a/pkg/kn/commands/human_readable_flags.go b/pkg/kn/commands/human_readable_flags.go index 309a71d8fa..cb1a21be37 100644 --- a/pkg/kn/commands/human_readable_flags.go +++ b/pkg/kn/commands/human_readable_flags.go @@ -30,7 +30,7 @@ import ( // Given the following flag values, a printer can be requested that knows // how to handle printing based on these values. type HumanPrintFlags struct { - //TODO: Add more flags as required + AllNamespaces bool } // AllowedFormats returns more customized formating options @@ -42,7 +42,7 @@ func (f *HumanPrintFlags) AllowedFormats() []string { // ToPrinter receives returns a printer capable of // handling human-readable output. func (f *HumanPrintFlags) ToPrinter(getHandlerFunc func(h hprinters.PrintHandler)) (hprinters.ResourcePrinter, error) { - p := hprinters.NewTablePrinter(hprinters.PrintOptions{}) + p := hprinters.NewTablePrinter(hprinters.PrintOptions{f.AllNamespaces}) getHandlerFunc(p) return p, nil } @@ -50,7 +50,12 @@ func (f *HumanPrintFlags) ToPrinter(getHandlerFunc func(h hprinters.PrintHandler // AddFlags receives a *cobra.Command reference and binds // flags related to human-readable printing to it func (f *HumanPrintFlags) AddFlags(c *cobra.Command) { - //TODO: Add more flags as required + // Bind AllNamespaces flag + all, err := c.Flags().GetBool("all-namespaces") + if err != nil { + fmt.Fprintln(c.OutOrStdout(), err) + } + f.AllNamespaces = all } // NewHumanPrintFlags returns flags associated with diff --git a/pkg/kn/commands/service/human_readable_flags.go b/pkg/kn/commands/service/human_readable_flags.go index 50f8387663..6496e5f31d 100644 --- a/pkg/kn/commands/service/human_readable_flags.go +++ b/pkg/kn/commands/service/human_readable_flags.go @@ -22,19 +22,30 @@ import ( servingv1alpha1 "knative.dev/serving/pkg/apis/serving/v1alpha1" ) +var baseKServiceColumnDefinitions = []metav1beta1.TableColumnDefinition{ + {Name: "Name", Type: "string", Description: "Name of the Knative service."}, + {Name: "Url", Type: "string", Description: "URL of the Knative service."}, + //{Name: "LastCreatedRevision", Type: "string", Description: "Name of last revision created."}, + //{Name: "LastReadyRevision", Type: "string", Description: "Name of last ready revision."}, + {Name: "Generation", Type: "integer", Description: "Sequence number of 'Generation' of the service that was last processed by the controller."}, + {Name: "Age", Type: "string", Description: "Age of the service."}, + {Name: "Conditions", Type: "string", Description: "Conditions describing statuses of service components."}, + {Name: "Ready", Type: "string", Description: "Ready condition status of the service."}, + {Name: "Reason", Type: "string", Description: "Reason for non-ready condition of the service."}, +} + // ServiceListHandlers adds print handlers for service list command func ServiceListHandlers(h hprinters.PrintHandler) { + kServiceColumnDefinitions := baseKServiceColumnDefinitions + h.TableHandler(kServiceColumnDefinitions, printKService) + h.TableHandler(kServiceColumnDefinitions, printKServiceList) +} + +func ServiceListAllNamespaceHandlers(h hprinters.PrintHandler) { kServiceColumnDefinitions := []metav1beta1.TableColumnDefinition{ - {Name: "Name", Type: "string", Description: "Name of the Knative service."}, - {Name: "Url", Type: "string", Description: "URL of the Knative service."}, - //{Name: "LastCreatedRevision", Type: "string", Description: "Name of last revision created."}, - //{Name: "LastReadyRevision", Type: "string", Description: "Name of last ready revision."}, - {Name: "Generation", Type: "integer", Description: "Sequence number of 'Generation' of the service that was last processed by the controller."}, - {Name: "Age", Type: "string", Description: "Age of the service."}, - {Name: "Conditions", Type: "string", Description: "Conditions describing statuses of service components."}, - {Name: "Ready", Type: "string", Description: "Ready condition status of the service."}, - {Name: "Reason", Type: "string", Description: "Reason for non-ready condition of the service."}, + {Name: "Namespace", Type: "string", Description: "Namespace of the Knative service"}, } + kServiceColumnDefinitions = append(kServiceColumnDefinitions, baseKServiceColumnDefinitions...) h.TableHandler(kServiceColumnDefinitions, printKService) h.TableHandler(kServiceColumnDefinitions, printKServiceList) } @@ -69,6 +80,11 @@ func printKService(kService *servingv1alpha1.Service, options hprinters.PrintOpt row := metav1beta1.TableRow{ Object: runtime.RawExtension{Object: kService}, } + + if options.AllNamespaces { + row.Cells = append(row.Cells, kService.Namespace) + } + row.Cells = append(row.Cells, name, url, diff --git a/pkg/kn/commands/service/service_list.go b/pkg/kn/commands/service/service_list.go index 73fe4b43b2..d38dcf1068 100644 --- a/pkg/kn/commands/service/service_list.go +++ b/pkg/kn/commands/service/service_list.go @@ -40,6 +40,9 @@ func NewServiceListCommand(p *commands.KnParams) *cobra.Command { # List service 'web' kn service list web`, + PreRun: func(cmd *cobra.Command, args []string) { + serviceListFlags.HumanReadableFlags.AddFlags(cmd) + }, RunE: func(cmd *cobra.Command, args []string) error { namespace, err := p.GetNamespace(cmd) if err != nil { diff --git a/pkg/kn/commands/service/service_list_flags.go b/pkg/kn/commands/service/service_list_flags.go index 77f16e569b..4486ce5101 100644 --- a/pkg/kn/commands/service/service_list_flags.go +++ b/pkg/kn/commands/service/service_list_flags.go @@ -47,6 +47,13 @@ func (f *ServiceListFlags) ToPrinter() (hprinters.ResourcePrinter, error) { return p, nil } // if no flags specified, use the table printing + if f.HumanReadableFlags.AllNamespaces { + p, err := f.HumanReadableFlags.ToPrinter(ServiceListAllNamespaceHandlers) + if err != nil { + return nil, err + } + return p, nil + } p, err := f.HumanReadableFlags.ToPrinter(ServiceListHandlers) if err != nil { return nil, err @@ -58,7 +65,6 @@ func (f *ServiceListFlags) ToPrinter() (hprinters.ResourcePrinter, error) { // flags related to humanreadable and template printing. func (f *ServiceListFlags) AddFlags(cmd *cobra.Command) { f.GenericPrintFlags.AddFlags(cmd) - f.HumanReadableFlags.AddFlags(cmd) } // NewServiceListFlags returns flags associated with humanreadable, diff --git a/pkg/kn/commands/service/service_list_mock_test.go b/pkg/kn/commands/service/service_list_mock_test.go new file mode 100644 index 0000000000..a171659a0e --- /dev/null +++ b/pkg/kn/commands/service/service_list_mock_test.go @@ -0,0 +1,88 @@ +// Copyright © 2019 The Knative Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package service + +import ( + "fmt" + "strings" + "testing" + + knclient "github.com/knative/client/pkg/serving/v1alpha1" + "github.com/knative/client/pkg/util" + "github.com/knative/serving/pkg/apis/serving/v1alpha1" + "gotest.tools/assert" + "k8s.io/apimachinery/pkg/api/errors" +) + +func TestServiceListAllNamespaceMock(t *testing.T) { + client := knclient.NewMockKnClient(t) + + r := client.Recorder() + + r.GetService("svc1", nil, errors.NewNotFound(v1alpha1.Resource("service"), "svc1")) + r.CreateService(knclient.Any(), nil) + r.WaitForService("svc1", knclient.Any(), nil) + r.GetService("svc1", getServiceWithNamespace("svc1", "ns1"), nil) + + r.GetService("svc2", nil, errors.NewNotFound(v1alpha1.Resource("service"), "svc2")) + r.CreateService(knclient.Any(), nil) + r.WaitForService("svc2", knclient.Any(), nil) + r.GetService("svc2", getServiceWithNamespace("svc2", "ns2"), nil) + + r.GetService("svc3", nil, errors.NewNotFound(v1alpha1.Resource("service"), "svc3")) + r.CreateService(knclient.Any(), nil) + r.WaitForService("svc3", knclient.Any(), nil) + r.GetService("svc3", getServiceWithNamespace("svc3", "ns3"), nil) + + r.ListServices(knclient.Any(), &v1alpha1.ServiceList{ + Items: []v1alpha1.Service{ + *getServiceWithNamespace("svc1", "ns1"), + *getServiceWithNamespace("svc2", "ns2"), + *getServiceWithNamespace("svc3", "ns3"), + }, + }, nil) + + output, err := executeServiceCommand(client, "create", "svc1", "--image", "gcr.io/foo/bar:baz", "--namespace", "ns1") + assert.NilError(t, err) + assert.Assert(t, util.ContainsAll(output, "created", "svc1", "ns1", "Waiting")) + + output, err = executeServiceCommand(client, "create", "svc2", "--image", "gcr.io/foo/bar:baz", "--namespace", "ns2") + assert.NilError(t, err) + assert.Assert(t, util.ContainsAll(output, "created", "svc2", "ns2", "Waiting")) + + output, err = executeServiceCommand(client, "create", "svc3", "--image", "gcr.io/foo/bar:baz", "--namespace", "ns3") + assert.NilError(t, err) + assert.Assert(t, util.ContainsAll(output, "created", "svc3", "ns3", "Waiting")) + + output, err = executeServiceCommand(client, "list", "--all-namespaces") + assert.NilError(t, err) + + outputLines := strings.Split(output, "\n") + assert.Assert(t, util.ContainsAll(outputLines[0], "NAMESPACE", "NAME", "URL", "GENERATION", "AGE", "CONDITIONS", "READY", "REASON")) + for i, line := range outputLines[1 : len(outputLines)-1] { + ns := fmt.Sprintf("ns%d", i+1) + svc := fmt.Sprintf("svc%d", i+1) + assert.Check(t, util.ContainsAll(line, ns, svc)) + } + + r.Validate() +} + +func getServiceWithNamespace(name, namespace string) *v1alpha1.Service { + service := v1alpha1.Service{} + service.Name = name + service.Namespace = namespace + return &service +} diff --git a/pkg/kn/commands/service/service_list_test.go b/pkg/kn/commands/service/service_list_test.go index 6414519643..c7c7fba324 100644 --- a/pkg/kn/commands/service/service_list_test.go +++ b/pkg/kn/commands/service/service_list_test.go @@ -76,9 +76,9 @@ func TestGetEmpty(t *testing.T) { } func TestServiceListDefaultOutput(t *testing.T) { - service1 := createMockServiceWithParams("foo", "http://foo.default.example.com", 2) - service3 := createMockServiceWithParams("sss", "http://sss.default.example.com", 3) - service2 := createMockServiceWithParams("bar", "http://bar.default.example.com", 1) + service1 := createMockServiceWithParams("foo", "default", "http://foo.default.example.com", 2) + service3 := createMockServiceWithParams("sss", "default", "http://sss.default.example.com", 3) + service2 := createMockServiceWithParams("bar", "default", "http://bar.default.example.com", 1) serviceList := &v1alpha1.ServiceList{Items: []v1alpha1.Service{*service1, *service2, *service3}} action, output, err := fakeServiceList([]string{"service", "list"}, serviceList) if err != nil { @@ -96,8 +96,29 @@ func TestServiceListDefaultOutput(t *testing.T) { assert.Check(t, util.ContainsAll(output[3], "sss", "sss.default.example.com", "3")) } +func TestServiceListAllNamespacesOutput(t *testing.T) { + service1 := createMockServiceWithParams("foo", "ns1", "http://foo.ns1.example.com", 1) + service2 := createMockServiceWithParams("bar", "ns2", "http://bar.ns2.example.com", 2) + service3 := createMockServiceWithParams("sss", "ns3", "http://sss.ns3.example.com", 3) + serviceList := &v1alpha1.ServiceList{Items: []v1alpha1.Service{*service1, *service2, *service3}} + action, output, err := fakeServiceList([]string{"service", "list", "--all-namespaces"}, serviceList) + if err != nil { + t.Fatal(err) + } + if action == nil { + t.Errorf("No action") + } else if !action.Matches("list", "services") { + t.Errorf("Bad action %v", action) + } + // Outputs in alphabetical order + assert.Check(t, util.ContainsAll(output[0], "NAMESPACE", "NAME", "URL", "GENERATION", "AGE", "CONDITIONS", "READY", "REASON")) + assert.Check(t, util.ContainsAll(output[1], "ns2", "bar", "bar.ns2.example.com", "2")) + assert.Check(t, util.ContainsAll(output[2], "ns1", "foo", "foo.ns1.example.com", "1")) + assert.Check(t, util.ContainsAll(output[3], "ns3", "sss", "sss.ns3.example.com", "3")) +} + func TestServiceGetOneOutput(t *testing.T) { - service := createMockServiceWithParams("foo", "foo.default.example.com", 1) + service := createMockServiceWithParams("foo", "default", "foo.default.example.com", 1) serviceList := &v1alpha1.ServiceList{Items: []v1alpha1.Service{*service}} action, output, err := fakeServiceList([]string{"service", "list", "foo"}, serviceList) if err != nil { @@ -113,13 +134,13 @@ func TestServiceGetOneOutput(t *testing.T) { } func TestServiceGetWithTwoSrvName(t *testing.T) { - service := createMockServiceWithParams("foo", "foo.default.example.com", 1) + service := createMockServiceWithParams("foo", "default", "foo.default.example.com", 1) serviceList := &v1alpha1.ServiceList{Items: []v1alpha1.Service{*service}} _, _, err := fakeServiceList([]string{"service", "list", "foo", "bar"}, serviceList) assert.ErrorContains(t, err, "'kn service list' accepts maximum 1 argument") } -func createMockServiceWithParams(name, urlS string, generation int64) *v1alpha1.Service { +func createMockServiceWithParams(name, namespace, urlS string, generation int64) *v1alpha1.Service { url, _ := apis.ParseURL(urlS) service := &v1alpha1.Service{ TypeMeta: metav1.TypeMeta{ @@ -128,7 +149,7 @@ func createMockServiceWithParams(name, urlS string, generation int64) *v1alpha1. }, ObjectMeta: metav1.ObjectMeta{ Name: name, - Namespace: "default", + Namespace: namespace, }, Spec: v1alpha1.ServiceSpec{ DeprecatedRunLatest: &v1alpha1.RunLatestType{}, diff --git a/pkg/printers/interface.go b/pkg/printers/interface.go index 70aa97b4dd..1127b20d51 100644 --- a/pkg/printers/interface.go +++ b/pkg/printers/interface.go @@ -39,4 +39,5 @@ func (fn ResourcePrinterFunc) PrintObj(obj runtime.Object, w io.Writer) error { // PrintOptions for different table printing options type PrintOptions struct { //TODO: Add options for eg: with-kind, server-printing, wide etc + AllNamespaces bool } From 69672833359a6e493952d484722cd276577f1a11 Mon Sep 17 00:00:00 2001 From: toversus Date: Thu, 15 Aug 2019 18:10:30 +0900 Subject: [PATCH 02/10] Put services in default namespace at the top of the list --- .../commands/service/human_readable_flags.go | 44 +++++++++++++++++++ .../service/service_list_mock_test.go | 35 +++++++-------- pkg/kn/commands/service/service_list_test.go | 12 ++--- 3 files changed, 66 insertions(+), 25 deletions(-) diff --git a/pkg/kn/commands/service/human_readable_flags.go b/pkg/kn/commands/service/human_readable_flags.go index 6496e5f31d..c6f28048d7 100644 --- a/pkg/kn/commands/service/human_readable_flags.go +++ b/pkg/kn/commands/service/human_readable_flags.go @@ -15,6 +15,11 @@ package service import ( + "sort" + + "github.com/knative/client/pkg/kn/commands" + hprinters "github.com/knative/client/pkg/printers" + servingv1alpha1 "github.com/knative/serving/pkg/apis/serving/v1alpha1" metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1" "k8s.io/apimachinery/pkg/runtime" "knative.dev/client/pkg/kn/commands" @@ -41,6 +46,7 @@ func ServiceListHandlers(h hprinters.PrintHandler) { h.TableHandler(kServiceColumnDefinitions, printKServiceList) } +// ServiceListAllNamespaceHandlers adds print handlers for service list command with all namespaces option func ServiceListAllNamespaceHandlers(h hprinters.PrintHandler) { kServiceColumnDefinitions := []metav1beta1.TableColumnDefinition{ {Name: "Namespace", Type: "string", Description: "Namespace of the Knative service"}, @@ -55,6 +61,11 @@ func ServiceListAllNamespaceHandlers(h hprinters.PrintHandler) { // printKServiceList populates the knative service list table rows func printKServiceList(kServiceList *servingv1alpha1.ServiceList, options hprinters.PrintOptions) ([]metav1beta1.TableRow, error) { rows := make([]metav1beta1.TableRow, 0, len(kServiceList.Items)) + + if options.AllNamespaces { + return printKServiceWithNaemspace(kServiceList, options) + } + for _, ksvc := range kServiceList.Items { r, err := printKService(&ksvc, options) if err != nil { @@ -65,6 +76,39 @@ func printKServiceList(kServiceList *servingv1alpha1.ServiceList, options hprint return rows, nil } +// printKServiceWithNaemspace populates the knative service table rows with namespace column +func printKServiceWithNaemspace(kServiceList *servingv1alpha1.ServiceList, options hprinters.PrintOptions) ([]metav1beta1.TableRow, error) { + rows := make([]metav1beta1.TableRow, 0, len(kServiceList.Items)) + + // temporary slice for sorting services in non-default namespace + others := []metav1beta1.TableRow{} + + for _, ksvc := range kServiceList.Items { + // Fill in with services in `default` namespace at first + if ksvc.Namespace == "default" { + r, err := printKService(&ksvc, options) + if err != nil { + return nil, err + } + rows = append(rows, r...) + continue + } + // put other services in temporary slice + r, err := printKService(&ksvc, options) + if err != nil { + return nil, err + } + others = append(others, r...) + } + + // sort other services list alphabetically by namespace name + sort.SliceStable(others, func(i, j int) bool { + return others[i].Cells[0].(string) < others[j].Cells[0].(string) + }) + + return append(rows, others...), nil +} + // printKService populates the knative service table rows func printKService(kService *servingv1alpha1.Service, options hprinters.PrintOptions) ([]metav1beta1.TableRow, error) { name := kService.Name diff --git a/pkg/kn/commands/service/service_list_mock_test.go b/pkg/kn/commands/service/service_list_mock_test.go index a171659a0e..71813e8cfe 100644 --- a/pkg/kn/commands/service/service_list_mock_test.go +++ b/pkg/kn/commands/service/service_list_mock_test.go @@ -15,7 +15,6 @@ package service import ( - "fmt" "strings" "testing" @@ -34,48 +33,46 @@ func TestServiceListAllNamespaceMock(t *testing.T) { r.GetService("svc1", nil, errors.NewNotFound(v1alpha1.Resource("service"), "svc1")) r.CreateService(knclient.Any(), nil) r.WaitForService("svc1", knclient.Any(), nil) - r.GetService("svc1", getServiceWithNamespace("svc1", "ns1"), nil) + r.GetService("svc1", getServiceWithNamespace("svc1", "default"), nil) - r.GetService("svc2", nil, errors.NewNotFound(v1alpha1.Resource("service"), "svc2")) + r.GetService("svc2", nil, errors.NewNotFound(v1alpha1.Resource("service"), "foo")) r.CreateService(knclient.Any(), nil) r.WaitForService("svc2", knclient.Any(), nil) - r.GetService("svc2", getServiceWithNamespace("svc2", "ns2"), nil) + r.GetService("svc2", getServiceWithNamespace("svc2", "foo"), nil) r.GetService("svc3", nil, errors.NewNotFound(v1alpha1.Resource("service"), "svc3")) r.CreateService(knclient.Any(), nil) r.WaitForService("svc3", knclient.Any(), nil) - r.GetService("svc3", getServiceWithNamespace("svc3", "ns3"), nil) + r.GetService("svc3", getServiceWithNamespace("svc3", "bar"), nil) r.ListServices(knclient.Any(), &v1alpha1.ServiceList{ Items: []v1alpha1.Service{ - *getServiceWithNamespace("svc1", "ns1"), - *getServiceWithNamespace("svc2", "ns2"), - *getServiceWithNamespace("svc3", "ns3"), + *getServiceWithNamespace("svc1", "default"), + *getServiceWithNamespace("svc2", "foo"), + *getServiceWithNamespace("svc3", "bar"), }, }, nil) - output, err := executeServiceCommand(client, "create", "svc1", "--image", "gcr.io/foo/bar:baz", "--namespace", "ns1") + output, err := executeServiceCommand(client, "create", "svc1", "--image", "gcr.io/foo/bar:baz", "--namespace", "default") assert.NilError(t, err) - assert.Assert(t, util.ContainsAll(output, "created", "svc1", "ns1", "Waiting")) + assert.Assert(t, util.ContainsAll(output, "created", "svc1", "default", "Waiting")) - output, err = executeServiceCommand(client, "create", "svc2", "--image", "gcr.io/foo/bar:baz", "--namespace", "ns2") + output, err = executeServiceCommand(client, "create", "svc2", "--image", "gcr.io/foo/bar:baz", "--namespace", "foo") assert.NilError(t, err) - assert.Assert(t, util.ContainsAll(output, "created", "svc2", "ns2", "Waiting")) + assert.Assert(t, util.ContainsAll(output, "created", "svc2", "foo", "Waiting")) - output, err = executeServiceCommand(client, "create", "svc3", "--image", "gcr.io/foo/bar:baz", "--namespace", "ns3") + output, err = executeServiceCommand(client, "create", "svc3", "--image", "gcr.io/foo/bar:baz", "--namespace", "bar") assert.NilError(t, err) - assert.Assert(t, util.ContainsAll(output, "created", "svc3", "ns3", "Waiting")) + assert.Assert(t, util.ContainsAll(output, "created", "svc3", "bar", "Waiting")) output, err = executeServiceCommand(client, "list", "--all-namespaces") assert.NilError(t, err) outputLines := strings.Split(output, "\n") assert.Assert(t, util.ContainsAll(outputLines[0], "NAMESPACE", "NAME", "URL", "GENERATION", "AGE", "CONDITIONS", "READY", "REASON")) - for i, line := range outputLines[1 : len(outputLines)-1] { - ns := fmt.Sprintf("ns%d", i+1) - svc := fmt.Sprintf("svc%d", i+1) - assert.Check(t, util.ContainsAll(line, ns, svc)) - } + assert.Assert(t, util.ContainsAll(outputLines[1], "svc1", "default")) + assert.Assert(t, util.ContainsAll(outputLines[2], "svc3", "bar")) + assert.Assert(t, util.ContainsAll(outputLines[3], "svc2", "foo")) r.Validate() } diff --git a/pkg/kn/commands/service/service_list_test.go b/pkg/kn/commands/service/service_list_test.go index c7c7fba324..023b206d08 100644 --- a/pkg/kn/commands/service/service_list_test.go +++ b/pkg/kn/commands/service/service_list_test.go @@ -97,9 +97,9 @@ func TestServiceListDefaultOutput(t *testing.T) { } func TestServiceListAllNamespacesOutput(t *testing.T) { - service1 := createMockServiceWithParams("foo", "ns1", "http://foo.ns1.example.com", 1) - service2 := createMockServiceWithParams("bar", "ns2", "http://bar.ns2.example.com", 2) - service3 := createMockServiceWithParams("sss", "ns3", "http://sss.ns3.example.com", 3) + service1 := createMockServiceWithParams("foo", "default", "http://foo.default.example.com", 1) + service2 := createMockServiceWithParams("bar", "foo", "http://bar.foo.example.com", 2) + service3 := createMockServiceWithParams("sss", "bar", "http://sss.bar.example.com", 3) serviceList := &v1alpha1.ServiceList{Items: []v1alpha1.Service{*service1, *service2, *service3}} action, output, err := fakeServiceList([]string{"service", "list", "--all-namespaces"}, serviceList) if err != nil { @@ -112,9 +112,9 @@ func TestServiceListAllNamespacesOutput(t *testing.T) { } // Outputs in alphabetical order assert.Check(t, util.ContainsAll(output[0], "NAMESPACE", "NAME", "URL", "GENERATION", "AGE", "CONDITIONS", "READY", "REASON")) - assert.Check(t, util.ContainsAll(output[1], "ns2", "bar", "bar.ns2.example.com", "2")) - assert.Check(t, util.ContainsAll(output[2], "ns1", "foo", "foo.ns1.example.com", "1")) - assert.Check(t, util.ContainsAll(output[3], "ns3", "sss", "sss.ns3.example.com", "3")) + assert.Check(t, util.ContainsAll(output[1], "default", "foo", "foo.default.example.com", "1")) + assert.Check(t, util.ContainsAll(output[2], "bar", "sss", "sss.bar.example.com", "3")) + assert.Check(t, util.ContainsAll(output[3], "foo", "bar", "bar.foo.example.com", "2")) } func TestServiceGetOneOutput(t *testing.T) { From 37354ccdade0ea278569cd1992ce13e1c3e24e92 Mon Sep 17 00:00:00 2001 From: toversus Date: Sun, 18 Aug 2019 10:26:58 +0900 Subject: [PATCH 03/10] Switch output of column definitions by priority field A priority field is handled as follows: - priority 0: print namespace column - priority 1: print default columns - priority 2: print additional columns for wide option --- .../commands/revision/human_readable_flags.go | 14 ++++---- pkg/kn/commands/route/human_readable_flags.go | 10 +++--- .../commands/service/human_readable_flags.go | 32 +++++++------------ pkg/printers/tableprinter.go | 3 ++ 4 files changed, 26 insertions(+), 33 deletions(-) diff --git a/pkg/kn/commands/revision/human_readable_flags.go b/pkg/kn/commands/revision/human_readable_flags.go index af6181987f..b3bb44b930 100644 --- a/pkg/kn/commands/revision/human_readable_flags.go +++ b/pkg/kn/commands/revision/human_readable_flags.go @@ -26,13 +26,13 @@ import ( // RevisionListHandlers adds print handlers for revision list command func RevisionListHandlers(h hprinters.PrintHandler) { RevisionColumnDefinitions := []metav1beta1.TableColumnDefinition{ - {Name: "Name", Type: "string", Description: "Name of the revision."}, - {Name: "Service", Type: "string", Description: "Name of the Knative service."}, - {Name: "Generation", Type: "string", Description: "Generation of the revision"}, - {Name: "Age", Type: "string", Description: "Age of the revision."}, - {Name: "Conditions", Type: "string", Description: "Conditions describing statuses of the revision."}, - {Name: "Ready", Type: "string", Description: "Ready condition status of the revision."}, - {Name: "Reason", Type: "string", Description: "Reason for non-ready condition of the revision."}, + {Name: "Name", Type: "string", Description: "Name of the revision.", Priority: 1}, + {Name: "Service", Type: "string", Description: "Name of the Knative service.", Priority: 1}, + {Name: "Generation", Type: "string", Description: "Generation of the revision", Priority: 1}, + {Name: "Age", Type: "string", Description: "Age of the revision.", Priority: 1}, + {Name: "Conditions", Type: "string", Description: "Conditions describing statuses of the revision.", Priority: 1}, + {Name: "Ready", Type: "string", Description: "Ready condition status of the revision.", Priority: 1}, + {Name: "Reason", Type: "string", Description: "Reason for non-ready condition of the revision.", Priority: 1}, } h.TableHandler(RevisionColumnDefinitions, printRevision) h.TableHandler(RevisionColumnDefinitions, printRevisionList) diff --git a/pkg/kn/commands/route/human_readable_flags.go b/pkg/kn/commands/route/human_readable_flags.go index f9d6ddf8c3..c87cb8a15f 100644 --- a/pkg/kn/commands/route/human_readable_flags.go +++ b/pkg/kn/commands/route/human_readable_flags.go @@ -27,11 +27,11 @@ import ( // RouteListHandlers adds print handlers for route list command func RouteListHandlers(h hprinters.PrintHandler) { kRouteColumnDefinitions := []metav1beta1.TableColumnDefinition{ - {Name: "Name", Type: "string", Description: "Name of the Knative route."}, - {Name: "URL", Type: "string", Description: "URL of the Knative route."}, - {Name: "Age", Type: "string", Description: "Age of the Knative route."}, - {Name: "Conditions", Type: "string", Description: "Conditions describing statuses of route components."}, - {Name: "Traffic", Type: "integer", Description: "Traffic configured for route."}, + {Name: "Name", Type: "string", Description: "Name of the Knative route.", Priority: 1}, + {Name: "URL", Type: "string", Description: "URL of the Knative route.", Priority: 1}, + {Name: "Age", Type: "string", Description: "Age of the Knative route.", Priority: 1}, + {Name: "Conditions", Type: "string", Description: "Conditions describing statuses of route components.", Priority: 1}, + {Name: "Traffic", Type: "integer", Description: "Traffic configured for route.", Priority: 1}, } h.TableHandler(kRouteColumnDefinitions, printRoute) h.TableHandler(kRouteColumnDefinitions, printKRouteList) diff --git a/pkg/kn/commands/service/human_readable_flags.go b/pkg/kn/commands/service/human_readable_flags.go index c6f28048d7..560c877c9f 100644 --- a/pkg/kn/commands/service/human_readable_flags.go +++ b/pkg/kn/commands/service/human_readable_flags.go @@ -27,31 +27,21 @@ import ( servingv1alpha1 "knative.dev/serving/pkg/apis/serving/v1alpha1" ) -var baseKServiceColumnDefinitions = []metav1beta1.TableColumnDefinition{ - {Name: "Name", Type: "string", Description: "Name of the Knative service."}, - {Name: "Url", Type: "string", Description: "URL of the Knative service."}, - //{Name: "LastCreatedRevision", Type: "string", Description: "Name of last revision created."}, - //{Name: "LastReadyRevision", Type: "string", Description: "Name of last ready revision."}, - {Name: "Generation", Type: "integer", Description: "Sequence number of 'Generation' of the service that was last processed by the controller."}, - {Name: "Age", Type: "string", Description: "Age of the service."}, - {Name: "Conditions", Type: "string", Description: "Conditions describing statuses of service components."}, - {Name: "Ready", Type: "string", Description: "Ready condition status of the service."}, - {Name: "Reason", Type: "string", Description: "Reason for non-ready condition of the service."}, -} - // ServiceListHandlers adds print handlers for service list command func ServiceListHandlers(h hprinters.PrintHandler) { - kServiceColumnDefinitions := baseKServiceColumnDefinitions - h.TableHandler(kServiceColumnDefinitions, printKService) - h.TableHandler(kServiceColumnDefinitions, printKServiceList) -} - -// ServiceListAllNamespaceHandlers adds print handlers for service list command with all namespaces option -func ServiceListAllNamespaceHandlers(h hprinters.PrintHandler) { kServiceColumnDefinitions := []metav1beta1.TableColumnDefinition{ - {Name: "Namespace", Type: "string", Description: "Namespace of the Knative service"}, + {Name: "Namespace", Type: "string", Description: "Namespace of the Knative service", Priority: 0}, + {Name: "Name", Type: "string", Description: "Name of the Knative service.", Priority: 1}, + {Name: "Url", Type: "string", Description: "URL of the Knative service.", Priority: 1}, + //{Name: "LastCreatedRevision", Type: "string", Description: "Name of last revision created.", Priority: 1}, + //{Name: "LastReadyRevision", Type: "string", Description: "Name of last ready revision.", Priority: 1}, + {Name: "Generation", Type: "integer", Description: "Sequence number of 'Generation' of the service that was last processed by the controller.", Priority: 1}, + {Name: "Age", Type: "string", Description: "Age of the service.", Priority: 1}, + {Name: "Conditions", Type: "string", Description: "Conditions describing statuses of service components.", Priority: 1}, + {Name: "Ready", Type: "string", Description: "Ready condition status of the service.", Priority: 1}, + {Name: "Reason", Type: "string", Description: "Reason for non-ready condition of the service.", Priority: 1}, } - kServiceColumnDefinitions = append(kServiceColumnDefinitions, baseKServiceColumnDefinitions...) + h.TableHandler(kServiceColumnDefinitions, printKService) h.TableHandler(kServiceColumnDefinitions, printKServiceList) } diff --git a/pkg/printers/tableprinter.go b/pkg/printers/tableprinter.go index 0f3c18d6cc..ac4824eff0 100644 --- a/pkg/printers/tableprinter.go +++ b/pkg/printers/tableprinter.go @@ -76,6 +76,9 @@ func printRowsForHandlerEntry(output io.Writer, handler *handlerEntry, obj runti var headers []string for _, column := range handler.columnDefinitions { + if !options.AllNamespaces && column.Priority == 0 { + continue + } headers = append(headers, strings.ToUpper(column.Name)) } printHeader(headers, output) From 33158c3362431eff44f4004a76508593f750f2d4 Mon Sep 17 00:00:00 2001 From: toversus Date: Sun, 18 Aug 2019 10:36:50 +0900 Subject: [PATCH 04/10] Don't use AddFlags to bind all namespaces flag --- pkg/kn/commands/human_readable_flags.go | 17 +++++++++-------- pkg/kn/commands/service/service_list.go | 6 ++++++ pkg/kn/commands/service/service_list_flags.go | 16 ++++++++-------- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/pkg/kn/commands/human_readable_flags.go b/pkg/kn/commands/human_readable_flags.go index cb1a21be37..6965bd3bfc 100644 --- a/pkg/kn/commands/human_readable_flags.go +++ b/pkg/kn/commands/human_readable_flags.go @@ -30,7 +30,7 @@ import ( // Given the following flag values, a printer can be requested that knows // how to handle printing based on these values. type HumanPrintFlags struct { - AllNamespaces bool + WithNamespace bool } // AllowedFormats returns more customized formating options @@ -42,7 +42,7 @@ func (f *HumanPrintFlags) AllowedFormats() []string { // ToPrinter receives returns a printer capable of // handling human-readable output. func (f *HumanPrintFlags) ToPrinter(getHandlerFunc func(h hprinters.PrintHandler)) (hprinters.ResourcePrinter, error) { - p := hprinters.NewTablePrinter(hprinters.PrintOptions{f.AllNamespaces}) + p := hprinters.NewTablePrinter(hprinters.PrintOptions{f.WithNamespace}) getHandlerFunc(p) return p, nil } @@ -50,12 +50,7 @@ func (f *HumanPrintFlags) ToPrinter(getHandlerFunc func(h hprinters.PrintHandler // AddFlags receives a *cobra.Command reference and binds // flags related to human-readable printing to it func (f *HumanPrintFlags) AddFlags(c *cobra.Command) { - // Bind AllNamespaces flag - all, err := c.Flags().GetBool("all-namespaces") - if err != nil { - fmt.Fprintln(c.OutOrStdout(), err) - } - f.AllNamespaces = all + // TODO: Add more flags as required } // NewHumanPrintFlags returns flags associated with @@ -64,6 +59,12 @@ func NewHumanPrintFlags() *HumanPrintFlags { return &HumanPrintFlags{} } +// EnsureWithNamespace sets the "WithNamespace" humanreadable option to true. +func (f *HumanPrintFlags) EnsureWithNamespace() { + f.WithNamespace = true + return +} + // Private functions // conditionsValue returns the True conditions count among total conditions diff --git a/pkg/kn/commands/service/service_list.go b/pkg/kn/commands/service/service_list.go index d38dcf1068..7080bed53e 100644 --- a/pkg/kn/commands/service/service_list.go +++ b/pkg/kn/commands/service/service_list.go @@ -60,6 +60,12 @@ func NewServiceListCommand(p *commands.KnParams) *cobra.Command { fmt.Fprintf(cmd.OutOrStdout(), "No resources found.\n") return nil } + + // empty namespace indicates all-namespaces flag is specified + if namespace == "" { + serviceListFlags.EnsureWithNamespace() + } + printer, err := serviceListFlags.ToPrinter() if err != nil { return err diff --git a/pkg/kn/commands/service/service_list_flags.go b/pkg/kn/commands/service/service_list_flags.go index 4486ce5101..1cbc114e10 100644 --- a/pkg/kn/commands/service/service_list_flags.go +++ b/pkg/kn/commands/service/service_list_flags.go @@ -46,14 +46,7 @@ func (f *ServiceListFlags) ToPrinter() (hprinters.ResourcePrinter, error) { } return p, nil } - // if no flags specified, use the table printing - if f.HumanReadableFlags.AllNamespaces { - p, err := f.HumanReadableFlags.ToPrinter(ServiceListAllNamespaceHandlers) - if err != nil { - return nil, err - } - return p, nil - } + p, err := f.HumanReadableFlags.ToPrinter(ServiceListHandlers) if err != nil { return nil, err @@ -75,3 +68,10 @@ func NewServiceListFlags() *ServiceListFlags { HumanReadableFlags: commands.NewHumanPrintFlags(), } } + +// EnsureWithNamespace ensures that humanreadable flags return +// a printer capable of printing with a "namespace" column. +func (f *ServiceListFlags) EnsureWithNamespace() { + f.HumanReadableFlags.EnsureWithNamespace() + return +} From 3041b976e706ad61858dd1a4b8ddc70ed13137d4 Mon Sep 17 00:00:00 2001 From: toversus Date: Sun, 18 Aug 2019 10:37:19 +0900 Subject: [PATCH 05/10] Update CHANGELOG --- CHANGELOG.adoc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 366f7218e3..f90d6e4eca 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -47,6 +47,10 @@ | https://github.com/knative/client/pull/345[#345] +| 🎁 +| Print NAMESPACE column as the first column when --all-namespaces is specified +| https://github.com/knative/client/pull/366[#366] + |=== ## v0.2.0 (2019-07-10) From 24b010e9aef05e9dcd4856d20ce8f9ecdddc75d3 Mon Sep 17 00:00:00 2001 From: toversus Date: Tue, 20 Aug 2019 12:55:37 +0900 Subject: [PATCH 06/10] Remove commented codes --- pkg/kn/commands/service/human_readable_flags.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/kn/commands/service/human_readable_flags.go b/pkg/kn/commands/service/human_readable_flags.go index 560c877c9f..bdf651293a 100644 --- a/pkg/kn/commands/service/human_readable_flags.go +++ b/pkg/kn/commands/service/human_readable_flags.go @@ -33,8 +33,6 @@ func ServiceListHandlers(h hprinters.PrintHandler) { {Name: "Namespace", Type: "string", Description: "Namespace of the Knative service", Priority: 0}, {Name: "Name", Type: "string", Description: "Name of the Knative service.", Priority: 1}, {Name: "Url", Type: "string", Description: "URL of the Knative service.", Priority: 1}, - //{Name: "LastCreatedRevision", Type: "string", Description: "Name of last revision created.", Priority: 1}, - //{Name: "LastReadyRevision", Type: "string", Description: "Name of last ready revision.", Priority: 1}, {Name: "Generation", Type: "integer", Description: "Sequence number of 'Generation' of the service that was last processed by the controller.", Priority: 1}, {Name: "Age", Type: "string", Description: "Age of the service.", Priority: 1}, {Name: "Conditions", Type: "string", Description: "Conditions describing statuses of service components.", Priority: 1}, From badd4fdfc9d2ca05e946b79908037fd6db728bf0 Mon Sep 17 00:00:00 2001 From: toversus Date: Tue, 20 Aug 2019 13:35:50 +0900 Subject: [PATCH 07/10] Fix import path --- pkg/kn/commands/service/human_readable_flags.go | 3 --- pkg/kn/commands/service/service_list_mock_test.go | 6 +++--- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/pkg/kn/commands/service/human_readable_flags.go b/pkg/kn/commands/service/human_readable_flags.go index bdf651293a..c6b53211aa 100644 --- a/pkg/kn/commands/service/human_readable_flags.go +++ b/pkg/kn/commands/service/human_readable_flags.go @@ -17,9 +17,6 @@ package service import ( "sort" - "github.com/knative/client/pkg/kn/commands" - hprinters "github.com/knative/client/pkg/printers" - servingv1alpha1 "github.com/knative/serving/pkg/apis/serving/v1alpha1" metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1" "k8s.io/apimachinery/pkg/runtime" "knative.dev/client/pkg/kn/commands" diff --git a/pkg/kn/commands/service/service_list_mock_test.go b/pkg/kn/commands/service/service_list_mock_test.go index 71813e8cfe..3a554a4af8 100644 --- a/pkg/kn/commands/service/service_list_mock_test.go +++ b/pkg/kn/commands/service/service_list_mock_test.go @@ -18,11 +18,11 @@ import ( "strings" "testing" - knclient "github.com/knative/client/pkg/serving/v1alpha1" - "github.com/knative/client/pkg/util" - "github.com/knative/serving/pkg/apis/serving/v1alpha1" "gotest.tools/assert" "k8s.io/apimachinery/pkg/api/errors" + knclient "knative.dev/client/pkg/serving/v1alpha1" + "knative.dev/client/pkg/util" + "knative.dev/serving/pkg/apis/serving/v1alpha1" ) func TestServiceListAllNamespaceMock(t *testing.T) { From a85c3f05ce2a74bd3af63bed36f0d3fa9a956c33 Mon Sep 17 00:00:00 2001 From: toversus Date: Wed, 21 Aug 2019 00:13:17 +0900 Subject: [PATCH 08/10] Clean up --- pkg/kn/commands/human_readable_flags.go | 1 - pkg/kn/commands/service/human_readable_flags.go | 2 +- pkg/kn/commands/service/service_list.go | 3 --- 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/pkg/kn/commands/human_readable_flags.go b/pkg/kn/commands/human_readable_flags.go index 6965bd3bfc..a249466987 100644 --- a/pkg/kn/commands/human_readable_flags.go +++ b/pkg/kn/commands/human_readable_flags.go @@ -62,7 +62,6 @@ func NewHumanPrintFlags() *HumanPrintFlags { // EnsureWithNamespace sets the "WithNamespace" humanreadable option to true. func (f *HumanPrintFlags) EnsureWithNamespace() { f.WithNamespace = true - return } // Private functions diff --git a/pkg/kn/commands/service/human_readable_flags.go b/pkg/kn/commands/service/human_readable_flags.go index c6b53211aa..8932fa439d 100644 --- a/pkg/kn/commands/service/human_readable_flags.go +++ b/pkg/kn/commands/service/human_readable_flags.go @@ -86,7 +86,7 @@ func printKServiceWithNaemspace(kServiceList *servingv1alpha1.ServiceList, optio others = append(others, r...) } - // sort other services list alphabetically by namespace name + // sort other services list alphabetically by namespace sort.SliceStable(others, func(i, j int) bool { return others[i].Cells[0].(string) < others[j].Cells[0].(string) }) diff --git a/pkg/kn/commands/service/service_list.go b/pkg/kn/commands/service/service_list.go index 7080bed53e..7a867bc44a 100644 --- a/pkg/kn/commands/service/service_list.go +++ b/pkg/kn/commands/service/service_list.go @@ -40,9 +40,6 @@ func NewServiceListCommand(p *commands.KnParams) *cobra.Command { # List service 'web' kn service list web`, - PreRun: func(cmd *cobra.Command, args []string) { - serviceListFlags.HumanReadableFlags.AddFlags(cmd) - }, RunE: func(cmd *cobra.Command, args []string) error { namespace, err := p.GetNamespace(cmd) if err != nil { From 1b7576c87a89ae20d393e653c1ebc27cab85dfb4 Mon Sep 17 00:00:00 2001 From: toversus Date: Wed, 21 Aug 2019 23:41:06 +0900 Subject: [PATCH 09/10] Remove unnecessary return --- pkg/kn/commands/service/service_list_flags.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/kn/commands/service/service_list_flags.go b/pkg/kn/commands/service/service_list_flags.go index 1cbc114e10..4b31fe7ebf 100644 --- a/pkg/kn/commands/service/service_list_flags.go +++ b/pkg/kn/commands/service/service_list_flags.go @@ -73,5 +73,4 @@ func NewServiceListFlags() *ServiceListFlags { // a printer capable of printing with a "namespace" column. func (f *ServiceListFlags) EnsureWithNamespace() { f.HumanReadableFlags.EnsureWithNamespace() - return } From c131194fcf7674361a3106b0f071edb7940df8bd Mon Sep 17 00:00:00 2001 From: toversus Date: Thu, 22 Aug 2019 09:31:27 +0900 Subject: [PATCH 10/10] Swap the order of records --- pkg/kn/commands/service/service_list_mock_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/kn/commands/service/service_list_mock_test.go b/pkg/kn/commands/service/service_list_mock_test.go index 3a554a4af8..ba6149446a 100644 --- a/pkg/kn/commands/service/service_list_mock_test.go +++ b/pkg/kn/commands/service/service_list_mock_test.go @@ -70,9 +70,9 @@ func TestServiceListAllNamespaceMock(t *testing.T) { outputLines := strings.Split(output, "\n") assert.Assert(t, util.ContainsAll(outputLines[0], "NAMESPACE", "NAME", "URL", "GENERATION", "AGE", "CONDITIONS", "READY", "REASON")) - assert.Assert(t, util.ContainsAll(outputLines[1], "svc1", "default")) - assert.Assert(t, util.ContainsAll(outputLines[2], "svc3", "bar")) - assert.Assert(t, util.ContainsAll(outputLines[3], "svc2", "foo")) + assert.Assert(t, util.ContainsAll(outputLines[1], "default", "svc1")) + assert.Assert(t, util.ContainsAll(outputLines[2], "bar", "svc3")) + assert.Assert(t, util.ContainsAll(outputLines[3], "foo", "svc2")) r.Validate() }