feature(service describe): Output of service details

As discussed in knative#48 this `kn service describe` as changed in this commit
mimics `kubectl describe <sth>` as that it output detail information
in human readable output.

Summary information of a list of services or a single services as well
as exporting the service in various formats (JSON, YAML) is reserved to
`kn service list` (or `kn service get` as suggested to align with
kubectl conventions).

For this reason, the generic Printer handling from cli-runtime has been
removed as `kn service describe` only outputs human readable format
(that is btw also true for `kubectl` describe which does not allow for
`-o` kind of options).

The presented information is just a start and the full content
should be discussed.

I could imagine to add the following additional fields in the output:

- Revisions overview (like image)
- Traffic distribution
- Timeout seconds
- Conditions
- Relevant Events (if any)
- Annotations / Labels
- Reference to configuration

Surely there can be more, but I suggest to make this an iterative excercise.

pkg/kn/commands/service_describe.go

@@ -16,53 +16,120 @@ package commands
 
 import (
 	"errors"
+	"fmt"
+	"github.com/knative/serving/pkg/apis/serving/v1alpha1"
+	"io"
+	"k8s.io/apimachinery/pkg/util/duration"
+	"text/tabwriter"
+	"time"
 
 	"github.com/spf13/cobra"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/cli-runtime/pkg/genericclioptions"
 )
 
 func NewServiceDescribeCommand(p *KnParams) *cobra.Command {
-
-	serviceDescribePrintFlags := genericclioptions.NewPrintFlags("").WithDefaultOutput("yaml")
-	serviceDescribeCommand := &cobra.Command{
+	command := &cobra.Command{
 		Use:   "describe NAME",
-		Short: "Describe available services.",
+		Short: "Show details for a given service",
 		RunE: func(cmd *cobra.Command, args []string) error {
 			if len(args) < 1 {
-				return errors.New("requires the service name.")
+				return errors.New("no service name provided")
 			}
-			client, err := p.ServingFactory()
-			if err != nil {
-				return err
+			if len(args) > 1 {
+				return errors.New("more than one service name provided")
 			}
-
 			namespace, err := GetNamespace(cmd)
 			if err != nil {
 				return err
 			}
-			describeService, err := client.Services(namespace).Get(args[0], v1.GetOptions{})
-			if err != nil {
-				return err
-			}
-
-			printer, err := serviceDescribePrintFlags.ToPrinter()
-			if err != nil {
-				return err
-			}
-			describeService.GetObjectKind().SetGroupVersionKind(schema.GroupVersionKind{
-				Group:   "knative.dev",
-				Version: "v1alpha1",
-				Kind:    "Service"})
-			err = printer.PrintObj(describeService, cmd.OutOrStdout())
+			service, err := getService(namespace, args[0], p)
 			if err != nil {
 				return err
 			}
+			describeService(cmd.OutOrStdout(), service)
 			return nil
 		},
 	}
-	AddNamespaceFlags(serviceDescribeCommand.Flags(), false)
-	serviceDescribePrintFlags.AddFlags(serviceDescribeCommand)
-	return serviceDescribeCommand
+	AddNamespaceFlags(command.Flags(), false)
+	return command
+}
+
+func describeService(w io.Writer, service *v1alpha1.Service) {
+	dw := newDescribeWriter(w)
+	defer dw.flush()
+
+	dw.sCol("Name", service.Name)
+	dw.sCol("Namespace", service.Namespace)
+	dw.sCol("Address", service.Status.Address.Hostname)
+	dw.sCol("Domain", service.Status.Domain)
+	dw.sCol("Age", dw.age(service.CreationTimestamp.Time))
+	dw.newline()
+
+	/*
+	   Additional information which might make sense to add:
+	   - Revisions overview (like image)
+	   - Traffic distribution
+	   - Timeout seconds
+	   - Conditions
+	   - Relevant Events (if any)
+	   - Annotations / Labels
+	   - Reference to configuration
+	*/
+}
+
+func getService(namespace string, name string, p *KnParams) (*v1alpha1.Service, error) {
+	client, err := p.ServingFactory()
+	if err != nil {
+		return nil, err
+	}
+	service, err := client.Services(namespace).Get(name, v1.GetOptions{})
+	if err != nil {
+		return nil, err
+	}
+	service.GetObjectKind().SetGroupVersionKind(schema.GroupVersionKind{
+		Group:   "knative.dev",
+		Version: "v1alpha1",
+		Kind:    "Service"})
+	return service, nil
+}
+
+type describeWriter struct {
+	writer *tabwriter.Writer
+}
+
+// TODO: Should be moved to util class to be reused across all human readable output ?
+func newDescribeWriter(output io.Writer) *describeWriter {
+	return &describeWriter{
+		writer: tabwriter.NewWriter(output, 6, 4, 3, ' ', tabwriter.TabIndent),
+	}
+}
+
+func (tw *describeWriter) sCol(label string, val string) {
+	fmt.Fprintf(tw.writer, "%s:\t%s\n", label, val)
+}
+
+func (tw *describeWriter) flush() {
+	tw.writer.Flush()
+}
+
+func (tw *describeWriter) println(args ...interface{}) {
+	fmt.Fprintln(tw.writer, args...)
+}
+
+func (tw *describeWriter) printf(format string, args ...interface{}) {
+	fmt.Fprintf(tw.writer, format, args...)
+}
+
+func (tw *describeWriter) newline() {
+	tw.println("")
+	tw.flush()
+}
+
+// TODO: Move to an utility function
+func (tw *describeWriter) age(t time.Time) string {
+	if t.IsZero() {
+		return ""
+	}
+	return duration.ShortHumanDuration(time.Now().Sub(t))
 }

pkg/kn/commands/service_describe_test.go

@@ -16,28 +16,28 @@ package commands
 
 import (
 	"bytes"
-	"encoding/json"
+	"regexp"
+	"strings"
 	"testing"
 
+	duckv1alpha1 "github.com/knative/pkg/apis/duck/v1alpha1"
 	"github.com/knative/serving/pkg/apis/serving/v1alpha1"
 	serving "github.com/knative/serving/pkg/client/clientset/versioned/typed/serving/v1alpha1"
 	"github.com/knative/serving/pkg/client/clientset/versioned/typed/serving/v1alpha1/fake"
-	"k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	client_testing "k8s.io/client-go/testing"
-	"sigs.k8s.io/yaml"
+	clienttesting "k8s.io/client-go/testing"
 )
 
-func fakeServiceDescribe(args []string, response *v1alpha1.Service) (action client_testing.Action, output string, err error) {
+func fakeServiceDescribe(args []string, response *v1alpha1.Service) (action clienttesting.Action, output string, err error) {
 	buf := new(bytes.Buffer)
-	fakeServing := &fake.FakeServingV1alpha1{&client_testing.Fake{}}
+	fakeServing := &fake.FakeServingV1alpha1{Fake: &clienttesting.Fake{}}
 	cmd := NewKnCommand(KnParams{
 		Output:         buf,
 		ServingFactory: func() (serving.ServingV1alpha1Interface, error) { return fakeServing, nil },
 	})
 	fakeServing.AddReactor("*", "*",
-		func(a client_testing.Action) (bool, runtime.Object, error) {
+		func(a clienttesting.Action) (bool, runtime.Object, error) {
 			action = a
 			return true, response, nil
 		})
@@ -52,9 +52,11 @@ func fakeServiceDescribe(args []string, response *v1alpha1.Service) (action clie
 
 func TestEmptyServiceDescribe(t *testing.T) {
 	_, _, err := fakeServiceDescribe([]string{"service", "describe"}, &v1alpha1.Service{})
-	expectedError := "requires the service name."
-	if err == nil || err.Error() != expectedError {
-		t.Fatal("expect to fail with missing service name")
+	if err == nil ||
+		!strings.Contains(err.Error(), "no") ||
+		!strings.Contains(err.Error(), "service") ||
+		!strings.Contains(err.Error(), "provided") {
+		t.Fatalf("expect to fail with missing service name (got: %v)", err)
 	}
 }
 
@@ -68,6 +70,10 @@ func TestServiceDescribeDefaultOutput(t *testing.T) {
 			Name:      "foo",
 			Namespace: "default",
 		},
+		Status: v1alpha1.ServiceStatus{
+			Domain:  "foo.default.example.com",
+			Address: &duckv1alpha1.Addressable{Hostname: "foo.default.svc.cluster.local"},
+		},
 	}
 	action, output, err := fakeServiceDescribe([]string{"service", "describe", "test-foo"}, &expectedService)
 	if err != nil {
@@ -79,17 +85,19 @@ func TestServiceDescribeDefaultOutput(t *testing.T) {
 		t.Fatalf("Bad action %v", action)
 	}
 
-	jsonData, err := yaml.YAMLToJSON([]byte(output))
-	if err != nil {
-		t.Fatal(err)
-	}
-	var returnedService v1alpha1.Service
-	err = json.Unmarshal(jsonData, &returnedService)
+	assertMatches(t, output, "Name:\\s+foo")
+	assertMatches(t, output, "Namespace:\\s+default")
+	assertMatches(t, output, "Address:\\s+foo.default.svc.cluster.local")
+	assertMatches(t, output, "Domain:\\s+foo.default.example.com")
+	assertMatches(t, output, "Age:")
+}
+
+func assertMatches(t *testing.T, value, expr string) {
+	ok, err := regexp.MatchString(expr, value)
 	if err != nil {
-		t.Fatal(err)
+		t.Fatalf("invalid pattern %q. %v", expr, err)
 	}
-
-	if !equality.Semantic.DeepEqual(expectedService, returnedService) {
-		t.Fatal("mismatched objects")
+	if !ok {
+		t.Errorf("got %s which does not match %s\n", value, expr)
 	}
 }