Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 51 additions & 1 deletion src/scheduler/scheduler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,23 @@ async fn cleanup_deleted(pool: &SqlitePool, deleted: Vec<String>) {
/// across readiness checks so the most-cautious wins.
const DEFAULT_MIN_HEALTHY_TIME: Duration = Duration::from_secs(10);

/// Whether a deployment currently has a live container for the purpose of the
/// anti-flap window that refills `restart_count`.
///
/// A worker that reached `Running` obviously qualifies. So does one still in
/// `Creating` that already has an instance: a host-network worker can never pass
/// Ring's readiness checks (its container has no resolvable address), so it stays
/// `Creating` for life even though its container runs fine. Requiring `Running`
/// here left such a worker's `restart_count` monotonic, so its retry backoff
/// climbed to the 60s cap and never came back down — every redeploy got slower.
fn has_live_container(status: &DeploymentStatus, instances_empty: bool) -> bool {
match status {
DeploymentStatus::Running => true,
DeploymentStatus::Creating => !instances_empty,
_ => false,
}
}

/// Resolve the anti-flap window for a deployment: take the max of the
/// per-HC `min_healthy_time` (parsed via `HealthCheck::parse_duration`)
/// across readiness checks. Falls back to `DEFAULT_MIN_HEALTHY_TIME` when
Expand Down Expand Up @@ -1514,7 +1531,7 @@ pub(crate) async fn schedule(
if result.kind == "worker"
&& healthy_window.observe(
&result.id,
result.status == DeploymentStatus::Running,
has_live_container(&result.status, result.instances.is_empty()),
result.restart_count,
min_healthy_time_for(&result),
)
Expand Down Expand Up @@ -2129,4 +2146,37 @@ mod tests {
log_running_transition(&pool, &DeploymentStatus::Running, &d).await;
assert_eq!(count_state_transition_events(&pool, "trans-3").await, 0);
}

#[test]
fn live_container_running_is_healthy() {
// A worker that reached Running always counts as healthy-running.
assert!(has_live_container(&DeploymentStatus::Running, false));
assert!(has_live_container(&DeploymentStatus::Running, true));
}

#[test]
fn live_container_creating_with_instance_is_healthy() {
// The host-network case: stuck in Creating (readiness can't pass) but the
// container is actually up. This must count as healthy so restart_count
// can be forgiven and the retry backoff stops climbing.
assert!(has_live_container(&DeploymentStatus::Creating, false));
}

#[test]
fn live_container_creating_without_instance_is_not_healthy() {
// Creating with no instance yet: genuinely not running, no reset.
assert!(!has_live_container(&DeploymentStatus::Creating, true));
}

#[test]
fn live_container_other_states_are_not_healthy() {
for status in [
DeploymentStatus::Pending,
DeploymentStatus::Failed,
DeploymentStatus::Deleted,
] {
assert!(!has_live_container(&status, false));
assert!(!has_live_container(&status, true));
}
}
}
115 changes: 115 additions & 0 deletions tests/e2e/docker/t43_windowed_reset_stuck_creating.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
#!/usr/bin/env bash
# T43: windowed restart_count reset for a worker stuck in Creating with a live
# container (the host-network regression fixed by has_live_container).
#
# A host-network worker never passes Ring's readiness checks — its container has
# no resolvable address — so it stays in `Creating` for life even though the
# container runs fine. The old anti-flap gate required status == Running, so such
# a worker's restart_count stayed monotonic: its retry backoff climbed to the 60s
# cap and never came back down. The fix counts a Creating worker that already has
# a live instance as healthy, so the accrued count is forgiven.
#
# This reproduces that exact state end-to-end. Docker re-runs the SAME command on
# every (re)spawn, so the "crash a few times then stay up" behaviour is driven by
# a counter on a host bind mount: the first 3 starts exit 1 (each a crash Ring
# counts toward restart_count); the 4th stays up (sleep 3600). The worker uses
# host networking and a readiness probe that never turns green plus a long
# start_period, so once the container is alive it sits in `creating` — never
# `running`, never `failed` — with a live instance. That is the precise state
# where the old gate left restart_count monotonic.
#
# Invariants:
# 1. The crash phase registers: restart_count climbs to 3 (< MAX = 5), so a
# *windowed* reset is exercised rather than CrashLoopBackOff.
# 2. The worker then settles in `creating` (readiness can't pass on host net)
# and STAYS there — it must never reach `running` or `failed`.
# 3. After the anti-flap window elapses the scheduler forgives the accrued
# count: restart_count is reset to 0 while the status is still `creating`.

set -euo pipefail

SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
# shellcheck source=../lib.sh
source "$SCRIPT_DIR/../lib.sh"

log "== T43: restart_count is forgiven for a live worker stuck in Creating =="

NS="ring-e2e"
NAME="crash-then-stuck-creating"

# Host-side state directory the container increments a counter in. Start clean so
# the counter begins at 0 even across re-runs of this test.
STATE_DIR="/tmp/ring-e2e-t43"
rm -rf "$STATE_DIR"
mkdir -p "$STATE_DIR"

start_ring
ring_login

"$RING_BIN" apply --file "$SCRIPT_DIR/../fixtures/crash-then-stuck-creating.yaml"

# Invariant 1: the container crashes 3 times. Wait for restart_count to reach 3,
# which proves the crash phase registered. It must stop at 3 (the 4th start stays
# up), well short of MAX_RESTART_COUNT (5) — otherwise we couldn't test a
# *windowed* reset, only a CrashLoopBackOff.
log "waiting for the crash phase to accrue restart_count=3..."
REACHED=0
for _ in $(seq 1 60); do
RC=$(get_restart_count "$NS" "$NAME")
if [ "${RC:-0}" -ge 3 ]; then
REACHED=1
break
fi
sleep 1
done
if [ "$REACHED" -ne 1 ]; then
fail "restart_count never reached 3 during the crash phase (last: ${RC:-0})"
fi
log "crash phase registered: restart_count=$RC"

if [ "${RC:-0}" -ge 5 ]; then
fail "restart_count reached the CrashLoopBackOff bound ($RC); cannot test the windowed reset"
fi

# Invariant 2: the 4th container stays up but can never pass readiness on host
# networking, so it settles in `creating`. Wait for that, then confirm it STAYS
# there past the anti-flap window (DEFAULT_MIN_HEALTHY_TIME = 10s) — it must
# never be promoted to `running` nor failed by the rollout deadline (deferred by
# the 300s start_period).
wait_deployment_status "$NS" "$NAME" "creating" 60

log "worker is 'creating' with a live container; watching 40s that it stays there..."
for i in $(seq 1 40); do
STATUS=$("$RING_BIN" deployment list --output json 2>/dev/null \
| jq -r --arg ns "$NS" --arg n "$NAME" \
'.[] | select(.namespace==$ns and .name==$n) | .status' | head -n1)
case "$STATUS" in
creating) ;;
running) fail "worker reached 'running' at second $i — host-network readiness should never pass" ;;
failed) fail "worker reached 'failed' at second $i — start_period should defer the rollout deadline" ;;
*) fail "worker in unexpected status '$STATUS' at second $i" ;;
esac
sleep 1
done
log "Invariant 2: PASS (worker held 'creating' with a live container past the window)"

# Invariant 3: the crash budget must have refilled even though the worker never
# reached `running`. This is the regression: with status == Running gate, the
# count stayed at 3 forever; with has_live_container it is forgiven to 0.
RESTART_AFTER_WINDOW=$(get_restart_count "$NS" "$NAME")
STATUS=$("$RING_BIN" deployment list --output json \
| jq -r --arg ns "$NS" --arg n "$NAME" \
'.[] | select(.namespace==$ns and .name==$n) | .status' \
| head -n1)

log "observed: restart_count_after_window=$RESTART_AFTER_WINDOW status=$STATUS"

if [ "$STATUS" != "creating" ]; then
fail "expected status still 'creating' after the window, got '$STATUS'"
fi

if [ "${RESTART_AFTER_WINDOW:-99}" -ne 0 ]; then
fail "expected restart_count reset to 0 after the healthy window, got $RESTART_AFTER_WINDOW (has_live_container did not forgive the Creating worker)"
fi

log "== T43: PASS =="
43 changes: 43 additions & 0 deletions tests/e2e/fixtures/crash-then-stuck-creating.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
deployments:
crash-then-stuck-creating:
name: crash-then-stuck-creating
namespace: ring-e2e
runtime: docker
# Host-network windowed restart_count reset (the regression fixed by
# has_live_container). Docker re-runs the SAME command on every (re)spawn, so
# the "crash a few times then stay up" behaviour is driven by state on a host
# bind mount: each start increments a counter in /state/count. While count
# <= 3 the container exits 1 (a crash Ring counts toward restart_count); on
# the 4th start it stops crashing and sleeps forever.
#
# Unlike crash-then-heal, this worker uses host networking and a readiness
# probe that never turns green (the file it checks for is never created). A
# host-network container has no resolvable address, so it can never pass
# readiness and stays in `Creating` for life even though its container is
# alive. A long start_period keeps the rollout deadline from failing it, so
# it sits in `Creating` with a live instance — the exact state where the old
# `status == Running` gate left restart_count monotonic. With the fix the
# scheduler recognises the live container and forgives the accrued count.
network:
mode: host
image: alpine:3.19
command:
- "sh"
- "-c"
- 'n=$(cat /state/count 2>/dev/null || echo 0); n=$((n+1)); echo $n > /state/count; if [ "$n" -le 3 ]; then echo "crash #$n" >&2; exit 1; fi; echo "healthy after $n starts" >&2; exec sleep 3600'
replicas: 1
health_checks:
- type: command
command: test -f /var/run/kemeter/never-ready
interval: 2s
timeout: 1s
threshold: 3
on_failure: alert
readiness: true
start_period: 300s
volumes:
- type: bind
source: /tmp/ring-e2e-t43
destination: /state
driver: local
permission: rw