From a7bfe0e638e329d60cd5724bac0670c95db9412f Mon Sep 17 00:00:00 2001
From: Gustavo Paiva <guustavo.paiva@gmail.com>
Date: Wed, 9 Oct 2019 01:46:18 -0300
Subject: [PATCH] sort node roles

---
 internal/resource/no.go          | 39 ++++++--------------------------
 internal/resource/no_int_test.go | 37 ++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 32 deletions(-)

diff --git a/internal/resource/no.go b/internal/resource/no.go
index a4e34352..8b87cb86 100644
--- a/internal/resource/no.go
+++ b/internal/resource/no.go
@@ -143,8 +143,7 @@ func (r *Node) Fields(ns string) Row {
 
 	sta := make([]string, 10)
 	r.status(no.Status, no.Spec.Unschedulable, sta)
-	ro := make([]string, 10)
-	r.nodeRoles(no, ro)
+	ro := r.findNodeRoles(no)
 
 	return append(ff,
 		no.Name,
@@ -203,28 +202,20 @@ func gatherNodeMX(no *v1.Node, mx *mv1beta1.NodeMetrics) (c metric, a metric, p
 	return
 }
 
-func withPerc(v, p string) string {
-	return v + " (" + p + ")"
-}
-
-func (*Node) nodeRoles(node *v1.Node, res []string) {
-	index := 0
-	for k, v := range node.Labels {
+func (_ *Node) findNodeRoles(no *v1.Node) []string {
+	roles := sets.NewString()
+	for k, v := range no.Labels {
 		switch {
 		case strings.HasPrefix(k, labelNodeRolePrefix):
 			if role := strings.TrimPrefix(k, labelNodeRolePrefix); len(role) > 0 {
-				res[index] = role
-				index++
+				roles.Insert(role)
 			}
 		case k == nodeLabelRole && v != "":
-			res[index] = v
-			index++
+			roles.Insert(v)
 		}
 	}
 
-	if empty(res) {
-		res[index] = MissingValue
-	}
+	return roles.List()
 }
 
 func (*Node) getIPs(addrs []v1.NodeAddress) (iIP, eIP string) {
@@ -271,22 +262,6 @@ func (*Node) status(status v1.NodeStatus, exempt bool, res []string) {
 	}
 }
 
-func findNodeRoles(no *v1.Node) []string {
-	roles := sets.NewString()
-	for k, v := range no.Labels {
-		switch {
-		case strings.HasPrefix(k, labelNodeRolePrefix):
-			if role := strings.TrimPrefix(k, labelNodeRolePrefix); len(role) > 0 {
-				roles.Insert(role)
-			}
-		case k == nodeLabelRole && v != "":
-			roles.Insert(v)
-		}
-	}
-
-	return roles.List()
-}
-
 func (r *Node) podsResources(name string) (v1.ResourceList, v1.ResourceList, error) {
 	reqs, limits := v1.ResourceList{}, v1.ResourceList{}
 	pods, err := r.Connection.NodePods(name)
diff --git a/internal/resource/no_int_test.go b/internal/resource/no_int_test.go
index 477998b6..170b8b4d 100644
--- a/internal/resource/no_int_test.go
+++ b/internal/resource/no_int_test.go
@@ -34,6 +34,43 @@ func TestNodeStatus(t *testing.T) {
 	}
 }
 
+func TestNodeRoles(t *testing.T) {
+	uu := []struct {
+		node  v1.Node
+		roles []string
+	}{
+		{
+			node: v1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Labels: map[string]string{
+						"node-role.kubernetes.io/master": "true",
+						"node-role.kubernetes.io/worker": "true",
+					},
+				},
+			},
+			roles: []string{"master", "worker"},
+		},
+
+		{
+			node: v1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Labels: map[string]string{
+						"node-role.kubernetes.io/worker": "true",
+						"node-role.kubernetes.io/master": "true",
+					},
+				},
+			},
+			roles: []string{"master", "worker"},
+		},
+	}
+
+	no := NewNode(nil)
+	for _, u := range uu {
+		roles := no.findNodeRoles(&u.node)
+		assert.Equal(t, u.roles, roles)
+	}
+}
+
 func BenchmarkNodeFields(b *testing.B) {
 	n := NewNode(nil)
 	no := makeNode()