⏱️ fix: Recycle Sandboxes Before Clock Drift (#39)

* fix: recycle sandboxes before clock drift

* test: strengthen clock healthcheck coverage

* fix: address clock healthcheck review findings

* fix: address clock-skew review findings

* fix: opt in to clock recycling from helm

* fix: bound clock healthcheck arithmetic

* fix: preserve sandbox workspace readiness

Author: danny-avila

.github/workflows/ci.yml

@@ -31,6 +31,9 @@ jobs:
       - name: Block-root package delivery
         run: tests/block_root_package_delivery.sh
 
+      - name: Sandbox-runner liveness checks
+        run: tests/sandbox_runner_healthcheck.sh
+
       - name: Validate sandbox Dockerfiles
         run: |
           docker buildx build --check -f api/Dockerfile .

docker/sandbox-runner-healthcheck.sh

@@ -2,16 +2,84 @@
 set -euo pipefail
 
 fd_limit="${SANDBOX_RUNNER_FD_LIVENESS_LIMIT:-40000}"
+clock_skew_limit_seconds="${SANDBOX_RUNNER_CLOCK_SKEW_LIVENESS_LIMIT_SECONDS:-0}"
+clock_skew_jitter_seconds="${SANDBOX_RUNNER_CLOCK_SKEW_LIVENESS_JITTER_SECONDS:-2}"
 timeout_seconds="${SANDBOX_RUNNER_HEALTHCHECK_TIMEOUT_SECONDS:-5}"
+manifest_clock_tolerance_seconds=30
 port="${PORT:-2000}"
-url="${SANDBOX_RUNNER_HEALTHCHECK_URL:-http://127.0.0.1:${port}/api/v2/runtimes}"
+url="${SANDBOX_RUNNER_HEALTHCHECK_URL:-http://127.0.0.1:${port}/api/v2/health}"
 
-case "$fd_limit" in
-    ''|*[!0-9]*)
-        echo "invalid SANDBOX_RUNNER_FD_LIVENESS_LIMIT: $fd_limit" >&2
+validate_non_negative_integer() {
+    local name="$1"
+    local value="$2"
+
+    case "$value" in
+        0|[1-9]|[1-9][0-9]*) ;;
+        *)
+            echo "invalid ${name}: ${value} (expected a canonical non-negative integer)" >&2
+            exit 2
+            ;;
+    esac
+}
+
+validate_non_negative_integer SANDBOX_RUNNER_FD_LIVENESS_LIMIT "$fd_limit"
+validate_non_negative_integer \
+    SANDBOX_RUNNER_CLOCK_SKEW_LIVENESS_LIMIT_SECONDS \
+    "$clock_skew_limit_seconds"
+validate_non_negative_integer \
+    SANDBOX_RUNNER_CLOCK_SKEW_LIVENESS_JITTER_SECONDS \
+    "$clock_skew_jitter_seconds"
+validate_non_negative_integer \
+    SANDBOX_RUNNER_HEALTHCHECK_TIMEOUT_SECONDS \
+    "$timeout_seconds"
+
+if [ "$timeout_seconds" -eq 0 ]; then
+    echo "SANDBOX_RUNNER_HEALTHCHECK_TIMEOUT_SECONDS must be greater than zero" >&2
+    exit 2
+fi
+
+if [ "$clock_skew_limit_seconds" -gt 0 ]; then
+    if [ "${#clock_skew_limit_seconds}" -gt 2 ] || \
+        [ "$clock_skew_limit_seconds" -ge "$manifest_clock_tolerance_seconds" ]; then
+        echo "SANDBOX_RUNNER_CLOCK_SKEW_LIVENESS_LIMIT_SECONDS plus SANDBOX_RUNNER_HEALTHCHECK_TIMEOUT_SECONDS must be less than the ${manifest_clock_tolerance_seconds}-second execution-manifest tolerance" >&2
         exit 2
-        ;;
-esac
+    fi
+
+    remaining_tolerance_seconds=$((manifest_clock_tolerance_seconds - clock_skew_limit_seconds))
+    if [ "${#timeout_seconds}" -gt 2 ] || \
+        [ "$timeout_seconds" -ge "$remaining_tolerance_seconds" ]; then
+        echo "SANDBOX_RUNNER_CLOCK_SKEW_LIVENESS_LIMIT_SECONDS plus SANDBOX_RUNNER_HEALTHCHECK_TIMEOUT_SECONDS must be less than the ${manifest_clock_tolerance_seconds}-second execution-manifest tolerance" >&2
+        exit 2
+    fi
+fi
+
+if [ "$clock_skew_limit_seconds" -gt 0 ] && ! command -v date >/dev/null 2>&1; then
+    echo "sandbox-runner clock-skew check requires date" >&2
+    exit 2
+fi
+
+effective_clock_skew_limit_seconds="$clock_skew_limit_seconds"
+if [ "$clock_skew_limit_seconds" -gt 0 ] && [ "$clock_skew_jitter_seconds" -gt 0 ]; then
+    if [ "${#clock_skew_jitter_seconds}" -gt 2 ] || \
+        [ "$clock_skew_jitter_seconds" -ge "$clock_skew_limit_seconds" ]; then
+        echo "SANDBOX_RUNNER_CLOCK_SKEW_LIVENESS_JITTER_SECONDS must be less than SANDBOX_RUNNER_CLOCK_SKEW_LIVENESS_LIMIT_SECONDS" >&2
+        exit 2
+    fi
+    if ! command -v cksum >/dev/null 2>&1; then
+        echo "sandbox-runner clock-skew jitter requires cksum" >&2
+        exit 2
+    fi
+
+    # Pods from one rollout have nearly identical ages and drift rates. Give
+    # each pod a stable threshold within the configured range to reduce the
+    # chance that same-age replicas restart together.
+    jitter_key="${HOSTNAME:-sandbox-runner}"
+    jitter_checksum=$(printf '%s' "$jitter_key" | cksum)
+    jitter_checksum="${jitter_checksum%% *}"
+    effective_clock_skew_limit_seconds=$((
+        clock_skew_limit_seconds - (jitter_checksum % (clock_skew_jitter_seconds + 1))
+    ))
+fi
 
 if [ "$fd_limit" -gt 0 ]; then
     fd_count=$(find /proc/1/fd -mindepth 1 -maxdepth 1 2>/dev/null | wc -l | tr -d '[:space:]')
@@ -21,4 +89,57 @@ if [ "$fd_limit" -gt 0 ]; then
     fi
 fi
 
-curl -fsS --max-time "$timeout_seconds" "$url" >/dev/null
+if [ "$clock_skew_limit_seconds" -eq 0 ]; then
+    curl -fsS --max-time "$timeout_seconds" "$url" >/dev/null
+    exit 0
+fi
+
+host_before_seconds=$(date -u +%s)
+response_headers=$(curl -fsS --max-time "$timeout_seconds" --dump-header - --output /dev/null "$url")
+host_after_seconds=$(date -u +%s)
+
+guest_date=$(printf '%s\n' "$response_headers" | awk '
+    tolower($1) == "date:" {
+        sub(/\r$/, "")
+        sub(/^[^:]*:[[:space:]]*/, "")
+        value = $0
+    }
+    END { print value }
+')
+
+if [ -z "$guest_date" ]; then
+    echo "sandbox-runner unhealthy: guest response is missing the HTTP Date header" >&2
+    exit 1
+fi
+
+http_date_pattern='^(Mon|Tue|Wed|Thu|Fri|Sat|Sun), [0-9]{2} (Jan|Feb|Mar|Apr|May|Jun|Jul|Aug|Sep|Oct|Nov|Dec) [0-9]{4} [0-9]{2}:[0-9]{2}:[0-9]{2} GMT$'
+if ! [[ "$guest_date" =~ $http_date_pattern ]]; then
+    echo "sandbox-runner unhealthy: guest returned an invalid HTTP Date header: ${guest_date}" >&2
+    exit 1
+fi
+
+if ! guest_seconds=$(date -u -d "$guest_date" +%s 2>/dev/null); then
+    echo "sandbox-runner unhealthy: guest returned an invalid HTTP Date header: ${guest_date}" >&2
+    exit 1
+fi
+
+# The response can take time to arrive, so compare guest time with the host
+# interval that enclosed the request instead of with a single sample. This
+# prevents probe latency from looking like backward clock drift.
+if [ "$guest_seconds" -lt "$host_before_seconds" ]; then
+    skew_seconds=$((guest_seconds - host_before_seconds))
+elif [ "$guest_seconds" -gt "$host_after_seconds" ]; then
+    skew_seconds=$((guest_seconds - host_after_seconds))
+else
+    skew_seconds=0
+fi
+
+absolute_skew_seconds="$skew_seconds"
+if [ "$absolute_skew_seconds" -lt 0 ]; then
+    absolute_skew_seconds=$((-absolute_skew_seconds))
+fi
+
+if [ "$absolute_skew_seconds" -ge "$effective_clock_skew_limit_seconds" ]; then
+    echo "sandbox-runner unhealthy: guest clock skew is ${skew_seconds}s, pod limit is ${effective_clock_skew_limit_seconds}s (configured maximum ${clock_skew_limit_seconds}s)" >&2
+    exit 1
+fi

helm/codeapi/templates/worker-sandbox-deployment.yaml

@@ -27,6 +27,22 @@ Split runtime deployments:
 {{- if and (not $packagesFromPvc) (not .Values.workerSandbox.kvmEnabled) }}
 {{- fail "workerSandbox.packages.source=image requires workerSandbox.kvmEnabled=true because the baked runner boots from a libkrun block root image" }}
 {{- end }}
+{{- $clockSkewLimitValue := toString .Values.workerSandbox.sandboxRunner.clockSkewLivenessLimitSeconds }}
+{{- $clockSkewJitterValue := toString .Values.workerSandbox.sandboxRunner.clockSkewLivenessJitterSeconds }}
+{{- if not (regexMatch "^(0|[1-9][0-9]*)$" $clockSkewLimitValue) }}
+{{- fail "workerSandbox.sandboxRunner.clockSkewLivenessLimitSeconds must be a non-negative integer" }}
+{{- end }}
+{{- if not (regexMatch "^(0|[1-9][0-9]*)$" $clockSkewJitterValue) }}
+{{- fail "workerSandbox.sandboxRunner.clockSkewLivenessJitterSeconds must be a non-negative integer" }}
+{{- end }}
+{{- $clockSkewLimit := int $clockSkewLimitValue }}
+{{- $clockSkewJitter := int $clockSkewJitterValue }}
+{{- if or (lt $clockSkewLimit 0) (ge $clockSkewLimit 25) }}
+{{- fail "workerSandbox.sandboxRunner.clockSkewLivenessLimitSeconds must be between 0 and 24 so the 5-second healthcheck timeout stays below the 30-second execution-manifest tolerance" }}
+{{- end }}
+{{- if or (lt $clockSkewJitter 0) (and (gt $clockSkewLimit 0) (ge $clockSkewJitter $clockSkewLimit)) }}
+{{- fail "workerSandbox.sandboxRunner.clockSkewLivenessJitterSeconds must be non-negative and less than clockSkewLivenessLimitSeconds when the clock-skew guard is enabled" }}
+{{- end }}
 {{- range .Values.workerSandbox.sandboxExtraEnv }}
 {{- $name := .name | default "" }}
 {{- if and $name (or (regexMatch "(?i)(SECRET|TOKEN|PASSWORD|KEY)" $name) (hasPrefix "REDIS_" $name) (hasPrefix "AWS_" $name) (hasPrefix "S3_" $name) (hasPrefix "MINIO_" $name) (hasPrefix "CODEAPI_" $name) (eq $name "FILE_SERVER_URL") (eq $name "TOOL_CALL_SERVER_URL")) }}
@@ -258,6 +274,10 @@ spec:
               value: {{ .Values.workerSandbox.launcher.filterVsockEnotconn | quote }}
             - name: SANDBOX_RUNNER_FD_LIVENESS_LIMIT
               value: {{ .Values.workerSandbox.sandboxRunner.fdLivenessLimit | quote }}
+            - name: SANDBOX_RUNNER_CLOCK_SKEW_LIVENESS_LIMIT_SECONDS
+              value: {{ $clockSkewLimit | quote }}
+            - name: SANDBOX_RUNNER_CLOCK_SKEW_LIVENESS_JITTER_SECONDS
+              value: {{ $clockSkewJitter | quote }}
             - name: EGRESS_GATEWAY_URL
               value: "http://{{ include "codeapi.fullname" . }}-egress-gateway:{{ .Values.egressGateway.service.port }}"
             - name: SANDBOX_LOG_LEVEL
@@ -362,14 +382,15 @@ spec:
                 - /usr/local/bin/sandbox-runner-healthcheck.sh
             initialDelaySeconds: 60
             periodSeconds: 30
-            timeoutSeconds: 5
+            timeoutSeconds: 7
           readinessProbe:
-            httpGet:
-              path: /api/v2/health
-              port: sandbox
+            exec:
+              command:
+                - /usr/local/bin/sandbox-runner-healthcheck.sh
             initialDelaySeconds: 30
             periodSeconds: 10
-            timeoutSeconds: 5
+            timeoutSeconds: 7
+            failureThreshold: 1
           resources:
             {{- $sandboxResources := deepCopy .Values.workerSandbox.resources }}
             {{- if $useKvmDevicePlugin }}

helm/codeapi/values.yaml

@@ -200,6 +200,15 @@ workerSandbox:
     # Restart a sandbox-runner before launcher/VMM host-side fd retention can
     # exhaust the process file descriptor limit. Set 0 to disable.
     fdLivenessLimit: 40000
+    # Restart a sandbox-runner when its microVM guest wall clock differs from
+    # the host by this many seconds. This must remain comfortably below the
+    # 30-second execution-manifest clock tolerance; the chart reserves the
+    # five-second probe timeout and accepts at most 24. Set 0 to disable.
+    clockSkewLivenessLimitSeconds: 10
+    # Subtract a stable 0..N-second offset from each pod's clock-skew limit so
+    # replicas created together do not all recycle at once. Must be less than
+    # clockSkewLivenessLimitSeconds.
+    clockSkewLivenessJitterSeconds: 2
     inheritSharedScheduling: true
     nodeSelector: {}
     tolerations: []

tests/block_root_package_delivery.sh

@@ -36,6 +36,23 @@ assert_not_contains() {
     fi
 }
 
+assert_env_value() {
+    local file="$1"
+    local name="$2"
+    local expected="$3"
+    local message="$4"
+    if ! awk -v name="$name" -v expected="$expected" '
+        $0 ~ "- name: " name {
+            getline
+            if ($0 ~ "value: \\\"" expected "\\\"") found = 1
+        }
+        END { exit found ? 0 : 1 }
+    ' "$file"; then
+        echo "$message" >&2
+        exit 1
+    fi
+}
+
 assert_compose_mode() {
     local compose_file="$1"
     local service="$2"
@@ -196,6 +213,25 @@ assert_not_contains \
     "$TMP_DIR/helm-image.yaml" \
     'app.kubernetes.io/component: package-init' \
     "default Helm render must not create package-init"
+assert_env_value \
+    "$TMP_DIR/helm-image.yaml" \
+    SANDBOX_RUNNER_CLOCK_SKEW_LIVENESS_LIMIT_SECONDS \
+    10 \
+    "default Helm render must keep clock skew below the manifest tolerance"
+assert_env_value \
+    "$TMP_DIR/helm-image.yaml" \
+    SANDBOX_RUNNER_CLOCK_SKEW_LIVENESS_JITTER_SECONDS \
+    2 \
+    "default Helm render must stagger clock-skew recycling across replicas"
+assert_contains \
+    "$TMP_DIR/helm-image.yaml" \
+    '^[[:space:]]+failureThreshold: 1$' \
+    "sandbox-runner readiness must fail immediately when clock skew is detected"
+
+if [ "$(grep -Fc -- '- /usr/local/bin/sandbox-runner-healthcheck.sh' "$TMP_DIR/helm-image.yaml")" -lt 2 ]; then
+    echo "sandbox-runner liveness and readiness must both check guest clock skew" >&2
+    exit 1
+fi
 
 helm template codeapi "$TMP_DIR/chart" \
     --set executionManifest.privateKey=test \
@@ -243,6 +279,74 @@ assert_contains \
     'value: "0"' \
     "fdLivenessLimit=0 must reach the healthcheck instead of reverting to 40000"
 
+helm template codeapi "$TMP_DIR/chart" \
+    --set executionManifest.privateKey=test \
+    --set executionManifest.publicKey=test \
+    --set workerSandbox.sandboxRunner.clockSkewLivenessLimitSeconds=0 \
+    > "$TMP_DIR/helm-no-clock-skew-liveness.yaml"
+
+assert_env_value \
+    "$TMP_DIR/helm-no-clock-skew-liveness.yaml" \
+    SANDBOX_RUNNER_CLOCK_SKEW_LIVENESS_LIMIT_SECONDS \
+    0 \
+    "clockSkewLivenessLimitSeconds=0 must reach the healthcheck instead of reverting to 10"
+
+if helm template codeapi "$TMP_DIR/chart" \
+    --set executionManifest.privateKey=test \
+    --set executionManifest.publicKey=test \
+    --set workerSandbox.sandboxRunner.clockSkewLivenessLimitSeconds=25 \
+    > "$TMP_DIR/invalid-clock-skew-limit.yaml" 2> "$TMP_DIR/invalid-clock-skew-limit.log"; then
+    echo "clock-skew liveness limit must stay below manifest tolerance" >&2
+    exit 1
+fi
+
+assert_contains \
+    "$TMP_DIR/invalid-clock-skew-limit.log" \
+    'clockSkewLivenessLimitSeconds must be between 0 and 24' \
+    "clock-skew limit validation failed for an unexpected reason"
+
+if helm template codeapi "$TMP_DIR/chart" \
+    --set executionManifest.privateKey=test \
+    --set executionManifest.publicKey=test \
+    --set workerSandbox.sandboxRunner.clockSkewLivenessLimitSeconds=29.5 \
+    > "$TMP_DIR/fractional-clock-skew-limit.yaml" 2> "$TMP_DIR/fractional-clock-skew-limit.log"; then
+    echo "clock-skew liveness limit must reject fractional values" >&2
+    exit 1
+fi
+
+assert_contains \
+    "$TMP_DIR/fractional-clock-skew-limit.log" \
+    'clockSkewLivenessLimitSeconds must be a non-negative integer' \
+    "fractional clock-skew limit validation failed for an unexpected reason"
+
+if helm template codeapi "$TMP_DIR/chart" \
+    --set executionManifest.privateKey=test \
+    --set executionManifest.publicKey=test \
+    --set workerSandbox.sandboxRunner.clockSkewLivenessJitterSeconds=10 \
+    > "$TMP_DIR/invalid-clock-skew-jitter.yaml" 2> "$TMP_DIR/invalid-clock-skew-jitter.log"; then
+    echo "clock-skew liveness jitter must stay below the configured limit" >&2
+    exit 1
+fi
+
+assert_contains \
+    "$TMP_DIR/invalid-clock-skew-jitter.log" \
+    'clockSkewLivenessJitterSeconds must be non-negative and less than clockSkewLivenessLimitSeconds' \
+    "clock-skew jitter validation failed for an unexpected reason"
+
+if helm template codeapi "$TMP_DIR/chart" \
+    --set executionManifest.privateKey=test \
+    --set executionManifest.publicKey=test \
+    --set workerSandbox.sandboxRunner.clockSkewLivenessJitterSeconds=1.5 \
+    > "$TMP_DIR/fractional-clock-skew-jitter.yaml" 2> "$TMP_DIR/fractional-clock-skew-jitter.log"; then
+    echo "clock-skew liveness jitter must reject fractional values" >&2
+    exit 1
+fi
+
+assert_contains \
+    "$TMP_DIR/fractional-clock-skew-jitter.log" \
+    'clockSkewLivenessJitterSeconds must be a non-negative integer' \
+    "fractional clock-skew jitter validation failed for an unexpected reason"
+
 if helm template codeapi "$TMP_DIR/chart" \
     --set executionManifest.privateKey=test \
     --set executionManifest.publicKey=test \