From c214ed95464f2d240105572e7f1387e8aaa12ec1 Mon Sep 17 00:00:00 2001
From: Alexander Matyushentsev <Alexander_Matyushentsev@intuit.com>
Date: Fri, 31 May 2019 17:42:20 -0700
Subject: [PATCH] Issue #1668 - Replicasets ordering is not stable on app tree
 view (#1669)

---
 controller/cache/cluster.go      | 22 +++++++++++++++++-----
 controller/cache/cluster_test.go | 18 ++++++++++++++++++
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/controller/cache/cluster.go b/controller/cache/cluster.go
index c5e2aab92038e..ba9bc859ba2a6 100644
--- a/controller/cache/cluster.go
+++ b/controller/cache/cluster.go
@@ -4,6 +4,8 @@ import (
 	"context"
 	"fmt"
 	"runtime/debug"
+	"sort"
+	"strings"
 	"sync"
 	"time"
 
@@ -334,16 +336,26 @@ func (c *clusterInfo) iterateHierarchy(obj *unstructured.Unstructured, action fu
 	if objInfo, ok := c.nodes[key]; ok {
 		action(objInfo.asResourceNode())
 		nsNodes := c.nsIndex[key.Namespace]
-		children := make(map[types.UID]*node)
+		childrenByUID := make(map[types.UID][]*node)
 		for _, child := range nsNodes {
 			if objInfo.isParentOf(child) {
-				children[child.ref.UID] = child
+				childrenByUID[child.ref.UID] = append(childrenByUID[child.ref.UID], child)
 			}
 		}
 		// make sure children has no duplicates
-		for _, child := range children {
-			action(child.asResourceNode())
-			child.iterateChildren(nsNodes, map[kube.ResourceKey]bool{objInfo.resourceKey(): true}, action)
+		for _, children := range childrenByUID {
+			if len(children) > 0 {
+				// The object might have multiple children with the same UID (e.g. replicaset from apps and extensions group). It is ok to pick any object but we need to make sure
+				// we pick the same child after every refresh.
+				sort.Slice(children, func(i, j int) bool {
+					key1 := children[i].resourceKey()
+					key2 := children[j].resourceKey()
+					return strings.Compare(key1.String(), key2.String()) < 0
+				})
+				child := children[0]
+				action(child.asResourceNode())
+				child.iterateChildren(nsNodes, map[kube.ResourceKey]bool{objInfo.resourceKey(): true}, action)
+			}
 		}
 	} else {
 		action(c.createObjInfo(obj, c.settings.GetAppInstanceLabelKey()).asResourceNode())
diff --git a/controller/cache/cluster_test.go b/controller/cache/cluster_test.go
index a3bb19eee650b..aa767ea4508eb 100644
--- a/controller/cache/cluster_test.go
+++ b/controller/cache/cluster_test.go
@@ -433,3 +433,21 @@ func TestWatchCacheUpdated(t *testing.T) {
 	_, ok = cluster.nodes[kube.GetResourceKey(added)]
 	assert.True(t, ok)
 }
+
+func TestGetDuplicatedChildren(t *testing.T) {
+	extensionsRS := testRS.DeepCopy()
+	extensionsRS.SetGroupVersionKind(schema.GroupVersionKind{Group: "extensions", Kind: kube.ReplicaSetKind, Version: "v1beta1"})
+	cluster := newCluster(testDeploy, testRS, extensionsRS)
+	err := cluster.ensureSynced()
+
+	assert.Nil(t, err)
+
+	// Get children multiple times to make sure the right child is picked up every time.
+	for i := 0; i < 5; i++ {
+		children := getChildren(cluster, testDeploy)
+		assert.Len(t, children, 1)
+		assert.Equal(t, "apps", children[0].Group)
+		assert.Equal(t, kube.ReplicaSetKind, children[0].Kind)
+		assert.Equal(t, testRS.GetName(), children[0].Name)
+	}
+}