From 15f9c4883a4b5c0124ca8f152aea9bd84686eef6 Mon Sep 17 00:00:00 2001 From: Daisy Guo Date: Fri, 3 Apr 2020 00:09:06 +0800 Subject: [PATCH 1/3] fix error when output is set to name --- pkg/kn/commands/service/list.go | 13 ++++-- pkg/util/unstructured.go | 58 ++++++++++++++++++++++++ pkg/util/unstructured_test.go | 79 +++++++++++++++++++++++++++++++++ 3 files changed, 146 insertions(+), 4 deletions(-) create mode 100644 pkg/util/unstructured.go create mode 100644 pkg/util/unstructured_test.go diff --git a/pkg/kn/commands/service/list.go b/pkg/kn/commands/service/list.go index 50cc13e1e1..974db35340 100644 --- a/pkg/kn/commands/service/list.go +++ b/pkg/kn/commands/service/list.go @@ -24,6 +24,7 @@ import ( "knative.dev/client/pkg/kn/commands" "knative.dev/client/pkg/kn/commands/flags" clientservingv1 "knative.dev/client/pkg/serving/v1" + "knative.dev/client/pkg/util" ) // NewServiceListCommand represents 'kn service list' command @@ -81,11 +82,15 @@ func NewServiceListCommand(p *commands.KnParams) *cobra.Command { return a.ObjectMeta.Name < b.ObjectMeta.Name }) - err = printer.PrintObj(serviceList, cmd.OutOrStdout()) - if err != nil { - return err + if serviceListFlags.GenericPrintFlags.OutputFlagSpecified() { + unstructedList, err := util.ToUnstructuredList(serviceList) + if err != nil { + return err + } + return printer.PrintObj(unstructedList, cmd.OutOrStdout()) } - return nil + + return printer.PrintObj(serviceList, cmd.OutOrStdout()) }, } commands.AddNamespaceFlags(serviceListCommand.Flags(), true) diff --git a/pkg/util/unstructured.go b/pkg/util/unstructured.go new file mode 100644 index 0000000000..d69b4c82e9 --- /dev/null +++ b/pkg/util/unstructured.go @@ -0,0 +1,58 @@ +// Copyright © 2020 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 util + +import ( + "encoding/json" + + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" +) + +func ToUnstructuredList(obj runtime.Object) (*unstructured.UnstructuredList, error) { + unstructuredList := &unstructured.UnstructuredList{} + unstructuredList.SetGroupVersionKind(obj.GetObjectKind().GroupVersionKind()) + if meta.IsListType(obj) { + items, err := meta.ExtractList(obj) + if err != nil { + return nil, err + } + for _, obj := range items { + ud, err := toUnstructured(obj) + if err != nil { + return nil, err + } + unstructuredList.Items = append(unstructuredList.Items, *ud) + } + + } else { + + } + return unstructuredList, nil + +} + +func toUnstructured(obj runtime.Object) (*unstructured.Unstructured, error) { + b, err := json.Marshal(obj) + if err != nil { + return nil, err + } + ud := &unstructured.Unstructured{} + if err := json.Unmarshal(b, ud); err != nil { + return nil, err + } + return ud, nil +} diff --git a/pkg/util/unstructured_test.go b/pkg/util/unstructured_test.go new file mode 100644 index 0000000000..b4cf4ddaad --- /dev/null +++ b/pkg/util/unstructured_test.go @@ -0,0 +1,79 @@ +// 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 util + +import ( + "testing" + + "gotest.tools/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + servingv1 "knative.dev/serving/pkg/apis/serving/v1" +) + +func TestToUnstructuredList(t *testing.T) { + serviceList := servingv1.ServiceList{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "List", + }, + Items: []servingv1.Service{createService("s1"), createService("s2")}, + } + expectedList := &unstructured.UnstructuredList{ + Object: map[string]interface{}{ + "apiVersion": string("v1"), + "kind": string("List"), + }, + } + expectedList.Items = []unstructured.Unstructured{createUnstructured("s1"), createUnstructured("s2")} + unstructedList, err := ToUnstructuredList(&serviceList) + assert.NilError(t, err) + assert.DeepEqual(t, unstructedList, expectedList) +} + +func createService(name string) servingv1.Service { + service := servingv1.Service{ + TypeMeta: metav1.TypeMeta{ + Kind: "Service", + APIVersion: "serving.knative.dev/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: "default", + }, + } + return service +} + +func createUnstructured(name string) unstructured.Unstructured { + return unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "serving.knative.dev/v1", + "kind": "Service", + "metadata": map[string]interface{}{ + "namespace": "default", + "name": name, + "creationTimestamp": nil, + }, + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "metadata": map[string]interface{}{"creationTimestamp": nil}, + "spec": map[string]interface{}{"containers": nil}, + }, + }, + "status": map[string]interface{}{}, + }, + } +} From 3f440b547aed92ef0f3e0828cdea0121f2972ec8 Mon Sep 17 00:00:00 2001 From: Daisy Guo Date: Fri, 3 Apr 2020 16:55:41 +0800 Subject: [PATCH 2/3] add e2e test --- pkg/util/unstructured.go | 14 ++++++++++---- pkg/util/unstructured_test.go | 7 +++++++ test/e2e/basic_workflow_test.go | 9 +++++++++ 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/pkg/util/unstructured.go b/pkg/util/unstructured.go index d69b4c82e9..717f347f85 100644 --- a/pkg/util/unstructured.go +++ b/pkg/util/unstructured.go @@ -22,16 +22,18 @@ import ( "k8s.io/apimachinery/pkg/runtime" ) +// ToUnstructuredList is to converts an object to unstructured.UnstructuredList. +// If the object is not a list type, it will convert to a single item UnstructuredList. func ToUnstructuredList(obj runtime.Object) (*unstructured.UnstructuredList, error) { unstructuredList := &unstructured.UnstructuredList{} - unstructuredList.SetGroupVersionKind(obj.GetObjectKind().GroupVersionKind()) if meta.IsListType(obj) { + unstructuredList.SetGroupVersionKind(obj.GetObjectKind().GroupVersionKind()) items, err := meta.ExtractList(obj) if err != nil { return nil, err } - for _, obj := range items { - ud, err := toUnstructured(obj) + for _, obji := range items { + ud, err := toUnstructured(obji) if err != nil { return nil, err } @@ -39,7 +41,11 @@ func ToUnstructuredList(obj runtime.Object) (*unstructured.UnstructuredList, err } } else { - + ud, err := toUnstructured(obj) + if err != nil { + return nil, err + } + unstructuredList.Items = append(unstructuredList.Items, *ud) } return unstructuredList, nil diff --git a/pkg/util/unstructured_test.go b/pkg/util/unstructured_test.go index b4cf4ddaad..8f40db4ea0 100644 --- a/pkg/util/unstructured_test.go +++ b/pkg/util/unstructured_test.go @@ -41,6 +41,13 @@ func TestToUnstructuredList(t *testing.T) { unstructedList, err := ToUnstructuredList(&serviceList) assert.NilError(t, err) assert.DeepEqual(t, unstructedList, expectedList) + + service1 := createService("s3") + expectedList = &unstructured.UnstructuredList{} + expectedList.Items = []unstructured.Unstructured{createUnstructured("s3")} + unstructedList, err = ToUnstructuredList(&service1) + assert.NilError(t, err) + assert.DeepEqual(t, unstructedList, expectedList) } func createService(name string) servingv1.Service { diff --git a/test/e2e/basic_workflow_test.go b/test/e2e/basic_workflow_test.go index 129b06e518..8f9cd716ec 100644 --- a/test/e2e/basic_workflow_test.go +++ b/test/e2e/basic_workflow_test.go @@ -48,6 +48,9 @@ func TestBasicWorkflow(t *testing.T) { serviceList(r, "hello") serviceDescribe(r, "hello") + t.Log("return list --output name about hello service") + serviceListOutput(t, it, r, "hello") + t.Log("update hello service's configuration and return no error") serviceUpdate(r, "hello", "--env", "TARGET=kn", "--port", "8888") @@ -113,6 +116,12 @@ func serviceDescribe(r *test.KnRunResultCollector, serviceName string) { assert.Assert(r.T(), util.ContainsAll(out.Stdout, "Name", "Namespace", "URL", "Age", "Revisions")) } +func serviceListOutput(r *test.KnRunResultCollector, r *test.KnRunResultCollector, serviceName string) { + out := r.KnTest().Kn().Run("service", "list", serviceName, "--output", "name") + r.AssertNoError(out) + assert.Check(r.T(), util.ContainsAll(out.Stdout, serviceName, "service.serving.knative.dev")) +} + func serviceUpdate(r *test.KnRunResultCollector, serviceName string, args ...string) { fullArgs := append([]string{}, "service", "update", serviceName) fullArgs = append(fullArgs, args...) From c9c7841c2474822472769c25d699377f0bb676ac Mon Sep 17 00:00:00 2001 From: Daisy Guo Date: Wed, 8 Apr 2020 00:31:08 +0800 Subject: [PATCH 3/3] change to flags/listprint.go --- pkg/kn/commands/flags/listprint.go | 22 ++++++++++++++++++++++ pkg/kn/commands/flags/listprint_test.go | 15 +++++++++++++++ pkg/kn/commands/service/list.go | 16 +--------------- pkg/printers/tableprinter.go | 4 ++++ test/e2e/basic_workflow_test.go | 4 ++-- 5 files changed, 44 insertions(+), 17 deletions(-) diff --git a/pkg/kn/commands/flags/listprint.go b/pkg/kn/commands/flags/listprint.go index a19cca498c..da73381c01 100644 --- a/pkg/kn/commands/flags/listprint.go +++ b/pkg/kn/commands/flags/listprint.go @@ -15,11 +15,15 @@ package flags import ( + "io" + "github.com/spf13/cobra" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/cli-runtime/pkg/genericclioptions" "knative.dev/client/pkg/kn/commands" hprinters "knative.dev/client/pkg/printers" + "knative.dev/client/pkg/util" ) // ListFlags composes common printer flag structs @@ -56,6 +60,24 @@ func (f *ListPrintFlags) ToPrinter() (hprinters.ResourcePrinter, error) { return p, nil } +// Print is to print an Object to a Writer +func (f *ListPrintFlags) Print(obj runtime.Object, w io.Writer) error { + printer, err := f.ToPrinter() + if err != nil { + return err + } + + if f.GenericPrintFlags.OutputFlagSpecified() { + unstructuredList, err := util.ToUnstructuredList(obj) + if err != nil { + return err + } + return printer.PrintObj(unstructuredList, w) + } + + return printer.PrintObj(obj, w) +} + // AddFlags receives a *cobra.Command reference and binds // flags related to humanreadable and template printing. func (f *ListPrintFlags) AddFlags(cmd *cobra.Command) { diff --git a/pkg/kn/commands/flags/listprint_test.go b/pkg/kn/commands/flags/listprint_test.go index 7a9afb0c2b..da17e88de8 100644 --- a/pkg/kn/commands/flags/listprint_test.go +++ b/pkg/kn/commands/flags/listprint_test.go @@ -48,3 +48,18 @@ func TestListPrintFlags(t *testing.T) { _, ok := p.(hprinters.ResourcePrinter) assert.Check(t, ok == true) } + +func TestListPrintFlagsPrint(t *testing.T) { + var cmd *cobra.Command + flags := NewListPrintFlags(func(h hprinters.PrintHandler) {}) + + cmd = &cobra.Command{} + flags.AddFlags(cmd) + + pr, err := flags.ToPrinter() + assert.NilError(t, err) + assert.Assert(t, pr != nil) + + err = flags.Print(nil, cmd.OutOrStdout()) + assert.NilError(t, err) +} diff --git a/pkg/kn/commands/service/list.go b/pkg/kn/commands/service/list.go index 974db35340..10a3308cc2 100644 --- a/pkg/kn/commands/service/list.go +++ b/pkg/kn/commands/service/list.go @@ -24,7 +24,6 @@ import ( "knative.dev/client/pkg/kn/commands" "knative.dev/client/pkg/kn/commands/flags" clientservingv1 "knative.dev/client/pkg/serving/v1" - "knative.dev/client/pkg/util" ) // NewServiceListCommand represents 'kn service list' command @@ -66,11 +65,6 @@ func NewServiceListCommand(p *commands.KnParams) *cobra.Command { serviceListFlags.EnsureWithNamespace() } - printer, err := serviceListFlags.ToPrinter() - if err != nil { - return err - } - // Sort serviceList by namespace and name (in this order) sort.SliceStable(serviceList.Items, func(i, j int) bool { a := serviceList.Items[i] @@ -82,15 +76,7 @@ func NewServiceListCommand(p *commands.KnParams) *cobra.Command { return a.ObjectMeta.Name < b.ObjectMeta.Name }) - if serviceListFlags.GenericPrintFlags.OutputFlagSpecified() { - unstructedList, err := util.ToUnstructuredList(serviceList) - if err != nil { - return err - } - return printer.PrintObj(unstructedList, cmd.OutOrStdout()) - } - - return printer.PrintObj(serviceList, cmd.OutOrStdout()) + return serviceListFlags.Print(serviceList, cmd.OutOrStdout()) }, } commands.AddNamespaceFlags(serviceListCommand.Flags(), true) diff --git a/pkg/printers/tableprinter.go b/pkg/printers/tableprinter.go index f51bc1390f..0f309b40d9 100644 --- a/pkg/printers/tableprinter.go +++ b/pkg/printers/tableprinter.go @@ -41,6 +41,10 @@ func NewTablePrinter(options PrintOptions) *HumanReadablePrinter { // PrintObj prints the obj in a human-friendly format according to the type of the obj. func (h *HumanReadablePrinter) PrintObj(obj runtime.Object, output io.Writer) error { + if obj == nil { + return nil + } + w, found := output.(*tabwriter.Writer) if !found { w = NewTabWriter(output) diff --git a/test/e2e/basic_workflow_test.go b/test/e2e/basic_workflow_test.go index 8f9cd716ec..4d4ebdd3de 100644 --- a/test/e2e/basic_workflow_test.go +++ b/test/e2e/basic_workflow_test.go @@ -49,7 +49,7 @@ func TestBasicWorkflow(t *testing.T) { serviceDescribe(r, "hello") t.Log("return list --output name about hello service") - serviceListOutput(t, it, r, "hello") + serviceListOutput(r, "hello") t.Log("update hello service's configuration and return no error") serviceUpdate(r, "hello", "--env", "TARGET=kn", "--port", "8888") @@ -116,7 +116,7 @@ func serviceDescribe(r *test.KnRunResultCollector, serviceName string) { assert.Assert(r.T(), util.ContainsAll(out.Stdout, "Name", "Namespace", "URL", "Age", "Revisions")) } -func serviceListOutput(r *test.KnRunResultCollector, r *test.KnRunResultCollector, serviceName string) { +func serviceListOutput(r *test.KnRunResultCollector, serviceName string) { out := r.KnTest().Kn().Run("service", "list", serviceName, "--output", "name") r.AssertNoError(out) assert.Check(r.T(), util.ContainsAll(out.Stdout, serviceName, "service.serving.knative.dev"))