fix(service): Show URL (external) instead of Address (internal) when listing service (#247)

* fix(service): Pick up new field Address instead of Domain when listing service

Starting with 0.7.0 old fields are not populated anymore. Let's switch to the new fields then.

This fix should work with 0.6.0, too as the new fields already have been populated back then.

So we can unconditionally pick status.address.url which is accordance with
our policy to support the latest release and the release before.

Fixes #246.

* fix(service list): Use status.URL instead of status.Address in column

* chore: Rebase and update changelog

* chore: Rebase and test fix

CHANGELOG.adoc

@@ -11,12 +11,16 @@
 |===
 ////
 
-## v0.2.0 (2019-07-09)
+## v0.2.0 (2019-07-10)
 
 [cols="1,10,3", options="header", width="100%"]
 |===
 | | Description | PR
 
+| 🐛
+| Show URL instead of address when listing services
+| https://github.com/knative/client/pull/247[#247]
+
 | 🎁
 | Add `kn service list <svc-name>` and `kn revision list <rev-name>`
 | https://github.com/knative/client/pull/150[#150]

pkg/kn/commands/service/human_readable_flags.go

@@ -25,8 +25,8 @@ import (
 // ServiceListHandlers adds print handlers for service list command
 func ServiceListHandlers(h hprinters.PrintHandler) {
 	kServiceColumnDefinitions := []metav1beta1.TableColumnDefinition{
-		{Name: "Name", Type: "string", Description: "Name of the knative service."},
+		{Name: "Name", Type: "string", Description: "Name of the Knative service."},
-		{Name: "Domain", Type: "string", Description: "Domain name of the knative service."},
+		{Name: "Url", Type: "string", Description: "URL of the Knative service."},
 		//{Name: "LastCreatedRevision", Type: "string", Description: "Name of last revision created."},
 		//{Name: "LastReadyRevision", Type: "string", Description: "Name of last ready revision."},
 		{Name: "Generation", Type: "integer", Description: "Sequence number of 'Generation' of the service that was last processed by the controller."},
@@ -57,7 +57,7 @@ func printKServiceList(kServiceList *servingv1alpha1.ServiceList, options hprint
 // printKService populates the knative service table rows
 func printKService(kService *servingv1alpha1.Service, options hprinters.PrintOptions) ([]metav1beta1.TableRow, error) {
 	name := kService.Name
-	domain := kService.Status.RouteStatusFields.DeprecatedDomain
+	url := kService.Status.URL
 	//lastCreatedRevision := kService.Status.LatestCreatedRevisionName
 	//lastReadyRevision := kService.Status.LatestReadyRevisionName
 	generation := kService.Status.ObservedGeneration
@@ -71,7 +71,7 @@ func printKService(kService *servingv1alpha1.Service, options hprinters.PrintOpt
 	}
 	row.Cells = append(row.Cells,
 		name,
-		domain,
+		url,
 		//lastCreatedRevision,
 		//lastReadyRevision,
 		generation,

pkg/kn/commands/service/service_list_test.go

@@ -18,14 +18,16 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/knative/client/pkg/kn/commands"
+	"github.com/knative/pkg/apis"
-	"github.com/knative/client/pkg/util"
 	duckv1beta1 "github.com/knative/pkg/apis/duck/v1beta1"
 	"github.com/knative/serving/pkg/apis/serving/v1alpha1"
 	"gotest.tools/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	client_testing "k8s.io/client-go/testing"
+
+	"github.com/knative/client/pkg/kn/commands"
+	"github.com/knative/client/pkg/util"
 )
 
 func fakeServiceList(args []string, response *v1alpha1.ServiceList) (action client_testing.Action, output []string, err error) {
@@ -74,8 +76,8 @@ func TestGetEmpty(t *testing.T) {
 }
 
 func TestServiceListDefaultOutput(t *testing.T) {
-	service1 := createMockServiceWithParams("foo", "foo.default.example.com", 1)
+	service1 := createMockServiceWithParams("foo", "http://foo.default.example.com", 1)
-	service2 := createMockServiceWithParams("bar", "bar.default.example.com", 2)
+	service2 := createMockServiceWithParams("bar", "http://bar.default.example.com", 2)
 	serviceList := &v1alpha1.ServiceList{Items: []v1alpha1.Service{*service1, *service2}}
 	action, output, err := fakeServiceList([]string{"service", "list"}, serviceList)
 	if err != nil {
@@ -86,7 +88,7 @@ func TestServiceListDefaultOutput(t *testing.T) {
 	} else if !action.Matches("list", "services") {
 		t.Errorf("Bad action %v", action)
 	}
-	assert.Check(t, util.ContainsAll(output[0], "NAME", "DOMAIN", "GENERATION", "AGE", "CONDITIONS", "READY", "REASON"))
+	assert.Check(t, util.ContainsAll(output[0], "NAME", "URL", "GENERATION", "AGE", "CONDITIONS", "READY", "REASON"))
 	assert.Check(t, util.ContainsAll(output[1], "foo", "foo.default.example.com", "1"))
 	assert.Check(t, util.ContainsAll(output[2], "bar", "bar.default.example.com", "2"))
 }
@@ -103,7 +105,7 @@ func TestServiceGetOneOutput(t *testing.T) {
 	} else if !action.Matches("list", "services") {
 		t.Errorf("Bad action %v", action)
 	}
-	assert.Check(t, util.ContainsAll(output[0], "NAME", "DOMAIN", "GENERATION", "AGE", "CONDITIONS", "READY", "REASON"))
+	assert.Check(t, util.ContainsAll(output[0], "NAME", "URL", "GENERATION", "AGE", "CONDITIONS", "READY", "REASON"))
 	assert.Check(t, util.ContainsAll(output[1], "foo", "foo.default.example.com", "1"))
 }
 
@@ -114,7 +116,8 @@ func TestServiceGetWithTwoSrvName(t *testing.T) {
 	assert.ErrorContains(t, err, "'kn service list' accepts maximum 1 argument")
 }
 
-func createMockServiceWithParams(name, domain string, generation int64) *v1alpha1.Service {
+func createMockServiceWithParams(name, urlS string, generation int64) *v1alpha1.Service {
+	url, _ := apis.ParseURL(urlS)
 	service := &v1alpha1.Service{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "Service",
@@ -131,7 +134,7 @@ func createMockServiceWithParams(name, domain string, generation int64) *v1alpha
 			Status: duckv1beta1.Status{
 				ObservedGeneration: generation},
 			RouteStatusFields: v1alpha1.RouteStatusFields{
-				DeprecatedDomain: domain,
+				URL: url,
 			},
 		},
 	}