Repository: knative/client (go)

Issue:
Harmonize list headers for "trigger list", "cronjob list" and "apiserver list"
Actually the columns wrt to Sink / Subscription are different:

* CronJon and Apiserver source display the sink shortcut ('svc:myservice')
* Trigger shows the resolved URL ("SUBSCRIBER_URL")

Also, the condition and reason columns should be the same as for services. 
Should we add AGE, too ?

Pull request:
Harmonize list headers for trigger and sources
Fixes #567

## Proposed Changes

* Change `SUBSCRIBER_URI` to `SINK` in trigger list
* Add `CONDITIONS` to trigger list
* Add `AGE` to trigger and source ( cronjob, apiserver, binding ) list
* Move `SinkToString` to package `flags` and reuse in trigger list

Patch:
diff --git a/pkg/kn/commands/flags/sink.go b/pkg/kn/commands/flags/sink.go
index ba442ae3..2a6ce98f 100644
--- a/pkg/kn/commands/flags/sink.go
+++ b/pkg/kn/commands/flags/sink.go
@@ -105,3 +105,18 @@ func parseSink(sink string) (string, string) {
 		return parts[0], parts[1]
 	}
 }
+
+// SinkToString prepares a sink for list output
+func SinkToString(sink duckv1.Destination) string {
+	if sink.Ref != nil {
+		if sink.Ref.Kind == "Service" {
+			return fmt.Sprintf("svc:%s", sink.Ref.Name)
+		} else {
+			return fmt.Sprintf("%s:%s", strings.ToLower(sink.Ref.Kind), sink.Ref.Name)
+		}
+	}
+	if sink.URI != nil {
+		return sink.URI.String()
+	}
+	return ""
+}
diff --git a/pkg/kn/commands/source/apiserver/flags.go b/pkg/kn/commands/source/apiserver/flags.go
index 98f1cd33..0815214c 100644
--- a/pkg/kn/commands/source/apiserver/flags.go
+++ b/pkg/kn/commands/source/apiserver/flags.go
@@ -165,6 +165,7 @@ func APIServerSourceListHandlers(h hprinters.PrintHandler) {
 		{Name: "Name", Type: "string", Description: "Name of the ApiServer source", Priority: 1},
 		{Name: "Resources", Type: "string", Description: "Event resources configured for the ApiServer source", Priority: 1},
 		{Name: "Sink", Type: "string", Description: "Sink of the ApiServer source", Priority: 1},
+		{Name: "Age", Type: "string", Description: "Age of the ApiServer source", Priority: 1},
 		{Name: "Conditions", Type: "string", Description: "Ready state conditions", Priority: 1},
 		{Name: "Ready", Type: "string", Description: "Ready state of the ApiServer source", Priority: 1},
 		{Name: "Reason", Type: "string", Description: "Reason if state is not Ready", Priority: 1},
@@ -180,6 +181,7 @@ func printSource(source *v1alpha1.ApiServerSource, options hprinters.PrintOption
 	}
 
 	name := source.Name
+	age := commands.TranslateTimestampSince(source.CreationTimestamp)
 	conditions := commands.ConditionsValue(source.Status.Conditions)
 	ready := commands.ReadyCondition(source.Status.Conditions)
 	reason := strings.TrimSpace(commands.NonReadyConditionReason(source.Status.Conditions))
@@ -206,7 +208,7 @@ func printSource(source *v1alpha1.ApiServerSource, options hprinters.PrintOption
 		row.Cells = append(row.Cells, source.Namespace)
 	}
 
-	row.Cells = append(row.Cells, name, strings.Join(resources[:], ","), sink, conditions, ready, reason)
+	row.Cells = append(row.Cells, name, strings.Join(resources[:], ","), sink, age, conditions, ready, reason)
 	return []metav1beta1.TableRow{row}, nil
 }
 
diff --git a/pkg/kn/commands/source/binding/binding.go b/pkg/kn/commands/source/binding/binding.go
index 34c0ee00..b20e0c0d 100644
--- a/pkg/kn/commands/source/binding/binding.go
+++ b/pkg/kn/commands/source/binding/binding.go
@@ -23,7 +23,6 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/tools/clientcmd"
 	"knative.dev/eventing/pkg/client/clientset/versioned/typed/sources/v1alpha1"
-	duckv1 "knative.dev/pkg/apis/duck/v1"
 	"knative.dev/pkg/tracker"
 
 	"knative.dev/client/pkg/kn/commands"
@@ -133,23 +132,6 @@ func subjectToString(ref tracker.Reference) string {
 	return ret
 }
 
-// sinkToString prepares a sink for list output
-// This is kept here until we have moved everything to duckv1 (currently the other sources
-// are still on duckv1beta1)
-func sinkToString(sink duckv1.Destination) string {
-	if sink.Ref != nil {
-		if sink.Ref.Kind == "Service" {
-			return fmt.Sprintf("svc:%s", sink.Ref.Name)
-		} else {
-			return fmt.Sprintf("%s:%s", sink.Ref.Kind, sink.Ref.Name)
-		}
-	}
-	if sink.URI != nil {
-		return sink.URI.String()
-	}
-	return ""
-}
-
 // updateCeOverrides updates the values of the --ce-override flags if given
 func updateCeOverrides(bindingFlags bindingUpdateFlags, bindingBuilder *clientsourcesv1alpha1.SinkBindingBuilder) error {
 	if bindingFlags.ceOverrides != nil {
diff --git a/pkg/kn/commands/source/binding/flags.go b/pkg/kn/commands/source/binding/flags.go
index 12ef776e..e8b38884 100644
--- a/pkg/kn/commands/source/binding/flags.go
+++ b/pkg/kn/commands/source/binding/flags.go
@@ -20,6 +20,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 
 	"knative.dev/client/pkg/kn/commands"
+	"knative.dev/client/pkg/kn/commands/flags"
 	hprinters "knative.dev/client/pkg/printers"
 
 	"knative.dev/eventing/pkg/apis/sources/v1alpha1"
@@ -41,6 +42,7 @@ func BindingListHandlers(h hprinters.PrintHandler) {
 		{Name: "Name", Type: "string", Description: "Name of sink binding", Priority: 1},
 		{Name: "Subject", Type: "string", Description: "Subject part of binding", Priority: 1},
 		{Name: "Sink", Type: "string", Description: "Sink part of binding", Priority: 1},
+		{Name: "Age", Type: "string", Description: "Age of binding", Priority: 1},
 		{Name: "Conditions", Type: "string", Description: "Ready state conditions", Priority: 1},
 		{Name: "Ready", Type: "string", Description: "Ready state of the sink binding", Priority: 1},
 		{Name: "Reason", Type: "string", Description: "Reason if state is not Ready", Priority: 1},
@@ -57,7 +59,8 @@ func printSinkBinding(binding *v1alpha1.SinkBinding, options hprinters.PrintOpti
 
 	name := binding.Name
 	subject := subjectToString(binding.Spec.Subject)
-	sink := sinkToString(binding.Spec.Sink)
+	sink := flags.SinkToString(binding.Spec.Sink)
+	age := commands.TranslateTimestampSince(binding.CreationTimestamp)
 	conditions := commands.ConditionsValue(binding.Status.Conditions)
 	ready := commands.ReadyCondition(binding.Status.Conditions)
 	reason := commands.NonReadyConditionReason(binding.Status.Conditions)
@@ -66,7 +69,7 @@ func printSinkBinding(binding *v1alpha1.SinkBinding, options hprinters.PrintOpti
 		row.Cells = append(row.Cells, binding.Namespace)
 	}
 
-	row.Cells = append(row.Cells, name, subject, sink, conditions, ready, reason)
+	row.Cells = append(row.Cells, name, subject, sink, age, conditions, ready, reason)
 	return []metav1beta1.TableRow{row}, nil
 }
 
diff --git a/pkg/kn/commands/source/cronjob/flags.go b/pkg/kn/commands/source/cronjob/flags.go
index 564a7c39..cb100fb0 100644
--- a/pkg/kn/commands/source/cronjob/flags.go
+++ b/pkg/kn/commands/source/cronjob/flags.go
@@ -45,6 +45,7 @@ func CronJobSourceListHandlers(h hprinters.PrintHandler) {
 		{Name: "Name", Type: "string", Description: "Name of the CronJob source", Priority: 1},
 		{Name: "Schedule", Type: "string", Description: "Schedule of the CronJob source", Priority: 1},
 		{Name: "Sink", Type: "string", Description: "Sink of the CronJob source", Priority: 1},
+		{Name: "Age", Type: "string", Description: "Age of the CronJob source", Priority: 1},
 		{Name: "Conditions", Type: "string", Description: "Ready state conditions", Priority: 1},
 		{Name: "Ready", Type: "string", Description: "Ready state of the CronJob source", Priority: 1},
 		{Name: "Reason", Type: "string", Description: "Reason if state is not Ready", Priority: 1},
@@ -61,6 +62,7 @@ func printSource(source *v1alpha1.CronJobSource, options hprinters.PrintOptions)
 
 	name := source.Name
 	schedule := source.Spec.Schedule
+	age := commands.TranslateTimestampSince(source.CreationTimestamp)
 	conditions := commands.ConditionsValue(source.Status.Conditions)
 	ready := commands.ReadyCondition(source.Status.Conditions)
 	reason := commands.NonReadyConditionReason(source.Status.Conditions)
@@ -83,7 +85,7 @@ func printSource(source *v1alpha1.CronJobSource, options hprinters.PrintOptions)
 		row.Cells = append(row.Cells, source.Namespace)
 	}
 
-	row.Cells = append(row.Cells, name, schedule, sink, conditions, ready, reason)
+	row.Cells = append(row.Cells, name, schedule, sink, age, conditions, ready, reason)
 	return []metav1beta1.TableRow{row}, nil
 }
 
diff --git a/pkg/kn/commands/trigger/list_flags.go b/pkg/kn/commands/trigger/list_flags.go
index 3c65041d..741ea056 100644
--- a/pkg/kn/commands/trigger/list_flags.go
+++ b/pkg/kn/commands/trigger/list_flags.go
@@ -20,6 +20,7 @@ import (
 	metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"knative.dev/client/pkg/kn/commands"
+	"knative.dev/client/pkg/kn/commands/flags"
 	hprinters "knative.dev/client/pkg/printers"
 	eventing "knative.dev/eventing/pkg/apis/eventing/v1alpha1"
 )
@@ -27,12 +28,14 @@ import (
 // TriggerListHandlers handles printing human readable table for `kn source list-types` command's output
 func TriggerListHandlers(h hprinters.PrintHandler) {
 	sourceTypesColumnDefinitions := []metav1beta1.TableColumnDefinition{
-		{Name: "Namespace", Type: "string", Description: "Namespace of the trigger", Priority: 0},
-		{Name: "Name", Type: "string", Description: "Name of the trigger", Priority: 1},
-		{Name: "Broker", Type: "string", Description: "Name of the broker", Priority: 1},
-		{Name: "Subscriber_URI", Type: "string", Description: "URI of the subscriber", Priority: 1},
-		{Name: "Ready", Type: "string", Description: "Ready condition status of the trigger", Priority: 1},
-		{Name: "Reason", Type: "string", Description: "Reason for non-ready condition of the trigger", Priority: 1},
+		{Name: "Namespace", Type: "string", Description: "Namespace of the trigger.", Priority: 0},
+		{Name: "Name", Type: "string", Description: "Name of the trigger.", Priority: 1},
+		{Name: "Broker", Type: "string", Description: "Name of the broker.", Priority: 1},
+		{Name: "Sink", Type: "string", Description: "Sink for events, i.e. the subscriber.", Priority: 1},
+		{Name: "Age", Type: "string", Description: "Age of the trigger.", Priority: 1},
+		{Name: "Conditions", Type: "string", Description: "Ready state conditions.", Priority: 1},
+		{Name: "Ready", Type: "string", Description: "Ready condition status of the trigger.", Priority: 1},
+		{Name: "Reason", Type: "string", Description: "Reason for non-ready condition of the trigger.", Priority: 1},
 	}
 	h.TableHandler(sourceTypesColumnDefinitions, printTrigger)
 	h.TableHandler(sourceTypesColumnDefinitions, printTriggerList)
@@ -42,7 +45,9 @@ func TriggerListHandlers(h hprinters.PrintHandler) {
 func printTrigger(trigger *eventing.Trigger, options hprinters.PrintOptions) ([]metav1beta1.TableRow, error) {
 	name := trigger.Name
 	broker := trigger.Spec.Broker
-	uri := trigger.Status.SubscriberURI
+	sink := flags.SinkToString(trigger.Spec.Subscriber)
+	age := commands.TranslateTimestampSince(trigger.CreationTimestamp)
+	conditions := commands.ConditionsValue(trigger.Status.Conditions)
 	ready := commands.ReadyCondition(trigger.Status.Conditions)
 	reason := commands.NonReadyConditionReason(trigger.Status.Conditions)
 
@@ -57,7 +62,9 @@ func printTrigger(trigger *eventing.Trigger, options hprinters.PrintOptions) ([]
 	row.Cells = append(row.Cells,
 		name,
 		broker,
-		uri,
+		sink,
+		age,
+		conditions,
 		ready,
 		reason)
 	return []metav1beta1.TableRow{row}, nil


Test patch:
diff --git a/pkg/kn/commands/source/apiserver/list_test.go b/pkg/kn/commands/source/apiserver/list_test.go
index ea3ec354..652563f6 100644
--- a/pkg/kn/commands/source/apiserver/list_test.go
+++ b/pkg/kn/commands/source/apiserver/list_test.go
@@ -36,7 +36,7 @@ func TestListAPIServerSource(t *testing.T) {
 
 	out, err := executeAPIServerSourceCommand(apiServerClient, nil, "list")
 	assert.NilError(t, err, "sources should be listed")
-	util.ContainsAll(out, "NAME", "RESOURCES", "SINK", "CONDITIONS", "READY", "REASON")
+	util.ContainsAll(out, "NAME", "RESOURCES", "SINK", "AGE", "CONDITIONS", "READY", "REASON")
 	util.ContainsAll(out, "testsource", "Eventing:v1:false", "mysvc")
 
 	apiServerRecorder.Validate()
@@ -52,7 +52,7 @@ func TestListAPIServerSourceEmpty(t *testing.T) {
 
 	out, err := executeAPIServerSourceCommand(apiServerClient, nil, "list")
 	assert.NilError(t, err, "Sources should be listed")
-	util.ContainsNone(out, "NAME", "RESOURCES", "SINK", "CONDITIONS", "READY", "REASON")
+	util.ContainsNone(out, "NAME", "RESOURCES", "SINK", "AGE", "CONDITIONS", "READY", "REASON")
 	util.ContainsAll(out, "No", "ApiServer", "source", "found")
 
 	apiServerRecorder.Validate()
diff --git a/pkg/kn/commands/source/binding/list_test.go b/pkg/kn/commands/source/binding/list_test.go
index a8415eac..7367bea6 100644
--- a/pkg/kn/commands/source/binding/list_test.go
+++ b/pkg/kn/commands/source/binding/list_test.go
@@ -38,7 +38,7 @@ func TestListSimple(t *testing.T) {
 
 	out, err := executeSinkBindingCommand(bindingClient, nil, "list")
 	assert.NilError(t, err, "Sources should be listed")
-	util.ContainsAll(out, "NAME", "SUBJECT", "SINK", "CONDITIONS", "READY", "REASON")
+	util.ContainsAll(out, "NAME", "SUBJECT", "SINK", "AGE", "CONDITIONS", "READY", "REASON")
 	util.ContainsAll(out, "testbinding", "deployment:apps/v1:mydeploy", "mysvc")
 
 	bindingRecorder.Validate()
@@ -53,7 +53,7 @@ func TestListEmpty(t *testing.T) {
 
 	out, err := executeSinkBindingCommand(bindingClient, nil, "list")
 	assert.NilError(t, err, "Sources should be listed")
-	util.ContainsNone(out, "NAME", "SUBJECT", "SINK", "CONDITIONS", "READY", "REASON")
+	util.ContainsNone(out, "NAME", "SUBJECT", "SINK", "AGE", "CONDITIONS", "READY", "REASON")
 	util.ContainsAll(out, "No", "sink binding", "found")
 
 	bindingRecorder.Validate()
diff --git a/pkg/kn/commands/source/cronjob/list_test.go b/pkg/kn/commands/source/cronjob/list_test.go
index 9015ba13..a99a3ec7 100644
--- a/pkg/kn/commands/source/cronjob/list_test.go
+++ b/pkg/kn/commands/source/cronjob/list_test.go
@@ -36,7 +36,7 @@ func TestListCronJobSource(t *testing.T) {
 
 	out, err := executeCronJobSourceCommand(cronjobClient, nil, "list")
 	assert.NilError(t, err, "Sources should be listed")
-	util.ContainsAll(out, "NAME", "SCHEDULE", "SINK", "CONDITIONS", "READY", "REASON")
+	util.ContainsAll(out, "NAME", "SCHEDULE", "SINK", "AGE", "CONDITIONS", "READY", "REASON")
 	util.ContainsAll(out, "testsource", "* * * * */2", "mysvc")
 
 	cronJobRecorder.Validate()
@@ -52,7 +52,7 @@ func TestListCronJobSourceEmpty(t *testing.T) {
 
 	out, err := executeCronJobSourceCommand(cronjobClient, nil, "list")
 	assert.NilError(t, err, "Sources should be listed")
-	util.ContainsNone(out, "NAME", "SCHEDULE", "SINK", "CONDITIONS", "READY", "REASON")
+	util.ContainsNone(out, "NAME", "SCHEDULE", "SINK", "AGE", "CONDITIONS", "READY", "REASON")
 	util.ContainsAll(out, "No", "CronJob", "source", "found")
 
 	cronJobRecorder.Validate()
diff --git a/pkg/kn/commands/trigger/list_test.go b/pkg/kn/commands/trigger/list_test.go
index 5c290162..b4e735c6 100644
--- a/pkg/kn/commands/trigger/list_test.go
+++ b/pkg/kn/commands/trigger/list_test.go
@@ -49,7 +49,7 @@ func TestTriggerList(t *testing.T) {
 	assert.NilError(t, err)
 
 	outputLines := strings.Split(output, "\n")
-	assert.Check(t, util.ContainsAll(outputLines[0], "NAME", "BROKER", "SUBSCRIBER_URI", "READY", "REASON"))
+	assert.Check(t, util.ContainsAll(outputLines[0], "NAME", "BROKER", "SINK", "AGE", "CONDITIONS", "READY", "REASON"))
 	assert.Check(t, util.ContainsAll(outputLines[1], "trigger1", "mybroker1", "mysink"))
 	assert.Check(t, util.ContainsAll(outputLines[2], "trigger2", "mybroker2", "mysink"))
 	assert.Check(t, util.ContainsAll(outputLines[3], "trigger3", "mybroker3", "mysink"))
@@ -90,7 +90,7 @@ func TestTriggerListAllNamespace(t *testing.T) {
 	assert.NilError(t, err)
 
 	outputLines := strings.Split(output, "\n")
-	assert.Check(t, util.ContainsAll(outputLines[0], "NAMESPACE", "NAME", "BROKER", "SUBSCRIBER_URI", "READY", "REASON"))
+	assert.Check(t, util.ContainsAll(outputLines[0], "NAMESPACE", "NAME", "BROKER", "SINK", "AGE", "CONDITIONS", "READY", "REASON"))
 	assert.Check(t, util.ContainsAll(outputLines[1], "default1", "trigger1", "mybroker1", "mysink"))
 	assert.Check(t, util.ContainsAll(outputLines[2], "default2", "trigger2", "mybroker2", "mysink"))
 	assert.Check(t, util.ContainsAll(outputLines[3], "default3", "trigger3", "mybroker3", "mysink"))