Repository: numaproj/numaflow
Pull request #2447 (2025-03-06 21:40:30)

chore: mark unhealthy if there is recent container restart
Explain what this PR does.

If there's container restart in the last 2 mins, mark it unhealthy.

Closes: #2446

<!--

Before you push your changes:

* Run `make pre-push -B` to fix codegen and lint problems.

Then, you MUST:

* Sign-off your commit (otherwise the DCO check will fail).
* Use [a conventional commit message](https://www.conventionalcommits.org/en/v1.0.0/) (otherwise the commit message check will fail).

If you did not do this, reset all your commit and replace them with a single commit:

```
git reset HEAD~1 ;# change 1 to how many commits you made
git commit --signoff -m 'feat: my feat. Fixes #1234'
```

When creating your PR: 

* Make sure that "Fixes #" or "Closes #" is in both the PR title (for release notes) and description (to automatically link and close the issue).
* Say how you tested your changes. If you changed the UI, attach screenshots.
* Set your PR as a draft initially.
* Your PR needs to pass the required checks before it can be approved. 
* Once required tests have passed, mark your PR "Ready for review".

If changes were requested, once you've made them, you MUST dismiss the review to get it reviewed again.

-->


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/numaproj/numaflow/pull/2447?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=numaproj) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 71.14%. Comparing base [(`9a223c0`)](https://app.codecov.io/gh/numaproj/numaflow/commit/9a223c0ecb692cdffab508e866841bdc4d57044d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=numaproj) to head [(`dc2e691`)](https://app.codecov.io/gh/numaproj/numaflow/commit/dc2e6910623ff3819329a2d6a29fa61cdb5bb91b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=numaproj).

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main    #2447      +/-   ##
==========================================
- Coverage   71.20%   71.14%   -0.06%     
==========================================
  Files         368      368              
  Lines       53611    53619       +8     
==========================================
- Hits        38172    38146      -26     
- Misses      14363    14394      +31     
- Partials     1076     1079       +3     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/numaproj/numaflow/pull/2447?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=numaproj).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=numaproj).

<details><summary>🚀 New features to boost your workflow: </summary>

- ❄ [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- 📦 [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
whynowy: Be aware this will mark the pipline/mvtx unhealthy for 2 mins if there's any container restart. Maybe we should we should make it shorter.

Patch:
diff --git a/pkg/reconciler/util.go b/pkg/reconciler/util.go
index 8edad6b1..c0225a60 100644
--- a/pkg/reconciler/util.go
+++ b/pkg/reconciler/util.go
@@ -19,6 +19,7 @@ package reconciler
 import (
 	"fmt"
 	"slices"
+	"time"
 
 	appv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -49,6 +50,7 @@ func CheckPodsStatus(pods *corev1.PodList) (healthy bool, reason string, message
 }
 
 func isPodHealthy(pod *corev1.Pod) (healthy bool, reason string) {
+	var lastRestartTime time.Time
 	for _, c := range pod.Status.ContainerStatuses {
 		if c.State.Waiting != nil && slices.Contains(unhealthyWaitingStatus, c.State.Waiting.Reason) {
 			return false, c.State.Waiting.Reason
@@ -56,6 +58,19 @@ func isPodHealthy(pod *corev1.Pod) (healthy bool, reason string) {
 		if c.State.Terminated != nil && c.State.Terminated.Reason == "Error" {
 			return false, c.State.Terminated.Reason
 		}
+		if x := c.LastTerminationState.Terminated; x != nil && !x.FinishedAt.Time.IsZero() {
+			if lastRestartTime.IsZero() || x.FinishedAt.Time.After(lastRestartTime) {
+				// Only check OOM or exit with Error
+				// TODO: revisit later if needed.
+				if x.ExitCode == 137 || (x.ExitCode == 143 && x.Reason == "Error") {
+					lastRestartTime = x.FinishedAt.Time
+				}
+			}
+		}
+	}
+	// Container restart happened in the last 2 mins
+	if !lastRestartTime.IsZero() && lastRestartTime.Add(2*time.Minute).After(time.Now()) {
+		return false, "RecentRestart"
 	}
 	return true, ""
 }
diff --git a/rust/Cargo.lock b/rust/Cargo.lock
index 8c3b7ef5..cc37b922 100644
--- a/rust/Cargo.lock
+++ b/rust/Cargo.lock
@@ -824,9 +824,9 @@ dependencies = [
 
 [[package]]
 name = "cc"
-version = "1.2.2"
+version = "1.2.16"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f34d93e62b03caf570cccc334cbc6c2fceca82f39211051345108adcba3eebdc"
+checksum = "be714c154be609ec7f5dad223a33bf1482fff90472de28f7362806e6d4832b8c"
 dependencies = [
  "jobserver",
  "libc",
@@ -2081,7 +2081,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "fc2f4eb4bc735547cfed7c0a4922cbd04a4655978c09b54f1f7b228750664c34"
 dependencies = [
  "cfg-if",
- "windows-targets 0.52.6",
+ "windows-targets 0.48.5",
 ]
 
 [[package]]
@@ -3177,15 +3177,14 @@ dependencies = [
 
 [[package]]
 name = "ring"
-version = "0.17.8"
+version = "0.17.13"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "c17fa4cb658e3583423e915b9f3acc01cceaee1860e33d59ebae66adc3a2dc0d"
+checksum = "70ac5d832aa16abd7d1def883a8545280c20a60f523a370aa3a9617c2b8550ee"
 dependencies = [
  "cc",
  "cfg-if",
  "getrandom",
  "libc",
- "spin",
  "untrusted",
  "windows-sys 0.52.0",
 ]
@@ -3687,12 +3686,6 @@ dependencies = [
  "windows-sys 0.52.0",
 ]
 
-[[package]]
-name = "spin"
-version = "0.9.8"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "6980e8d7511241f8acf4aebddbb1ff938df5eebe98691418c4468d0b72a96a67"
-
 [[package]]
 name = "spki"
 version = "0.7.3"


Test patch:
diff --git a/pkg/reconciler/util_test.go b/pkg/reconciler/util_test.go
index 72a09da1..e23841e1 100644
--- a/pkg/reconciler/util_test.go
+++ b/pkg/reconciler/util_test.go
@@ -18,6 +18,7 @@ package reconciler
 
 import (
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	appv1 "k8s.io/api/apps/v1"
@@ -67,6 +68,53 @@ func TestCheckVertexPodsStatus(t *testing.T) {
 		assert.Equal(t, "NoPodsFound", reason)
 		assert.True(t, done)
 	})
+
+	t.Run("Test Vertex status as true with non-recent restart", func(t *testing.T) {
+		pods := corev1.PodList{Items: []corev1.Pod{
+			{ObjectMeta: metav1.ObjectMeta{Name: "test-pod"}, Status: corev1.PodStatus{
+				ContainerStatuses: []corev1.ContainerStatus{
+					{
+						State: corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "Running"}},
+						LastTerminationState: corev1.ContainerState{
+							Terminated: &corev1.ContainerStateTerminated{
+								FinishedAt: metav1.Time{
+									Time: time.Now().Add(-3 * time.Minute),
+								},
+							},
+						},
+					},
+				}},
+			}},
+		}
+		done, reason, message := CheckPodsStatus(&pods)
+		assert.Equal(t, "All pods are healthy", message)
+		assert.Equal(t, "Running", reason)
+		assert.True(t, done)
+	})
+
+	t.Run("Test Vertex status as false with recent restart", func(t *testing.T) {
+		pods := corev1.PodList{Items: []corev1.Pod{
+			{ObjectMeta: metav1.ObjectMeta{Name: "test-pod"}, Status: corev1.PodStatus{
+				ContainerStatuses: []corev1.ContainerStatus{
+					{
+						State: corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "Running"}},
+						LastTerminationState: corev1.ContainerState{
+							Terminated: &corev1.ContainerStateTerminated{
+								FinishedAt: metav1.Time{
+									Time: time.Now().Add(-1 * time.Minute),
+								},
+								ExitCode: 137,
+							},
+						},
+					},
+				}},
+			}},
+		}
+		done, reason, message := CheckPodsStatus(&pods)
+		assert.Equal(t, "Pod test-pod is unhealthy", message)
+		assert.Equal(t, "PodRecentRestart", reason)
+		assert.False(t, done)
+	})
 }
 
 var (