Fix sorting to keep container/pod intervals together in the chart

openshift · Oct 11, 2023 · 3411adc · 3411adc
1 parent 48f03bc
commit 3411adc
Show file tree

Hide file tree

Showing 9 changed files with 71 additions and 70 deletions.
diff --git a/pkg/monitor/monitorapi/types.go b/pkg/monitor/monitorapi/types.go
@@ -328,41 +328,8 @@ func (i Locator) OldLocator() string {
 	for k := range i.Keys {
 		keys = append(keys, string(k))
 	}
-	// Some components rely on the ordering of keys in the locator for groupings in interval charts.
-	// (i.e. namespace pod uid container, which groups container events with their pod.
-	// Blindly going through the keys results in alphabetical ordering, container comes first, and then we've
-	// got container events separated from their pod events on the intervals chart.
-	// This will hopefully eventually go away but for now we need it.
-
-	// Ensure these keys appear in this order. Other keys can be mixed in and their order will be preserved at the end,
-	// but we want these specific combos to come in this order so items appear together in interval charts
-	// which sort on locator.
-	// This function is courtesy of chatgpt, but unit tested.
-	keysToEnsure := []string{"namespace", "pod", "uid", "container"}
-
-	// Create a map to store the index of each key in the keysToEnsure slice
-	orderMap := make(map[string]int, len(keysToEnsure))
-	for i, key := range keysToEnsure {
-		orderMap[key] = i
-	}
-
-	// Use a custom sorting function to reorder the keys
-	customSort(keys, func(i, j int) bool {
-		idxI, foundI := orderMap[keys[i]]
-		idxJ, foundJ := orderMap[keys[j]]
-
-		// If either key is not in the keysToEnsure, keep their relative order
-		if !foundI && !foundJ {
-			return i < j
-		} else if !foundI {
-			return false
-		} else if !foundJ {
-			return true
-		}
 
-		// Compare the indices of keysToEnsure
-		return idxI < idxJ
-	})
+	keys = sortKeys(keys)
 
 	annotations := []string{}
 	for _, k := range keys {
@@ -379,15 +346,49 @@ func (i Locator) OldLocator() string {
 	return annotationString
 }
 
-// customSort sorts the keys slice using a custom sorting function
-func customSort(keys []string, less func(i, j int) bool) {
-	for i := 0; i < len(keys); i++ {
-		for j := i + 1; j < len(keys); j++ {
-			if less(j, i) {
-				keys[i], keys[j] = keys[j], keys[i]
-			}
-		}
+// sortKeys ensures that some keys appear in the order we require (least specific to most), so rows with locators
+// are grouped together. (i.e. keeping containers within the same pod together, or rows for a specific container)
+// Blindly going through the keys results in alphabetical ordering, container comes first, and then we've
+// got container events separated from their pod events on the intervals chart.
+// This will hopefully eventually go away but for now we need it.
+// Courtesy of ChatGPT but unit tested.
+func sortKeys(keys []string) []string {
+
+	// Ensure these keys appear in this order. Other keys can be mixed in and will appear at the end in alphabetical
+	// order.
+	orderedKeys := []string{"namespace", "node", "pod", "uid", "server", "container", "shutdown"}
+
+	// Create a map to store the indices of keys in the orderedKeys array.
+	// This will allow us to efficiently check if a key is in orderedKeys and find its position.
+	orderedKeyIndices := make(map[string]int)
+
+	for i, key := range orderedKeys {
+		orderedKeyIndices[key] = i
 	}
+
+	// Define a custom sorting function that orders the keys based on the orderedKeys array.
+	sort.Slice(keys, func(i, j int) bool {
+		// Get the indices of keys i and j in orderedKeys.
+		indexI, existsI := orderedKeyIndices[keys[i]]
+		indexJ, existsJ := orderedKeyIndices[keys[j]]
+
+		// If both keys exist in orderedKeys, sort them based on their order.
+		if existsI && existsJ {
+			return indexI < indexJ
+		}
+
+		// If only one of the keys exists in orderedKeys, move it to the front.
+		if existsI {
+			return true
+		} else if existsJ {
+			return false
+		}
+
+		// If neither key is in orderedKeys, sort alphabetically so we have predictable ordering
+		return keys[i] < keys[j]
+	})
+
+	return keys
 }
 
 type IntervalFilter func(i Interval) bool

diff --git a/pkg/monitortests/node/watchpods/podTest/container-life/expected.json b/pkg/monitortests/node/watchpods/podTest/container-life/expected.json
@@ -27,7 +27,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/without-label namespace/e2e-kubectl-3271 pod/without-label uid/e185b70c-ea3e-4600-850a-b2370a729a73",
+            "locator": "namespace/e2e-kubectl-3271 pod/without-label uid/e185b70c-ea3e-4600-850a-b2370a729a73 container/without-label",
             "message": "constructed/pod-lifecycle-constructor reason/ContainerWait missed real \"ContainerWait\"",
             "tempSource": "PodState",
             "tempStructuredLocator": {
@@ -79,7 +79,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/without-label namespace/e2e-kubectl-3271 pod/without-label uid/e185b70c-ea3e-4600-850a-b2370a729a73",
+            "locator": "namespace/e2e-kubectl-3271 pod/without-label uid/e185b70c-ea3e-4600-850a-b2370a729a73 container/without-label",
             "message": "constructed/pod-lifecycle-constructor reason/NotReady missed real \"NotReady\"",
             "tempSource": "PodState",
             "tempStructuredLocator": {
@@ -105,7 +105,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/without-label namespace/e2e-kubectl-3271 pod/without-label uid/e185b70c-ea3e-4600-850a-b2370a729a73",
+            "locator": "namespace/e2e-kubectl-3271 pod/without-label uid/e185b70c-ea3e-4600-850a-b2370a729a73 container/without-label",
             "message": "cause/ constructed/pod-lifecycle-constructor duration/6.00s reason/ContainerStart",
             "tempSource": "PodState",
             "tempStructuredLocator": {
@@ -133,7 +133,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/without-label namespace/e2e-kubectl-3271 pod/without-label uid/e185b70c-ea3e-4600-850a-b2370a729a73",
+            "locator": "namespace/e2e-kubectl-3271 pod/without-label uid/e185b70c-ea3e-4600-850a-b2370a729a73 container/without-label",
             "message": "constructed/pod-lifecycle-constructor reason/Ready",
             "tempSource": "PodState",
             "tempStructuredLocator": {

diff --git a/pkg/monitortests/node/watchpods/podTest/end-before-begin/expected.json b/pkg/monitortests/node/watchpods/podTest/end-before-begin/expected.json
@@ -53,7 +53,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/pruner namespace/openshift-kube-apiserver pod/revision-pruner-7-ip-10-0-214-214.us-west-1.compute.internal uid/6a81964d-169c-47e0-a986-551429370ae9",
+            "locator": "namespace/openshift-kube-apiserver pod/revision-pruner-7-ip-10-0-214-214.us-west-1.compute.internal uid/6a81964d-169c-47e0-a986-551429370ae9 container/pruner",
             "message": "constructed/pod-lifecycle-constructor reason/NotReady",
             "tempSource": "PodState",
             "tempStructuredLocator": {

diff --git a/pkg/monitortests/node/watchpods/podTest/installer-pod/expected.json b/pkg/monitortests/node/watchpods/podTest/installer-pod/expected.json
@@ -27,7 +27,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/installer namespace/openshift-etcd pod/installer-9-ci-op-97t906zm-db044-bwrrn-master-0 uid/6fb10c53-7ed9-4f51-88db-f8a689050f21",
+            "locator": "namespace/openshift-etcd pod/installer-9-ci-op-97t906zm-db044-bwrrn-master-0 uid/6fb10c53-7ed9-4f51-88db-f8a689050f21 container/installer",
             "message": "constructed/pod-lifecycle-constructor reason/ContainerWait missed real \"ContainerWait\"",
             "tempSource": "PodState",
             "tempStructuredLocator": {
@@ -79,7 +79,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/installer namespace/openshift-etcd pod/installer-9-ci-op-97t906zm-db044-bwrrn-master-0 uid/6fb10c53-7ed9-4f51-88db-f8a689050f21",
+            "locator": "namespace/openshift-etcd pod/installer-9-ci-op-97t906zm-db044-bwrrn-master-0 uid/6fb10c53-7ed9-4f51-88db-f8a689050f21 container/installer",
             "message": "constructed/pod-lifecycle-constructor reason/NotReady",
             "tempSource": "PodState",
             "tempStructuredLocator": {
@@ -105,7 +105,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/installer namespace/openshift-etcd pod/installer-9-ci-op-97t906zm-db044-bwrrn-master-0 uid/6fb10c53-7ed9-4f51-88db-f8a689050f21",
+            "locator": "namespace/openshift-etcd pod/installer-9-ci-op-97t906zm-db044-bwrrn-master-0 uid/6fb10c53-7ed9-4f51-88db-f8a689050f21 container/installer",
             "message": "cause/ constructed/pod-lifecycle-constructor duration/3.00s reason/ContainerStart",
             "tempSource": "PodState",
             "tempStructuredLocator": {
@@ -133,7 +133,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/installer namespace/openshift-etcd pod/installer-9-ci-op-97t906zm-db044-bwrrn-master-0 uid/6fb10c53-7ed9-4f51-88db-f8a689050f21",
+            "locator": "namespace/openshift-etcd pod/installer-9-ci-op-97t906zm-db044-bwrrn-master-0 uid/6fb10c53-7ed9-4f51-88db-f8a689050f21 container/installer",
             "message": "constructed/pod-lifecycle-constructor reason/Ready",
             "tempSource": "PodState",
             "tempStructuredLocator": {
@@ -159,7 +159,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/installer namespace/openshift-etcd pod/installer-9-ci-op-97t906zm-db044-bwrrn-master-0 uid/6fb10c53-7ed9-4f51-88db-f8a689050f21",
+            "locator": "namespace/openshift-etcd pod/installer-9-ci-op-97t906zm-db044-bwrrn-master-0 uid/6fb10c53-7ed9-4f51-88db-f8a689050f21 container/installer",
             "message": "constructed/pod-lifecycle-constructor reason/NotReady",
             "tempSource": "PodState",
             "tempStructuredLocator": {

diff --git a/pkg/monitortests/node/watchpods/podTest/long-not-ready/expected.json b/pkg/monitortests/node/watchpods/podTest/long-not-ready/expected.json
@@ -2,7 +2,7 @@
     "items": [
         {
             "level": "Info",
-            "locator": "container/fix-audit-permissions namespace/openshift-apiserver pod/apiserver-5b9785f765-qk9hl uid/d5f66519-ca7a-4808-94a0-b889552d411c",
+            "locator": "namespace/openshift-apiserver pod/apiserver-5b9785f765-qk9hl uid/d5f66519-ca7a-4808-94a0-b889552d411c container/fix-audit-permissions",
             "message": "constructed/pod-lifecycle-constructor reason/NotReady missed real \"NotReady\"",
             "tempSource": "PodState",
             "tempStructuredLocator": {
@@ -53,7 +53,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/openshift-apiserver namespace/openshift-apiserver pod/apiserver-5b9785f765-qk9hl uid/d5f66519-ca7a-4808-94a0-b889552d411c",
+            "locator": "namespace/openshift-apiserver pod/apiserver-5b9785f765-qk9hl uid/d5f66519-ca7a-4808-94a0-b889552d411c container/openshift-apiserver",
             "message": "constructed/pod-lifecycle-constructor reason/NotReady missed real \"NotReady\"",
             "tempSource": "PodState",
             "tempStructuredLocator": {
@@ -79,7 +79,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/openshift-apiserver-check-endpoints namespace/openshift-apiserver pod/apiserver-5b9785f765-qk9hl uid/d5f66519-ca7a-4808-94a0-b889552d411c",
+            "locator": "namespace/openshift-apiserver pod/apiserver-5b9785f765-qk9hl uid/d5f66519-ca7a-4808-94a0-b889552d411c container/openshift-apiserver-check-endpoints",
             "message": "constructed/pod-lifecycle-constructor reason/NotReady missed real \"NotReady\"",
             "tempSource": "PodState",
             "tempStructuredLocator": {
@@ -105,7 +105,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/fix-audit-permissions namespace/openshift-apiserver pod/apiserver-5b9785f765-qk9hl uid/d5f66519-ca7a-4808-94a0-b889552d411c",
+            "locator": "namespace/openshift-apiserver pod/apiserver-5b9785f765-qk9hl uid/d5f66519-ca7a-4808-94a0-b889552d411c container/fix-audit-permissions",
             "message": "constructed/pod-lifecycle-constructor reason/Ready",
             "tempSource": "PodState",
             "tempStructuredLocator": {
@@ -157,7 +157,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/openshift-apiserver namespace/openshift-apiserver pod/apiserver-5b9785f765-qk9hl uid/d5f66519-ca7a-4808-94a0-b889552d411c",
+            "locator": "namespace/openshift-apiserver pod/apiserver-5b9785f765-qk9hl uid/d5f66519-ca7a-4808-94a0-b889552d411c container/openshift-apiserver",
             "message": "constructed/pod-lifecycle-constructor reason/Ready",
             "tempSource": "PodState",
             "tempStructuredLocator": {
@@ -183,7 +183,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/openshift-apiserver-check-endpoints namespace/openshift-apiserver pod/apiserver-5b9785f765-qk9hl uid/d5f66519-ca7a-4808-94a0-b889552d411c",
+            "locator": "namespace/openshift-apiserver pod/apiserver-5b9785f765-qk9hl uid/d5f66519-ca7a-4808-94a0-b889552d411c container/openshift-apiserver-check-endpoints",
             "message": "constructed/pod-lifecycle-constructor reason/Ready",
             "tempSource": "PodState",
             "tempStructuredLocator": {

diff --git a/pkg/monitortests/node/watchpods/podTest/run-once-done/expected.json b/pkg/monitortests/node/watchpods/podTest/run-once-done/expected.json
@@ -53,7 +53,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/installer namespace/openshift-kube-scheduler pod/installer-3-ip-10-0-136-132.us-west-2.compute.internal uid/b7d89367-600a-49a3-95e1-a3ef2c91ecb9",
+            "locator": "namespace/openshift-kube-scheduler pod/installer-3-ip-10-0-136-132.us-west-2.compute.internal uid/b7d89367-600a-49a3-95e1-a3ef2c91ecb9 container/installer",
             "message": "constructed/pod-lifecycle-constructor reason/NotReady",
             "tempSource": "PodState",
             "tempStructuredLocator": {

diff --git a/pkg/monitortests/node/watchpods/podTest/simple/expected.json b/pkg/monitortests/node/watchpods/podTest/simple/expected.json
@@ -27,7 +27,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/openshift-apiserver-operator namespace/openshift-apiserver-operator pod/openshift-apiserver-operator-845779f5d-975gr uid/d9a5b0ba-6958-44aa-bc32-03d62944f973",
+            "locator": "namespace/openshift-apiserver-operator pod/openshift-apiserver-operator-845779f5d-975gr uid/d9a5b0ba-6958-44aa-bc32-03d62944f973 container/openshift-apiserver-operator",
             "message": "constructed/pod-lifecycle-constructor reason/NotReady missed real \"NotReady\"",
             "tempSource": "PodState",
             "tempStructuredLocator": {
@@ -53,7 +53,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/openshift-apiserver-operator namespace/openshift-apiserver-operator pod/openshift-apiserver-operator-845779f5d-975gr uid/d9a5b0ba-6958-44aa-bc32-03d62944f973",
+            "locator": "namespace/openshift-apiserver-operator pod/openshift-apiserver-operator-845779f5d-975gr uid/d9a5b0ba-6958-44aa-bc32-03d62944f973 container/openshift-apiserver-operator",
             "message": "constructed/pod-lifecycle-constructor reason/ContainerWait missed real \"ContainerWait\"",
             "tempSource": "PodState",
             "tempStructuredLocator": {
@@ -105,7 +105,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/openshift-apiserver-operator namespace/openshift-apiserver-operator pod/openshift-apiserver-operator-845779f5d-975gr uid/d9a5b0ba-6958-44aa-bc32-03d62944f973",
+            "locator": "namespace/openshift-apiserver-operator pod/openshift-apiserver-operator-845779f5d-975gr uid/d9a5b0ba-6958-44aa-bc32-03d62944f973 container/openshift-apiserver-operator",
             "message": "constructed/pod-lifecycle-constructor reason/Ready",
             "tempSource": "PodState",
             "tempStructuredLocator": {
@@ -131,7 +131,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/openshift-apiserver-operator namespace/openshift-apiserver-operator pod/openshift-apiserver-operator-845779f5d-975gr uid/d9a5b0ba-6958-44aa-bc32-03d62944f973",
+            "locator": "namespace/openshift-apiserver-operator pod/openshift-apiserver-operator-845779f5d-975gr uid/d9a5b0ba-6958-44aa-bc32-03d62944f973 container/openshift-apiserver-operator",
             "message": "constructed/pod-lifecycle-constructor reason/NotReady",
             "tempSource": "PodState",
             "tempStructuredLocator": {

diff --git a/pkg/monitortests/node/watchpods/podTest/trailing-ready-2/expected.json b/pkg/monitortests/node/watchpods/podTest/trailing-ready-2/expected.json
@@ -27,7 +27,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/machine-config-operator namespace/openshift-machine-config-operator pod/machine-config-operator-7d5bf78cff-bbbwb uid/27e57fd1-c8f9-4528-8a04-0054dad5d38f",
+            "locator": "namespace/openshift-machine-config-operator pod/machine-config-operator-7d5bf78cff-bbbwb uid/27e57fd1-c8f9-4528-8a04-0054dad5d38f container/machine-config-operator",
             "message": "constructed/pod-lifecycle-constructor reason/NotReady missed real \"NotReady\"",
             "tempSource": "PodState",
             "tempStructuredLocator": {
@@ -79,7 +79,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/machine-config-operator namespace/openshift-machine-config-operator pod/machine-config-operator-7d5bf78cff-bbbwb uid/27e57fd1-c8f9-4528-8a04-0054dad5d38f",
+            "locator": "namespace/openshift-machine-config-operator pod/machine-config-operator-7d5bf78cff-bbbwb uid/27e57fd1-c8f9-4528-8a04-0054dad5d38f container/machine-config-operator",
             "message": "constructed/pod-lifecycle-constructor reason/Ready",
             "tempSource": "PodState",
             "tempStructuredLocator": {

diff --git a/pkg/monitortests/node/watchpods/podTest/trailing-ready/expected.json b/pkg/monitortests/node/watchpods/podTest/trailing-ready/expected.json
@@ -27,7 +27,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/registry-server namespace/openshift-marketplace pod/community-operators-sp6lm uid/efb1885a-1fe1-4f5b-ad41-044e55f806a9",
+            "locator": "namespace/openshift-marketplace pod/community-operators-sp6lm uid/efb1885a-1fe1-4f5b-ad41-044e55f806a9 container/registry-server",
             "message": "constructed/pod-lifecycle-constructor reason/ContainerWait missed real \"ContainerWait\"",
             "tempSource": "PodState",
             "tempStructuredLocator": {
@@ -79,7 +79,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/registry-server namespace/openshift-marketplace pod/community-operators-sp6lm uid/efb1885a-1fe1-4f5b-ad41-044e55f806a9",
+            "locator": "namespace/openshift-marketplace pod/community-operators-sp6lm uid/efb1885a-1fe1-4f5b-ad41-044e55f806a9 container/registry-server",
             "message": "constructed/pod-lifecycle-constructor reason/NotReady missed real \"NotReady\"",
             "tempSource": "PodState",
             "tempStructuredLocator": {
@@ -105,7 +105,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/registry-server namespace/openshift-marketplace pod/community-operators-sp6lm uid/efb1885a-1fe1-4f5b-ad41-044e55f806a9",
+            "locator": "namespace/openshift-marketplace pod/community-operators-sp6lm uid/efb1885a-1fe1-4f5b-ad41-044e55f806a9 container/registry-server",
             "message": "cause/ constructed/pod-lifecycle-constructor duration/3.00s reason/ContainerStart",
             "tempSource": "PodState",
             "tempStructuredLocator": {
@@ -159,7 +159,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/registry-server namespace/openshift-marketplace pod/community-operators-sp6lm uid/efb1885a-1fe1-4f5b-ad41-044e55f806a9",
+            "locator": "namespace/openshift-marketplace pod/community-operators-sp6lm uid/efb1885a-1fe1-4f5b-ad41-044e55f806a9 container/registry-server",
             "message": "constructed/pod-lifecycle-constructor reason/Ready",
             "tempSource": "PodState",
             "tempStructuredLocator": {
@@ -185,7 +185,7 @@
         },
         {
             "level": "Info",
-            "locator": "container/registry-server namespace/openshift-marketplace pod/community-operators-sp6lm uid/efb1885a-1fe1-4f5b-ad41-044e55f806a9",
+            "locator": "namespace/openshift-marketplace pod/community-operators-sp6lm uid/efb1885a-1fe1-4f5b-ad41-044e55f806a9 container/registry-server",
             "message": "constructed/pod-lifecycle-constructor reason/NotReady",
             "tempSource": "PodState",
             "tempStructuredLocator": {