Implement human readable output for kn route describe (#643)

* Implement human readable output for kn route describe

 - Keeps the machine readable output
 - Align the command description text, check for single argument, reported error messages and unit tests in service, revision, route, source binding describe commands

* Dont print separate section for owner references

* Print Service heading irrespective of owner.Kind == Service

* Align desc, err msg for trigger, apiserver and cronjob source

CHANGELOG.adoc

@@ -20,6 +20,10 @@
 |===
 | | Description | PR
 
+| 🎁
+| Add human readable `kn route describe`
+| https://github.com/knative/client/pull/643[#643]
+
 | 🐛
 | Show envFrom when running describe service or revision
 | https://github.com/knative/client/pull/630[#630]

docs/cmd/kn_revision.md

@@ -28,6 +28,6 @@ kn revision [flags]
 
 * [kn](kn.md)	 - Knative client
 * [kn revision delete](kn_revision_delete.md)	 - Delete a revision.
-* [kn revision describe](kn_revision_describe.md)	 - Describe revisions.
+* [kn revision describe](kn_revision_describe.md)	 - Show details of a revision
 * [kn revision list](kn_revision_list.md)	 - List available revisions.
 

docs/cmd/kn_revision_describe.md

@@ -1,10 +1,10 @@
 ## kn revision describe
 
-Describe revisions.
+Show details of a revision
 
 ### Synopsis
 
-Describe revisions.
+Show details of a revision
 
 ```
 kn revision describe NAME [flags]

docs/cmd/kn_route.md

@@ -27,6 +27,6 @@ kn route [flags]
 ### SEE ALSO
 
 * [kn](kn.md)	 - Knative client
-* [kn route describe](kn_route_describe.md)	 - Describe available route.
+* [kn route describe](kn_route_describe.md)	 - Show details of a route
 * [kn route list](kn_route_list.md)	 - List available routes.
 

docs/cmd/kn_route_describe.md

@@ -1,10 +1,10 @@
 ## kn route describe
 
-Describe available route.
+Show details of a route
 
 ### Synopsis
 
-Describe available route.
+Show details of a route
 
 ```
 kn route describe NAME [flags]
@@ -16,8 +16,9 @@ kn route describe NAME [flags]
       --allow-missing-template-keys   If true, ignore any errors in templates when a field or map key is missing in the template. Only applies to golang and jsonpath output formats. (default true)
   -h, --help                          help for describe
   -n, --namespace string              Specify the namespace to operate in.
-  -o, --output string                 Output format. One of: json|yaml|name|go-template|go-template-file|template|templatefile|jsonpath|jsonpath-file. (default "yaml")
+  -o, --output string                 Output format. One of: json|yaml|name|go-template|go-template-file|template|templatefile|jsonpath|jsonpath-file.
       --template string               Template string or path to template file to use when -o=go-template, -o=go-template-file. The template format is golang templates [http://golang.org/pkg/text/template/#pkg-overview].
+  -v, --verbose                       More output.
 ```
 
 ### Options inherited from parent commands

docs/cmd/kn_service.md

@@ -29,7 +29,7 @@ kn service [flags]
 * [kn](kn.md)	 - Knative client
 * [kn service create](kn_service_create.md)	 - Create a service.
 * [kn service delete](kn_service_delete.md)	 - Delete a service.
-* [kn service describe](kn_service_describe.md)	 - Show details for a given service
+* [kn service describe](kn_service_describe.md)	 - Show details of a service
 * [kn service list](kn_service_list.md)	 - List available services.
 * [kn service update](kn_service_update.md)	 - Update a service.
 

docs/cmd/kn_service_describe.md

@@ -1,10 +1,10 @@
 ## kn service describe
 
-Show details for a given service
+Show details of a service
 
 ### Synopsis
 
-Show details for a given service
+Show details of a service
 
 ```
 kn service describe NAME [flags]

docs/cmd/kn_source_apiserver.md

@@ -29,7 +29,7 @@ kn source apiserver [flags]
 * [kn source](kn_source.md)	 - Event source command group
 * [kn source apiserver create](kn_source_apiserver_create.md)	 - Create an ApiServer source.
 * [kn source apiserver delete](kn_source_apiserver_delete.md)	 - Delete an ApiServer source.
-* [kn source apiserver describe](kn_source_apiserver_describe.md)	 - Describe an ApiServer source.
+* [kn source apiserver describe](kn_source_apiserver_describe.md)	 - Show details of an ApiServer source
 * [kn source apiserver list](kn_source_apiserver_list.md)	 - List ApiServer sources.
 * [kn source apiserver update](kn_source_apiserver_update.md)	 - Update an ApiServer source.
 

docs/cmd/kn_source_apiserver_describe.md

@@ -1,10 +1,10 @@
 ## kn source apiserver describe
 
-Describe an ApiServer source.
+Show details of an ApiServer source
 
 ### Synopsis
 
-Describe an ApiServer source.
+Show details of an ApiServer source
 
 ```
 kn source apiserver describe NAME [flags]

docs/cmd/kn_source_binding.md

@@ -29,7 +29,7 @@ kn source binding [flags]
 * [kn source](kn_source.md)	 - Event source command group
 * [kn source binding create](kn_source_binding_create.md)	 - Create a sink binding.
 * [kn source binding delete](kn_source_binding_delete.md)	 - Delete a sink binding.
-* [kn source binding describe](kn_source_binding_describe.md)	 - Describe a sink binding.
+* [kn source binding describe](kn_source_binding_describe.md)	 - Show details of a sink binding
 * [kn source binding list](kn_source_binding_list.md)	 - List sink bindings.
 * [kn source binding update](kn_source_binding_update.md)	 - Update a sink binding.
 

docs/cmd/kn_source_binding_describe.md

@@ -1,10 +1,10 @@
 ## kn source binding describe
 
-Describe a sink binding.
+Show details of a sink binding
 
 ### Synopsis
 
-Describe a sink binding.
+Show details of a sink binding
 
 ```
 kn source binding describe NAME [flags]

docs/cmd/kn_source_cronjob.md

@@ -29,7 +29,7 @@ kn source cronjob [flags]
 * [kn source](kn_source.md)	 - Event source command group
 * [kn source cronjob create](kn_source_cronjob_create.md)	 - Create a CronJob source.
 * [kn source cronjob delete](kn_source_cronjob_delete.md)	 - Delete a CronJob source.
-* [kn source cronjob describe](kn_source_cronjob_describe.md)	 - Describe a CronJob source.
+* [kn source cronjob describe](kn_source_cronjob_describe.md)	 - Show details of a CronJob source
 * [kn source cronjob list](kn_source_cronjob_list.md)	 - List CronJob sources.
 * [kn source cronjob update](kn_source_cronjob_update.md)	 - Update a CronJob source.
 

docs/cmd/kn_source_cronjob_describe.md

@@ -1,10 +1,10 @@
 ## kn source cronjob describe
 
-Describe a CronJob source.
+Show details of a CronJob source
 
 ### Synopsis
 
-Describe a CronJob source.
+Show details of a CronJob source
 
 ```
 kn source cronjob describe NAME [flags]

docs/cmd/kn_trigger.md

@@ -29,7 +29,7 @@ kn trigger [flags]
 * [kn](kn.md)	 - Knative client
 * [kn trigger create](kn_trigger_create.md)	 - Create a trigger
 * [kn trigger delete](kn_trigger_delete.md)	 - Delete a trigger.
-* [kn trigger describe](kn_trigger_describe.md)	 - Describe a trigger.
+* [kn trigger describe](kn_trigger_describe.md)	 - Show details of a trigger
 * [kn trigger list](kn_trigger_list.md)	 - List available triggers.
 * [kn trigger update](kn_trigger_update.md)	 - Update a trigger
 

docs/cmd/kn_trigger_describe.md

@@ -1,10 +1,10 @@
 ## kn trigger describe
 
-Describe a trigger.
+Show details of a trigger
 
 ### Synopsis
 
-Describe a trigger.
+Show details of a trigger
 
 ```
 kn trigger describe NAME [flags]

pkg/kn/commands/revision/describe.go

@@ -43,12 +43,11 @@ func NewRevisionDescribeCommand(p *commands.KnParams) *cobra.Command {
 
 	command := &cobra.Command{
 		Use:   "describe NAME",
-		Short: "Describe revisions.",
+		Short: "Show details of a revision",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			if len(args) < 1 {
-				return errors.New("requires the revision name.")
+			if len(args) != 1 {
+				return errors.New("'kn revision describe' requires name of the revision as single argument")
 			}
-
 			namespace, err := p.GetNamespace(cmd)
 			if err != nil {
 				return err

pkg/kn/commands/revision/describe_test.go

@@ -59,10 +59,7 @@ func fakeRevision(args []string, response *servingv1.Revision) (action clienttes
 
 func TestDescribeRevisionWithNoName(t *testing.T) {
 	_, _, err := fakeRevision([]string{"revision", "describe"}, &servingv1.Revision{})
-	expectedError := "requires the revision name."
-	if err == nil || err.Error() != expectedError {
-		t.Fatal("expect to fail with missing revision name")
-	}
+	assert.ErrorContains(t, err, "requires", "name", "revision", "single", "argument")
 }
 
 func TestDescribeRevisionYaml(t *testing.T) {

pkg/kn/commands/route/describe.go

@@ -16,22 +16,27 @@ package route
 
 import (
 	"errors"
+	"fmt"
+	"io"
 
 	"github.com/spf13/cobra"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
+	servingv1 "knative.dev/serving/pkg/apis/serving/v1"
 
 	"knative.dev/client/pkg/kn/commands"
+	"knative.dev/client/pkg/printers"
 )
 
 // NewRouteDescribeCommand represents 'kn route describe' command
 func NewRouteDescribeCommand(p *commands.KnParams) *cobra.Command {
-	routeDescribePrintFlags := genericclioptions.NewPrintFlags("").WithDefaultOutput("yaml")
-	routeDescribeCommand := &cobra.Command{
+	// For machine readable output
+	machineReadablePrintFlags := genericclioptions.NewPrintFlags("")
+	command := &cobra.Command{
 		Use:   "describe NAME",
-		Short: "Describe available route.",
+		Short: "Show details of a route",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			if len(args) < 1 {
-				return errors.New("requires the route name.")
+			if len(args) != 1 {
+				return errors.New("'kn route describe' requires name of the route as single argument")
 			}
 			namespace, err := p.GetNamespace(cmd)
 			if err != nil {
@@ -43,23 +48,79 @@ func NewRouteDescribeCommand(p *commands.KnParams) *cobra.Command {
 				return err
 			}
 
-			describeRoute, err := client.GetRoute(args[0])
+			route, err := client.GetRoute(args[0])
 			if err != nil {
 				return err
 			}
 
-			printer, err := routeDescribePrintFlags.ToPrinter()
+			if machineReadablePrintFlags.OutputFlagSpecified() {
+				printer, err := machineReadablePrintFlags.ToPrinter()
+				if err != nil {
+					return err
+				}
+				return printer.PrintObj(route, cmd.OutOrStdout())
+			}
+			printDetails, err := cmd.Flags().GetBool("verbose")
 			if err != nil {
 				return err
 			}
-			err = printer.PrintObj(describeRoute, cmd.OutOrStdout())
-			if err != nil {
-				return err
-			}
-			return nil
+			return describe(cmd.OutOrStdout(), route, printDetails)
 		},
 	}
-	commands.AddNamespaceFlags(routeDescribeCommand.Flags(), false)
-	routeDescribePrintFlags.AddFlags(routeDescribeCommand)
-	return routeDescribeCommand
+	flags := command.Flags()
+	commands.AddNamespaceFlags(flags, false)
+	machineReadablePrintFlags.AddFlags(command)
+	flags.BoolP("verbose", "v", false, "More output.")
+	return command
+}
+
+func describe(w io.Writer, route *servingv1.Route, printDetails bool) error {
+	dw := printers.NewPrefixWriter(w)
+	commands.WriteMetadata(dw, &route.ObjectMeta, printDetails)
+	dw.WriteAttribute("URL", route.Status.URL.String())
+	writeService(dw, route, printDetails)
+	dw.WriteLine()
+	writeTraffic(dw, route)
+	dw.WriteLine()
+	commands.WriteConditions(dw, route.Status.Conditions, printDetails)
+	if err := dw.Flush(); err != nil {
+		return err
+	}
+	return nil
+}
+
+func writeService(dw printers.PrefixWriter, route *servingv1.Route, printDetails bool) {
+	svcName := ""
+	for _, owner := range route.ObjectMeta.OwnerReferences {
+		if owner.Kind == "Service" {
+			svcName = owner.Name
+			if printDetails {
+				svcName = fmt.Sprintf("%s (%s)", svcName, owner.APIVersion)
+			}
+		}
+
+		dw.WriteAttribute("Service", svcName)
+	}
+}
+
+func writeTraffic(dw printers.PrefixWriter, route *servingv1.Route) {
+	trafficSection := dw.WriteAttribute("Traffic Targets", "")
+	dw.Flush()
+	for _, target := range route.Status.Traffic {
+		section := trafficSection.WriteColsLn(fmt.Sprintf("%3d%%", *target.Percent), formatTarget(target))
+		if target.Tag != "" {
+			section.WriteAttribute("URL", target.URL.String())
+		}
+	}
+}
+
+func formatTarget(target servingv1.TrafficTarget) string {
+	targetHeader := target.RevisionName
+	if target.LatestRevision != nil && *target.LatestRevision {
+		targetHeader = fmt.Sprintf("@latest (%s)", target.RevisionName)
+	}
+	if target.Tag != "" {
+		targetHeader = fmt.Sprintf("%s #%s", targetHeader, target.Tag)
+	}
+	return targetHeader
 }

pkg/kn/commands/route/describe_test.go

@@ -28,6 +28,8 @@ import (
 	"sigs.k8s.io/yaml"
 
 	"knative.dev/client/pkg/kn/commands"
+	"knative.dev/client/pkg/util"
+	"knative.dev/pkg/ptr"
 )
 
 func fakeRouteDescribe(args []string, response *servingv1.Route) (action clienttesting.Action, output string, err error) {
@@ -59,17 +61,41 @@ func TestCompletion(t *testing.T) {
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      "foo",
 				Namespace: "default",
+				OwnerReferences: []metav1.OwnerReference{
+					{
+						Kind:       "Service",
+						Name:       "foo",
+						APIVersion: "serving.knative.dev/v1",
+					},
+				},
+			},
+			Status: servingv1.RouteStatus{
+				RouteStatusFields: servingv1.RouteStatusFields{
+					Traffic: []servingv1.TrafficTarget{
+						{
+							RevisionName: "foo-v2",
+							Tag:          "v2",
+							Percent:      ptr.Int64(90),
+						},
+						{
+							LatestRevision: ptr.Bool(true),
+							RevisionName:   "foo-v3",
+							Tag:            "latest",
+							Percent:        ptr.Int64(10),
+						},
+					},
+				},
 			},
 		}
 	}
 
-	t.Run("requires the route name", func(t *testing.T) {
+	t.Run("describe route without route name arg", func(t *testing.T) {
 		_, _, err := fakeRouteDescribe([]string{"route", "describe"}, &servingv1.Route{})
 		assert.Assert(t, err != nil)
-		assert.Assert(t, strings.Contains(err.Error(), "requires the route name."))
+		assert.ErrorContains(t, err, "requires", "name", "route", "single", "argument")
 	})
 
-	t.Run("describe a valid route with default output", func(t *testing.T) {
+	t.Run("describe a route with human readable output", func(t *testing.T) {
 		setup(t)
 
 		action, output, err := fakeRouteDescribe([]string{"route", "describe", "foo"}, &expectedRoute)
@@ -77,16 +103,19 @@ func TestCompletion(t *testing.T) {
 		assert.Assert(t, action != nil)
 		assert.Assert(t, action.Matches("get", "routes"))
 
-		jsonData, err := yaml.YAMLToJSON([]byte(output))
-		assert.Assert(t, err == nil)
-
-		var returnedRoute servingv1.Route
-		err = json.Unmarshal(jsonData, &returnedRoute)
-		assert.Assert(t, err == nil)
-		assert.Assert(t, equality.Semantic.DeepEqual(expectedRoute, returnedRoute))
+		assert.Check(t, util.ContainsAll(output,
+			"Name", "URL", "Service", "Traffic Targets", "Conditions",
+			"foo", "default", "90%", "foo-v2", "#v2", "10%", "@latest", "foo-v3"))
 	})
 
-	t.Run("describe a valid route with special output", func(t *testing.T) {
+	t.Run("describe a route with verbose output", func(t *testing.T) {
+		_, output, err := fakeRouteDescribe([]string{"route", "describe", "foo", "-v"}, &expectedRoute)
+		assert.Assert(t, err == nil)
+
+		assert.Check(t, util.ContainsAll(output, "foo", "default", "serving.knative.dev/v1"))
+	})
+
+	t.Run("describe a valid route with machine readable output", func(t *testing.T) {
 		t.Run("yaml", func(t *testing.T) {
 			setup(t)
 

pkg/kn/commands/service/describe.go

@@ -78,13 +78,10 @@ func NewServiceDescribeCommand(p *commands.KnParams) *cobra.Command {
 
 	command := &cobra.Command{
 		Use:   "describe NAME",
-		Short: "Show details for a given service",
+		Short: "Show details of a service",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			if len(args) < 1 {
-				return errors.New("no service name provided")
-			}
-			if len(args) > 1 {
-				return errors.New("more than one service name provided")
+			if len(args) != 1 {
+				return errors.New("'kn service describe' requires name of the service as single argument")
 			}
 			serviceName := args[0]
 
@@ -178,7 +175,7 @@ func writeService(dw printers.PrefixWriter, service *servingv1.Service) {
 }
 
 // Write out revisions associated with this service. By default only active
-// target revisions are printed, but with --all also inactive revisions
+// target revisions are printed, but with --verbose also inactive revisions
 // created by this services are shown
 func writeRevisions(dw printers.PrefixWriter, revisions []*revisionDesc, printDetails bool) {
 	revSection := dw.WriteAttribute("Revisions", "")

pkg/kn/commands/service/describe_test.go

@@ -466,10 +466,10 @@ func TestServiceDescribeVerbose(t *testing.T) {
 func TestServiceDescribeWithWrongArguments(t *testing.T) {
 	client := knclient.NewMockKnServiceClient(t)
 	_, err := executeServiceCommand(client, "describe")
-	assert.ErrorContains(t, err, "no", "service", "provided")
+	assert.ErrorContains(t, err, "requires", "name", "service", "single", "argument")
 
 	_, err = executeServiceCommand(client, "describe", "foo", "bar")
-	assert.ErrorContains(t, err, "more than one", "service", "provided")
+	assert.ErrorContains(t, err, "requires", "name", "service", "single", "argument")
 }
 
 func TestServiceDescribeMachineReadable(t *testing.T) {

pkg/kn/commands/source/apiserver/describe.go

@@ -32,13 +32,13 @@ func NewAPIServerDescribeCommand(p *commands.KnParams) *cobra.Command {
 
 	apiServerDescribe := &cobra.Command{
 		Use:   "describe NAME",
-		Short: "Describe an ApiServer source.",
+		Short: "Show details of an ApiServer source",
 		Example: `
   # Describe an ApiServer source with name 'k8sevents'
   kn source apiserver describe k8sevents`,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			if len(args) != 1 {
-				return errors.New("requires the name of the source as single argument")
+				return errors.New("'kn source apiserver describe' requires name of the source as single argument")
 			}
 			name := args[0]
 

pkg/kn/commands/source/binding/describe.go

@@ -32,13 +32,13 @@ import (
 func NewBindingDescribeCommand(p *commands.KnParams) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "describe NAME",
-		Short: "Describe a sink binding.",
+		Short: "Show details of a sink binding",
 		Example: `
   # Describe a sink binding with name 'mysinkbinding'
   kn source binding describe mysinkbinding`,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			if len(args) != 1 {
-				return errors.New("'kn source binding describe' requires the name of the source as single argument")
+				return errors.New("'kn source binding describe' requires name of the sink binding as single argument")
 			}
 			name := args[0]
 

pkg/kn/commands/source/cronjob/describe.go

@@ -31,13 +31,13 @@ func NewCronJobDescribeCommand(p *commands.KnParams) *cobra.Command {
 
 	cronJobDescribe := &cobra.Command{
 		Use:   "describe NAME",
-		Short: "Describe a CronJob source.",
+		Short: "Show details of a CronJob source",
 		Example: `
   # Describe a cronjob source with name 'mycronjob'
   kn source cronjob describe mycronjob`,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			if len(args) != 1 {
-				return errors.New("'source cronjob describe' requires the name of the source as single argument")
+				return errors.New("'kn source cronjob describe' requires name of the source as single argument")
 			}
 			name := args[0]
 

pkg/kn/commands/trigger/describe.go

@@ -31,13 +31,13 @@ func NewTriggerDescribeCommand(p *commands.KnParams) *cobra.Command {
 
 	triggerDescribe := &cobra.Command{
 		Use:   "describe NAME",
-		Short: "Describe a trigger.",
+		Short: "Show details of a trigger",
 		Example: `
   # Describe a trigger with name 'my-trigger'
   kn trigger describe my-trigger`,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			if len(args) != 1 {
-				return errors.New("'trigger describe' requires the name of the trigger as single argument")
+				return errors.New("'kn trigger describe' requires name of the trigger as single argument")
 			}
 			name := args[0]
 

pkg/printers/prefixwriter.go

@@ -47,7 +47,8 @@ type PrefixWriter interface {
 	WriteColsLn(cols ...string) PrefixWriter
 	// Flush forces indentation to be reset.
 	Flush() error
-
+	// WriteAttribute writes the attr (as a label) with the given value and returns
+	// a PrefixWriter for writing any subattributes.
 	WriteAttribute(attr, value string) PrefixWriter
 }
 

test/e2e/route_test.go

@@ -80,11 +80,8 @@ func (test *e2eTest) routeDescribe(t *testing.T, routeName string) {
 	out, err := test.kn.RunWithOpts([]string{"route", "describe", routeName}, runOpts{})
 	assert.NilError(t, err)
 
-	expectedGVK := `apiVersion: serving.knative.dev/v1
-kind: Route`
-	expectedNamespace := fmt.Sprintf("namespace: %s", test.kn.namespace)
-	expectedServiceLabel := fmt.Sprintf("serving.knative.dev/service: %s", routeName)
-	assert.Check(t, util.ContainsAll(out, expectedGVK, expectedNamespace, expectedServiceLabel))
+	assert.Check(t, util.ContainsAll(out,
+		routeName, test.kn.namespace, "URL", "Service", "Traffic", "Targets", "Conditions"))
 }
 
 func (test *e2eTest) routeDescribeWithPrintFlags(t *testing.T, routeName string) {