mirror of
https://github.com/krkn-chaos/krkn.git
synced 2026-08-25 09:27:36 +00:00
fix: make test_recovery_timeout_fails deterministic (#1568)
* fix: make test_recovery_timeout_fails deterministic The test was flaky because it relied on a 1-second recovery timeout being too short for pods to recover. On fast Kind clusters, pod recovery can complete in under 1 second due to early-exit logic in the pod monitor. Instead, patch the deployment with a readiness probe that has a long initialDelaySeconds (120s), making it structurally impossible for replacement pods to become Ready within the 10-second recovery window. This eliminates the race condition entirely. Signed-off-by: ddjain <darjain@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> * fix: address review comments on flaky recovery timeout test - Use Recreate strategy so both pods roll out in parallel, avoiding sequential rollout timeout with default RollingUpdate strategy - Add back log-content assertion (expected_reasons=["unrecovered"]) to verify the failure is specifically a recovery-timeout failure Signed-off-by: ddjain <darjain@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> * fix: set rollingUpdate to None when switching to Recreate strategy Kubernetes rejects the patch if the existing rollingUpdate field conflicts with the Recreate strategy type. Signed-off-by: ddjain <darjain@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> * fix: reduce readiness delay and remove dead code - Lower READINESS_DELAY from 120s to 30s (only needs to exceed the 10s RECOVERY_TIMEOUT), cutting ~90s off CI runtime - Remove unused before_names variable Signed-off-by: ddjain <darjain@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> --------- Signed-off-by: ddjain <darjain@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -87,25 +87,51 @@ class TestPodErrorScenarios(BaseScenarioTest):
|
||||
result, ns, expected_reasons=["not enough pods match", "expected 100", "found only 2 pods"]
|
||||
)
|
||||
|
||||
def test_recovery_timeout_fails(self):
|
||||
def test_recovery_timeout_fails(self, wait_for_pods_running):
|
||||
"""Verify Krkn reports failure when pods cannot recover within the timeout.
|
||||
|
||||
Instead of relying on a very short timeout (which is racy on fast
|
||||
clusters), we add a readiness probe with a long initialDelaySeconds
|
||||
so replacement pods *structurally* cannot become Ready within the
|
||||
recovery window.
|
||||
"""
|
||||
ns = self.ns
|
||||
before = get_pods_list(self.k8s_core, ns, self.LABEL_SELECTOR)
|
||||
before_names = [p.metadata.name for p in before.items]
|
||||
READINESS_DELAY = 30
|
||||
RECOVERY_TIMEOUT = 10
|
||||
|
||||
# Patch deployment: use Recreate strategy so both pods roll out in
|
||||
# parallel, and add a readiness probe that won't pass for 30s.
|
||||
# READINESS_DELAY only needs to be greater than RECOVERY_TIMEOUT.
|
||||
patch_body = {
|
||||
"spec": {
|
||||
"strategy": {"type": "Recreate", "rollingUpdate": None},
|
||||
"template": {
|
||||
"spec": {
|
||||
"containers": [{
|
||||
"name": "app",
|
||||
"readinessProbe": {
|
||||
"httpGet": {"path": "/", "port": 80},
|
||||
"initialDelaySeconds": READINESS_DELAY,
|
||||
"periodSeconds": 5,
|
||||
},
|
||||
}]
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
self.k8s_apps.patch_namespaced_deployment(
|
||||
name="krkn-pod-error-target", namespace=ns, body=patch_body
|
||||
)
|
||||
wait_for_pods_running(ns, self.LABEL_SELECTOR, timeout=READINESS_DELAY + 30)
|
||||
|
||||
result = self.run_scenario(
|
||||
self.tmp_path, ns, overrides={"krkn_pod_recovery_time": 1}
|
||||
self.tmp_path, ns, overrides={"krkn_pod_recovery_time": RECOVERY_TIMEOUT}
|
||||
)
|
||||
assert_kraken_failure(result, context=f"namespace={ns}", tmp_path=self.tmp_path)
|
||||
|
||||
# Verify no hang and logs contain namespace and expected timeout/recovery errors
|
||||
|
||||
self.assert_failure_logs_contain(
|
||||
result, ns, expected_reasons=["timeout", "recover"]
|
||||
result, ns, expected_reasons=["unrecovered"]
|
||||
)
|
||||
|
||||
# Verify at least one target pod name is in the logs
|
||||
combined_lower = ((result.stdout or "") + "\n" + (result.stderr or "")).lower()
|
||||
found_pod = any(name.lower() in combined_lower for name in before_names)
|
||||
assert found_pod, f"None of the target pods {before_names} were found in failure logs"
|
||||
|
||||
@pytest.mark.no_workload
|
||||
def test_invalid_namespace_pattern_fails(self):
|
||||
|
||||
Reference in New Issue
Block a user