Fix bug when checking if a pod has exceeded its active deadline (#3226)

* Fix bug when checking if a pod has exceeded its active deadline The bug is that the CreationTimestamp isn't always set immediately, meaning it gets a value of 0 This in turn makes the currentRunTimeSeconds calculation massively wrong Now: - Use StartTime instead of CreationTimestamp, as kubernetes uses StartTime for its ActiveDeadline check - Return false if StartTime is not set or is 0, so we don't incorrectly calculate the run time using invalid values Signed-off-by: JamesMurkin <[email protected]> * Fix bad merge Signed-off-by: JamesMurkin <[email protected]> --------- Signed-off-by: JamesMurkin <[email protected]>
armadaproject · Feb 12, 2024 · e84e073 · e84e073
1 parent 911a7a6
commit e84e073
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 3 deletions.
diff --git a/internal/executor/service/pod_issue_handler.go b/internal/executor/service/pod_issue_handler.go
@@ -244,7 +244,14 @@ func (p *IssueHandler) hasExceededActiveDeadline(pod *v1.Pod) bool {
 	if pod.Spec.ActiveDeadlineSeconds == nil {
 		return false
 	}
-	currentRunTimeSeconds := time.Now().Sub(pod.CreationTimestamp.Time).Seconds()
+
+	// Using StartTime here, as kubernetes bases its activeDeadlineSeconds check on the StartTime also
+	startTime := pod.Status.StartTime
+	if startTime == nil || startTime.Time.IsZero() {
+		return false
+	}
+	currentRunTimeSeconds := time.Now().Sub(startTime.Time).Seconds()
+
 	podTerminationGracePeriodSeconds := float64(0)
 	if pod.Spec.TerminationGracePeriodSeconds != nil {
 		podTerminationGracePeriodSeconds = float64(*pod.Spec.TerminationGracePeriodSeconds)

diff --git a/internal/executor/service/pod_issue_handler_test.go b/internal/executor/service/pod_issue_handler_test.go
@@ -97,6 +97,11 @@ func TestPodIssueService_DeletesPodAndReportsFailed_IfExceedsActiveDeadline(t *t
 			// Created 10 mins ago, 5 min deadline, 10 minute grace period
 			pod: makePodWithDeadline(startTime, 300, 600),
 		},
+		"PodWithNoStartTime": {
+			expectIssueDetected: false,
+			// Created 10 mins ago, 5 min deadline, no start time
+			pod: makePodWithDeadline(time.Time{}, 300, 0),
+		},
 	}
 
 	for name, tc := range tests {
@@ -307,13 +312,14 @@ func getActivePods(t *testing.T, clusterContext context.ClusterContext) []*v1.Po
 	return remainingActivePods
 }
 
-func makePodWithDeadline(createdTime time.Time, deadlineSeconds, gracePeriodSeconds int) *v1.Pod {
+func makePodWithDeadline(startTime time.Time, deadlineSeconds, gracePeriodSeconds int) *v1.Pod {
 	pod := makeTestPod(v1.PodStatus{Phase: v1.PodRunning})
 	activeDeadlineSeconds := int64(deadlineSeconds)
 	pod.Spec.ActiveDeadlineSeconds = &activeDeadlineSeconds
 	terminationGracePeriodSeconds := int64(gracePeriodSeconds)
 	pod.Spec.TerminationGracePeriodSeconds = &terminationGracePeriodSeconds
-	pod.CreationTimestamp = metav1.NewTime(createdTime)
+	podStartTime := metav1.NewTime(startTime)
+	pod.Status.StartTime = &podStartTime
 	return pod
 }