refactor(service list): Removed special "default" namespace handling (#608)

Fixes #607.

pkg/kn/commands/service/human_readable_flags.go

@@ -15,13 +15,12 @@
 package service
 
 import (
-	"sort"
-
 	metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1"
 	"k8s.io/apimachinery/pkg/runtime"
+	servingv1alpha1 "knative.dev/serving/pkg/apis/serving/v1alpha1"
+
 	"knative.dev/client/pkg/kn/commands"
 	hprinters "knative.dev/client/pkg/printers"
-	servingv1alpha1 "knative.dev/serving/pkg/apis/serving/v1alpha1"
 )
 
 // ServiceListHandlers adds print handlers for service list command
@@ -47,10 +46,6 @@ func ServiceListHandlers(h hprinters.PrintHandler) {
 func printKServiceList(kServiceList *servingv1alpha1.ServiceList, options hprinters.PrintOptions) ([]metav1beta1.TableRow, error) {
 	rows := make([]metav1beta1.TableRow, 0, len(kServiceList.Items))
 
-	if options.AllNamespaces {
-		return printKServiceWithNaemspace(kServiceList, options)
-	}
-
 	for _, ksvc := range kServiceList.Items {
 		r, err := printKService(&ksvc, options)
 		if err != nil {
@@ -61,39 +56,6 @@ func printKServiceList(kServiceList *servingv1alpha1.ServiceList, options hprint
 	return rows, nil
 }
 
-// printKServiceWithNaemspace populates the knative service table rows with namespace column
-func printKServiceWithNaemspace(kServiceList *servingv1alpha1.ServiceList, options hprinters.PrintOptions) ([]metav1beta1.TableRow, error) {
-	rows := make([]metav1beta1.TableRow, 0, len(kServiceList.Items))
-
-	// temporary slice for sorting services in non-default namespace
-	others := []metav1beta1.TableRow{}
-
-	for _, ksvc := range kServiceList.Items {
-		// Fill in with services in `default` namespace at first
-		if ksvc.Namespace == "default" {
-			r, err := printKService(&ksvc, options)
-			if err != nil {
-				return nil, err
-			}
-			rows = append(rows, r...)
-			continue
-		}
-		// put other services in temporary slice
-		r, err := printKService(&ksvc, options)
-		if err != nil {
-			return nil, err
-		}
-		others = append(others, r...)
-	}
-
-	// sort other services list alphabetically by namespace
-	sort.SliceStable(others, func(i, j int) bool {
-		return others[i].Cells[0].(string) < others[j].Cells[0].(string)
-	})
-
-	return append(rows, others...), nil
-}
-
 // printKService populates the knative service table rows
 func printKService(kService *servingv1alpha1.Service, options hprinters.PrintOptions) ([]metav1beta1.TableRow, error) {
 	name := kService.Name

pkg/kn/commands/service/list.go

@@ -69,9 +69,15 @@ func NewServiceListCommand(p *commands.KnParams) *cobra.Command {
 				return err
 			}
 
-			// Sort serviceList by name
+			// Sort serviceList by namespace and name (in this order)
 			sort.SliceStable(serviceList.Items, func(i, j int) bool {
-				return serviceList.Items[i].ObjectMeta.Name < serviceList.Items[j].ObjectMeta.Name
+				a := serviceList.Items[i]
+				b := serviceList.Items[j]
+
+				if a.Namespace != b.Namespace {
+					return a.Namespace < b.Namespace
+				}
+				return a.ObjectMeta.Name < b.ObjectMeta.Name
 			})
 
 			err = printer.PrintObj(serviceList, cmd.OutOrStdout())

pkg/kn/commands/service/list_mock_test.go

@@ -36,8 +36,8 @@ func TestServiceListAllNamespaceMock(t *testing.T) {
 
 	outputLines := strings.Split(output, "\n")
 	assert.Assert(t, util.ContainsAll(outputLines[0], "NAMESPACE", "NAME", "URL", "LATEST", "AGE", "CONDITIONS", "READY", "REASON"))
-	assert.Assert(t, util.ContainsAll(outputLines[1], "default", "svc1"))
+	assert.Assert(t, util.ContainsAll(outputLines[1], "bar", "svc3"))
-	assert.Assert(t, util.ContainsAll(outputLines[2], "bar", "svc3"))
+	assert.Assert(t, util.ContainsAll(outputLines[2], "default", "svc1"))
 	assert.Assert(t, util.ContainsAll(outputLines[3], "foo", "svc2"))
 
 	r.Validate()

pkg/kn/commands/service/list_test.go

@@ -103,8 +103,8 @@ func TestServiceListAllNamespacesOutput(t *testing.T) {
 	}
 	// Outputs in alphabetical order
 	assert.Check(t, util.ContainsAll(output[0], "NAMESPACE", "NAME", "URL", "LATEST", "AGE", "CONDITIONS", "READY", "REASON"))
-	assert.Check(t, util.ContainsAll(output[1], "default", "foo", "foo.default.example.com", "foo-xyz"))
+	assert.Check(t, util.ContainsAll(output[1], "bar", "sss", "sss.bar.example.com", "sss-xyz"))
-	assert.Check(t, util.ContainsAll(output[2], "bar", "sss", "sss.bar.example.com", "sss-xyz"))
+	assert.Check(t, util.ContainsAll(output[2], "default", "foo", "foo.default.example.com", "foo-xyz"))
 	assert.Check(t, util.ContainsAll(output[3], "foo", "bar", "bar.foo.example.com", "bar-xyz"))
 }