🌱 refactor SortForCreate to use sort.Slice

Author: kokes

util/resource/resource.go

@@ -17,61 +17,54 @@ limitations under the License.
 // Package resource implements resource utilites.
 package resource
 
-import "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+import (
+	"sort"
 
-var defaultCreatePriorities = []string{
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+)
+
+var priorityMap = map[string]int{
 	// Namespaces go first because all namespaced resources depend on them.
-	"Namespace",
+	"Namespace": -100,
 	// Custom Resource Definitions come before Custom Resource so that they can be
 	// restored with their corresponding CRD.
-	"CustomResourceDefinition",
+	"CustomResourceDefinition": -99,
 	// Storage Classes are needed to create PVs and PVCs correctly.
-	"StorageClass",
+	"StorageClass": -98,
 	// PVs go before PVCs because PVCs depend on them.
-	"PersistentVolume",
+	"PersistentVolume": -97,
 	// PVCs go before pods or controllers so they can be mounted as volumes.
-	"PersistentVolumeClaim",
+	"PersistentVolumeClaim": -96,
 	// Secrets and ConfigMaps go before pods or controllers so they can be mounted as volumes.
-	"Secret",
-	"ConfigMap",
+	"Secret":    -95,
+	"ConfigMap": -94,
 	// Service accounts go before pods or controllers so pods can use them.
-	"ServiceAccount",
+	"ServiceAccount": -93,
 	// Limit ranges go before pods or controllers so pods can use them.
-	"LimitRange",
+	"LimitRange": -92,
 	// Pods go before ReplicaSets
-	"Pods",
+	"Pod": -91,
 	// ReplicaSets go before Deployments
-	"ReplicaSet",
-	// Endpoints go before Services
-	"Endpoints",
+	"ReplicaSet": -90,
+	// Endpoints go before Services (and everything else)
+	"Endpoint": -89,
+}
+
+// priorityLess returns true if o1 should be created before o2.
+// To be used in sort.{Slice,SliceStable} to sort manifests in place.
+func priorityLess(o1, o2 unstructured.Unstructured) bool {
+	p1 := priorityMap[o1.GetKind()]
+	p2 := priorityMap[o2.GetKind()]
+	return p1 < p2
 }
 
 // SortForCreate sorts objects by creation priority.
 func SortForCreate(resources []unstructured.Unstructured) []unstructured.Unstructured {
-	var ret []unstructured.Unstructured
-
-	// First get resources by priority
-	for _, p := range defaultCreatePriorities {
-		for _, o := range resources {
-			if o.GetKind() == p {
-				ret = append(ret, o)
-			}
-		}
-	}
-
-	// Then get all the other resources
-	for _, o := range resources {
-		found := false
-		for _, r := range ret {
-			if o.GroupVersionKind() == r.GroupVersionKind() && o.GetNamespace() == r.GetNamespace() && o.GetName() == r.GetName() {
-				found = true
-				break
-			}
-		}
-		if !found {
-			ret = append(ret, o)
-		}
-	}
+	ret := make([]unstructured.Unstructured, len(resources))
+	copy(ret, resources)
+	sort.SliceStable(ret, func(i, j int) bool {
+		return priorityLess(ret[i], ret[j])
+	})
 
 	return ret
 }

util/resource/resource_test.go

@@ -17,12 +17,25 @@ limitations under the License.
 package resource
 
 import (
+	"math/rand"
 	"testing"
 
 	. "github.com/onsi/gomega"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
+var (
+	kindsWithPriority = []string{
+		"Namespace", "CustomResourceDefinition", "StorageClass", "PersistentVolume",
+		"PersistentVolumeClaim", "Secret", "ConfigMap", "ServiceAccount", "LimitRange",
+		"Pod", "ReplicaSet", "Endpoint",
+	}
+	kindsOther = []string{
+		"Deployment", "Service", "DaemonSet", "StatefulSet", "Job", "CronJob", "Ingress",
+		"NetworkPolicy", "Role", "RoleBinding",
+	}
+)
+
 func TestSortForCreate(t *testing.T) {
 	g := NewWithT(t)
 
@@ -35,9 +48,49 @@ func TestSortForCreate(t *testing.T) {
 	ep := unstructured.Unstructured{}
 	ep.SetKind("Endpoint")
 
-	resources := []unstructured.Unstructured{ep, cm, ns}
+	dp := unstructured.Unstructured{}
+	dp.SetKind("Deployment")
+
+	ds := unstructured.Unstructured{}
+	ds.SetKind("DaemonSet")
+
+	resources := []unstructured.Unstructured{ds, dp, ep, cm, ns}
 	sorted := SortForCreate(resources)
-	g.Expect(sorted).To(HaveLen(3))
+	g.Expect(sorted).To(HaveLen(5))
 	g.Expect(sorted[0].GetKind()).To(BeIdenticalTo("Namespace"))
 	g.Expect(sorted[1].GetKind()).To(BeIdenticalTo("ConfigMap"))
+	g.Expect(sorted[2].GetKind()).To(BeIdenticalTo("Endpoint"))
+	// we have no way to determine the order of the last two elements
+	g.Expect(sorted[3].GetKind()).To(BeElementOf([]string{"DaemonSet", "Deployment"}))
+	g.Expect(sorted[4].GetKind()).To(BeElementOf([]string{"DaemonSet", "Deployment"}))
+}
+
+func TestSortForCreateAllShuffle(t *testing.T) {
+	g := NewWithT(t)
+
+	resources := make([]unstructured.Unstructured, 0, len(kindsWithPriority)+len(kindsOther))
+	for _, kind := range append(kindsWithPriority, kindsOther...) {
+		resource := unstructured.Unstructured{}
+		resource.SetKind(kind)
+		resources = append(resources, resource)
+	}
+	for j := 0; j < 100; j++ {
+		// determinically shuffle resources
+		rnd := rand.New(rand.NewSource(int64(j))) //nolint:gosec
+		rnd.Shuffle(len(resources), func(i, j int) {
+			resources[i], resources[j] = resources[j], resources[i]
+		})
+
+		sorted := SortForCreate(resources)
+		g.Expect(sorted).To(HaveLen(len(kindsWithPriority) + len(kindsOther)))
+		// first check that the first len(kindsWithPriority) elements are from
+		// the list of kinds with priority (and in order)
+		for i, res := range sorted[:len(kindsWithPriority)] {
+			g.Expect(res.GetKind()).To(BeIdenticalTo(kindsWithPriority[i]))
+		}
+		// while the rest of resources can be any of the other kinds
+		for _, res := range sorted[len(kindsWithPriority):] {
+			g.Expect(kindsOther).To(ContainElement(res.GetKind()))
+		}
+	}
 }