Merge pull request #5140 from eladiw/master

Fix #3127: Sort resources output by 'helm status'

pkg/kube/client.go

@@ -25,6 +25,7 @@ import (
 	"io"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"log"
+	"sort"
 	"strings"
 	"time"
 
@@ -154,13 +155,41 @@ func (c *Client) Build(namespace string, reader io.Reader) (Result, error) {
 	return result, scrubValidationError(err)
 }
 
+// Return the resource info as internal
+func resourceInfoToObject(info *resource.Info, c *Client) runtime.Object {
+	internalObj, err := asInternal(info)
+	if err != nil {
+		// If the problem is just that the resource is not registered, don't print any
+		// error. This is normal for custom resources.
+		if !runtime.IsNotRegisteredError(err) {
+			c.Log("Warning: conversion to internal type failed: %v", err)
+		}
+		// Add the unstructured object in this situation. It will still get listed, just
+		// with less information.
+		return info.Object
+	}
+
+	return internalObj
+}
+
+func sortByKey(objs map[string](map[string]runtime.Object)) []string {
+	var keys []string
+	// Create a simple slice, so we can sort it
+	for key := range objs {
+		keys = append(keys, key)
+	}
+	// Sort alphabetically by version/kind keys
+	sort.Strings(keys)
+	return keys
+}
+
 // Get gets Kubernetes resources as pretty-printed string.
 //
 // Namespace will set the namespace.
 func (c *Client) Get(namespace string, reader io.Reader) (string, error) {
-	// Since we don't know what order the objects come in, let's group them by the types, so
+	// Since we don't know what order the objects come in, let's group them by the types and then sort them, so
 	// that when we print them, they come out looking good (headers apply to subgroups, etc.).
-	objs := make(map[string][]runtime.Object)
+	objs := make(map[string](map[string]runtime.Object))
 	infos, err := c.BuildUnstructured(namespace, reader)
 	if err != nil {
 		return "", err
@@ -181,19 +210,15 @@ func (c *Client) Get(namespace string, reader io.Reader) (string, error) {
 		// versions per cluster, but this certainly won't hurt anything, so let's be safe.
 		gvk := info.ResourceMapping().GroupVersionKind
 		vk := gvk.Version + "/" + gvk.Kind
-		internalObj, err := asInternal(info)
-		if err != nil {
-			// If the problem is just that the resource is not registered, don't print any
-			// error. This is normal for custom resources.
-			if !runtime.IsNotRegisteredError(err) {
-				c.Log("Warning: conversion to internal type failed: %v", err)
-			}
-			// Add the unstructured object in this situation. It will still get listed, just
-			// with less information.
-			objs[vk] = append(objs[vk], info.Object)
-		} else {
-			objs[vk] = append(objs[vk], internalObj)
+
+		// Initialize map. The main map groups resources based on version/kind
+		// The second level is a simple 'Name' to 'Object', that will help sort
+		// the individual resource later
+		if objs[vk] == nil {
+			objs[vk] = make(map[string]runtime.Object)
 		}
+		// Map between the resource name to the underlying info object
+		objs[vk][info.Name] = resourceInfoToObject(info, c)
 
 		//Get the relation pods
 		objPods, err = c.getSelectRelationPod(info, objPods)
@@ -211,8 +236,12 @@ func (c *Client) Get(namespace string, reader io.Reader) (string, error) {
 	for key, podItems := range objPods {
 		for i := range podItems {
 			pod := &core.Pod{}
+
 			legacyscheme.Scheme.Convert(&podItems[i], pod, nil)
-			objs[key+"(related)"] = append(objs[key+"(related)"], pod)
+			if objs[key+"(related)"] == nil {
+				objs[key+"(related)"] = make(map[string]runtime.Object)
+			}
+			objs[key+"(related)"][pod.ObjectMeta.Name] = runtime.Object(pod)
 		}
 	}
 
@@ -222,14 +251,28 @@ func (c *Client) Get(namespace string, reader io.Reader) (string, error) {
 	// track of tab widths.
 	buf := new(bytes.Buffer)
 	printFlags := get.NewHumanPrintFlags()
-	for t, ot := range objs {
+
+	// Sort alphabetically by version/kind keys
+	vkKeys := sortByKey(objs)
+	// Iterate on sorted version/kind types
+	for _, t := range vkKeys {
 		if _, err = fmt.Fprintf(buf, "==> %s\n", t); err != nil {
 			return "", err
 		}
 		typePrinter, _ := printFlags.ToPrinter("")
-		for _, o := range ot {
-			if err := typePrinter.PrintObj(o, buf); err != nil {
-				c.Log("failed to print object type %s, object: %q :\n %v", t, o, err)
+
+		var sortedResources []string
+		for resource := range objs[t] {
+			sortedResources = append(sortedResources, resource)
+		}
+		sort.Strings(sortedResources)
+
+		// Now that each individual resource within the specific version/kind
+		// is sorted, we print each resource using the k8s printer
+		vk := objs[t]
+		for _, resourceName := range sortedResources {
+			if err := typePrinter.PrintObj(vk[resourceName], buf); err != nil {
+				c.Log("failed to print object type %s, object: %q :\n %v", t, resourceName, err)
 				return "", err
 			}
 		}

pkg/kube/client_test.go

@@ -21,6 +21,7 @@ import (
 	"io"
 	"io/ioutil"
 	"net/http"
+	"sort"
 	"strings"
 	"testing"
 
@@ -77,6 +78,18 @@ func newPodList(names ...string) v1.PodList {
 	return list
 }
 
+func newService(name string) v1.Service {
+	ns := v1.NamespaceDefault
+	return v1.Service{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: ns,
+			SelfLink:  "/api/v1/namespaces/default/services/" + name,
+		},
+		Spec: v1.ServiceSpec{},
+	}
+}
+
 func notFoundBody() *metav1.Status {
 	return &metav1.Status{
 		Code:    http.StatusNotFound,
@@ -295,6 +308,95 @@ func TestGet(t *testing.T) {
 	}
 }
 
+func TestResourceTypeSortOrder(t *testing.T) {
+	pod := newPod("my-pod")
+	service := newService("my-service")
+	c := newTestClient()
+	defer c.Cleanup()
+	c.TestFactory.UnstructuredClient = &fake.RESTClient{
+		GroupVersion:         schema.GroupVersion{Version: "v1"},
+		NegotiatedSerializer: unstructuredSerializer,
+		Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
+			p, m := req.URL.Path, req.Method
+			t.Logf("got request %s %s", p, m)
+			switch {
+			case p == "/namespaces/default/pods/my-pod" && m == "GET":
+				return newResponse(200, &pod)
+			case p == "/namespaces/default/services/my-service" && m == "GET":
+				return newResponse(200, &service)
+			default:
+				t.Fatalf("unexpected request: %s %s", req.Method, req.URL.Path)
+				return nil, nil
+			}
+		}),
+	}
+
+	// Test sorting order
+	data := strings.NewReader(testResourceTypeSortOrder)
+	o, err := c.Get("default", data)
+	if err != nil {
+		t.Errorf("Expected missing results, got %q", err)
+	}
+	podIndex := strings.Index(o, "my-pod")
+	serviceIndex := strings.Index(o, "my-service")
+	if podIndex == -1 {
+		t.Errorf("Expected v1/Pod my-pod, got %s", o)
+	}
+	if serviceIndex == -1 {
+		t.Errorf("Expected v1/Service my-service, got %s", o)
+	}
+	if !sort.IntsAreSorted([]int{podIndex, serviceIndex}) {
+		t.Errorf("Expected order: [v1/Pod v1/Service], got %s", o)
+	}
+}
+
+func TestResourceSortOrder(t *testing.T) {
+	list := newPodList("albacore", "coral", "beluga")
+	c := newTestClient()
+	defer c.Cleanup()
+	c.TestFactory.UnstructuredClient = &fake.RESTClient{
+		GroupVersion:         schema.GroupVersion{Version: "v1"},
+		NegotiatedSerializer: unstructuredSerializer,
+		Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
+			p, m := req.URL.Path, req.Method
+			t.Logf("got request %s %s", p, m)
+			switch {
+			case p == "/namespaces/default/pods/albacore" && m == "GET":
+				return newResponse(200, &list.Items[0])
+			case p == "/namespaces/default/pods/coral" && m == "GET":
+				return newResponse(200, &list.Items[1])
+			case p == "/namespaces/default/pods/beluga" && m == "GET":
+				return newResponse(200, &list.Items[2])
+			default:
+				t.Fatalf("unexpected request: %s %s", req.Method, req.URL.Path)
+				return nil, nil
+			}
+		}),
+	}
+
+	// Test sorting order
+	data := strings.NewReader(testResourceSortOrder)
+	o, err := c.Get("default", data)
+	if err != nil {
+		t.Errorf("Expected missing results, got %q", err)
+	}
+	albacoreIndex := strings.Index(o, "albacore")
+	belugaIndex := strings.Index(o, "beluga")
+	coralIndex := strings.Index(o, "coral")
+	if albacoreIndex == -1 {
+		t.Errorf("Expected v1/Pod albacore, got %s", o)
+	}
+	if belugaIndex == -1 {
+		t.Errorf("Expected v1/Pod beluga, got %s", o)
+	}
+	if coralIndex == -1 {
+		t.Errorf("Expected v1/Pod coral, got %s", o)
+	}
+	if !sort.IntsAreSorted([]int{albacoreIndex, belugaIndex, coralIndex}) {
+		t.Errorf("Expected order: [albacore beluga coral], got %s", o)
+	}
+}
+
 func TestPerform(t *testing.T) {
 	tests := []struct {
 		name       string
@@ -376,6 +478,35 @@ func TestReal(t *testing.T) {
 	}
 }
 
+const testResourceTypeSortOrder = `
+kind: Service
+apiVersion: v1
+metadata:
+  name: my-service
+---
+kind: Pod
+apiVersion: v1
+metadata:
+  name: my-pod
+`
+
+const testResourceSortOrder = `
+kind: Pod
+apiVersion: v1
+metadata:
+  name: albacore
+---
+kind: Pod
+apiVersion: v1
+metadata:
+  name: coral
+---
+kind: Pod
+apiVersion: v1
+metadata:
+  name: beluga
+`
+
 const testServiceManifest = `
 kind: Service
 apiVersion: v1