Implement human readable output for kn route describe

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

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 given 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 given Revision
 
 ### Synopsis
 
-Describe revisions.
+Show details of a given 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 given 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 given Route
 
 ### Synopsis
 
-Describe available route.
+Show details of a given 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

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 given 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 given 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,82 @@ 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 err != nil {
-				return err
+			if machineReadablePrintFlags.OutputFlagSpecified() {
+				printer, err := machineReadablePrintFlags.ToPrinter()
+				if err != nil {
+					return err
+				}
+				return printer.PrintObj(route, cmd.OutOrStdout())
 			}
-			err = printer.PrintObj(describeRoute, cmd.OutOrStdout())
+			printDetails, err := cmd.Flags().GetBool("verbose")
 			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())
+	dw.WriteLine()
+	writeOwnerReferences(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 writeOwnerReferences(dw printers.PrefixWriter, route *servingv1.Route, printDetails bool) {
+	ownerSection := dw.WriteAttribute("Owner References", "")
+	dw.Flush()
+	for _, owner := range route.ObjectMeta.OwnerReferences {
+		if printDetails {
+			ownerSection.WriteAttribute(owner.Kind, fmt.Sprintf("%s (%s)", owner.Name, owner.APIVersion))
+		} else {
+			ownerSection.WriteAttribute(owner.Kind, owner.Name)
+		}
+	}
+
+}
+
+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,34 +61,61 @@ 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)
 		assert.Assert(t, err == nil)
 		assert.Assert(t, action != nil)
 		assert.Assert(t, action.Matches("get", "routes"))
 
-		jsonData, err := yaml.YAMLToJSON([]byte(output))
-		assert.Assert(t, err == nil)
+		assert.Check(t, util.ContainsAll(output,
+			"Name", "URL", "Owner References", "Traffic Targets", "Conditions",
+			"foo", "default", "90%", "foo-v2", "#v2", "10%", "@latest", "foo-v3"))
+	})
 
-		var returnedRoute servingv1.Route
-		err = json.Unmarshal(jsonData, &returnedRoute)
+	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.Assert(t, equality.Semantic.DeepEqual(expectedRoute, returnedRoute))
+
+		assert.Check(t, util.ContainsAll(output, "foo", "default", "serving.knative.dev/v1"))
 	})
 
-	t.Run("describe a valid route with special output", func(t *testing.T) {
+	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

@@ -80,11 +80,8 @@ func NewServiceDescribeCommand(p *commands.KnParams) *cobra.Command {
 		Use:   "describe NAME",
 		Short: "Show details for a given 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/binding/describe.go

@@ -38,7 +38,7 @@ func NewBindingDescribeCommand(p *commands.KnParams) *cobra.Command {
   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 source binding 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/basic_workflow_test.go

@@ -61,6 +61,10 @@ func TestBasicWorkflow(t *testing.T) {
 		test.revisionDescribe(t, "hello")
 	})
 
+	t.Run("describe route of hello service", func(t *testing.T) {
+		test.routeDescribe(t, "hello")
+	})
+
 	t.Run("delete hello and svc2 services and return no error", func(t *testing.T) {
 		test.serviceDelete(t, "hello")
 		test.serviceDelete(t, "svc2")
@@ -141,6 +145,12 @@ func (test *e2eTest) revisionDescribe(t *testing.T, serviceName string) {
 	assert.Check(t, util.ContainsAll(out, revName, test.kn.namespace, serviceName, "++ Ready", "TARGET=kn"))
 }
 
+func (test *e2eTest) routeDescribe(t *testing.T, routeName string) {
+	out, err := test.kn.RunWithOpts([]string{"route", "describe", routeName}, runOpts{})
+	assert.NilError(t, err)
+	assert.Check(t, util.ContainsAll(out, routeName, test.kn.namespace))
+}
+
 func (test *e2eTest) wrongSubCommand(t *testing.T) {
 
 	_, err := test.kn.RunWithOpts([]string{"source", "apiserver", "noverb", "--tag=0.13"}, runOpts{AllowError: true})