Refactor function validating pod revisions being in-sync

humio · Oct 23, 2024 · eb746d6 · eb746d6
1 parent 8a3bea7
commit eb746d6
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 28 deletions.
diff --git a/controllers/humiocluster_controller.go b/controllers/humiocluster_controller.go
@@ -335,7 +335,7 @@ func (r *HumioClusterReconciler) nodePoolPodsReady(ctx context.Context, hc *humi
 		r.Log.Info(fmt.Sprintf("cluster state is %s. waitingOnPods=%v, "+
 			"revisionsInSync=%v, podRevisions=%v, podDeletionTimestampSet=%v, podNames=%v, expectedRunningPods=%v, "+
 			"podsReady=%v, podsNotReady=%v",
-			hc.Status.State, podsStatus.waitingOnPods(), podsStatus.podRevisionCountMatchesNodeCountAndAllPodsHaveTheSameRevision(),
+			hc.Status.State, podsStatus.waitingOnPods(), podsStatus.podRevisionCountMatchesNodeCountAndAllPodsHaveRevision(hnp.GetDesiredPodRevision()),
 			podsStatus.podRevisions, podsStatus.podDeletionTimestampSet, podsStatus.podNames,
 			podsStatus.nodeCount, podsStatus.readyCount, podsStatus.notReadyCount))
 		return false, nil
@@ -1838,7 +1838,7 @@ func (r *HumioClusterReconciler) ensureMismatchedPodsAreDeleted(ctx context.Cont
 	// dump the current state of things
 	r.Log.Info(fmt.Sprintf("cluster state is %s. waitingOnPods=%v, ADifferenceWasDetectedAndManualDeletionsNotEnabled=%v, "+
 		"revisionsInSync=%v, podRevisions=%v, podDeletionTimestampSet=%v, podNames=%v, podHumioVersions=%v, expectedRunningPods=%v, podsReady=%v, podsNotReady=%v nodePoolStatus=%v",
-		hc.Status.State, podsStatus.waitingOnPods(), desiredLifecycleState.ADifferenceWasDetectedAndManualDeletionsNotEnabled(), podsStatus.podRevisionCountMatchesNodeCountAndAllPodsHaveTheSameRevision(),
+		hc.Status.State, podsStatus.waitingOnPods(), desiredLifecycleState.ADifferenceWasDetectedAndManualDeletionsNotEnabled(), podsStatus.podRevisionCountMatchesNodeCountAndAllPodsHaveRevision(hnp.GetDesiredPodRevision()),
 		podsStatus.podRevisions, podsStatus.podDeletionTimestampSet, podsStatus.podNames, podsStatus.podImageVersions, podsStatus.nodeCount, podsStatus.readyCount, podsStatus.notReadyCount, hc.Status.NodePoolStatus))
 
 	// when we detect changes, update status to reflect Upgrading/Restarting
@@ -1863,7 +1863,7 @@ func (r *HumioClusterReconciler) ensureMismatchedPodsAreDeleted(ctx context.Cont
 
 	// when no more changes are needed, update state to Running
 	if hnp.GetState() != humiov1alpha1.HumioClusterStateRunning &&
-		podsStatus.podRevisionCountMatchesNodeCountAndAllPodsHaveTheSameRevision() &&
+		podsStatus.podRevisionCountMatchesNodeCountAndAllPodsHaveRevision(hnp.GetDesiredPodRevision()) &&
 		!podsStatus.waitingOnPods() &&
 		!desiredLifecycleState.FoundConfigurationDifference() &&
 		!desiredLifecycleState.FoundVersionDifference() {

diff --git a/controllers/humiocluster_pod_status.go b/controllers/humiocluster_pod_status.go
@@ -192,30 +192,23 @@ func (s *podsStatusState) scaledMaxUnavailableMinusNotReadyDueToMinReadySeconds(
 	return max(deletionBudget, 0)
 }
 
-// podRevisionCountMatchesNodeCountAndAllPodsHaveTheSameRevision returns true if we have the correct number of pods
-// and all the pods have the same revision
-func (s *podsStatusState) podRevisionCountMatchesNodeCountAndAllPodsHaveTheSameRevision() bool {
-	// we don't have the correct number of revisions to check, so we have less pods than expected
+// podRevisionCountMatchesNodeCountAndAllPodsHaveRevision returns true if we have the correct number of pods
+// and all the pods have the same specified revision
+func (s *podsStatusState) podRevisionCountMatchesNodeCountAndAllPodsHaveRevision(podRevision int) bool {
+	// return early if number of revisions doesn't match nodeCount, this means we may have more or less pods than
+	// the target nodeCount
 	if len(s.podRevisions) != s.nodeCount {
 		return false
 	}
-	// if nodeCount says 1 and that we know we have one revision, it will always be in sync
-	if s.nodeCount == 1 {
-		return true
-	}
-	// if we have no revisions, return bool indicating if nodeCount matches that
-	if len(s.podRevisions) == 0 {
-		return s.nodeCount == 0
-	}
-	// fetch the first revision and compare the rest with that
-	revision := s.podRevisions[0]
-	for i := 1; i < len(s.podRevisions); i++ {
-		if s.podRevisions[i] != revision {
-			return false
+
+	numCorrectRevisionsFound := 0
+	for i := 0; i < len(s.podRevisions); i++ {
+		if s.podRevisions[i] == podRevision {
+			numCorrectRevisionsFound++
 		}
 	}
-	// no mismatching revisions found
-	return true
+
+	return numCorrectRevisionsFound == s.nodeCount
 }
 
 func (s *podsStatusState) haveUnschedulablePodsOrPodsWithBadStatusConditions() bool {

diff --git a/controllers/suite/clusters/humiocluster_controller_test.go b/controllers/suite/clusters/humiocluster_controller_test.go
@@ -369,12 +369,7 @@ var _ = Describe("HumioCluster Controller", func() {
 					}
 				}
 				return pendingPodsCount
-			}, testTimeout, suite.TestInterval).Should(Equal(1))
-
-			Eventually(func() string {
-				Expect(k8sClient.Get(ctx, key, &updatedHumioCluster)).Should(Succeed())
-				return updatedHumioCluster.Status.State
-			}, testTimeout, suite.TestInterval).Should(BeIdenticalTo(humiov1alpha1.HumioClusterStateRunning))
+			}, testTimeout, 250*time.Millisecond).Should(Equal(1))
 
 			suite.UsingClusterBy(key.Name, "Updating the cluster resources successfully with working affinity")
 			Eventually(func() error {