Update from RunOnFlux/flux@3581b93

Author: RunOnFluxBot

services/appManagement/appsRuntimeState.js

@@ -20,6 +20,15 @@ const STABLE_RUN_MS = config.fluxapps.crashBackoffStableRunMs ?? 10 * 60 * 1000;
 // only the count (capped by the ladder) and the last timestamp are ever read,
 // so the persisted history never needs to grow beyond the ladder length
 const MAX_HISTORY = BACKOFF_DELAYS_MS.length;
+// The exit code cannot prove a fault: an image whose entrypoint is a wrapper
+// script ending in `exit 0` reports a clean stop for a segfault, and no init we
+// wrap around it can recover a status the image already discarded. So pacing on
+// the code alone would leave such a container restarting without limit. This is
+// the cause-blind backstop - this many automatic restarts inside the window is
+// itself evidence of a fault, whatever Docker reported, and it disposes into the
+// same ladder rather than into a state a human has to clear.
+const RESTART_BURST_COUNT = config.fluxapps.restartBurstCount ?? 5;
+const RESTART_BURST_WINDOW_MS = config.fluxapps.restartBurstWindowMs ?? 60 * 1000;
 
 function collection() {
   const db = dbHelper.databaseConnection();
@@ -93,7 +102,10 @@ async function setOperatorStopped(identifier, stopped) {
   // app. Swallowing a write failure would let the API report success while the
   // lock never persisted - the caller must surface the failure instead.
   const fields = { operatorStopped: stopped };
-  if (!stopped) fields.restartHistory = [];
+  if (!stopped) {
+    fields.restartHistory = [];
+    fields.autoRestartWindow = [];
+  }
   await setFields(identifier, fields);
 }
 
@@ -110,20 +122,39 @@ async function isOperatorStopped(identifier) {
 }
 
 /**
- * Appends a restart attempt (wall-clock) and trims the history to the ladder
- * length so a perpetually crashing container never grows the array unbounded.
+ * Whether the automatic restarts already recorded fill the burst window. Read
+ * BEFORE the current attempt is appended, so it answers "have there already
+ * been enough" and the caller's own restart is the one over the line.
+ *
+ * @param {object|null} state
+ * @returns {boolean}
+ */
+function burstExceeded(state) {
+  const recent = (state && state.autoRestartWindow) || [];
+  if (recent.length < RESTART_BURST_COUNT) return false;
+  return Date.now() - recent[0] <= RESTART_BURST_WINDOW_MS;
+}
+
+/**
+ * Appends a restart attempt (wall-clock). Every automatic restart lands in the
+ * burst window; only one with crash evidence - or one that fills the burst
+ * window, which is the same conclusion reached without the exit code - also
+ * walks the ladder. Both arrays are trimmed to their own bound so a
+ * perpetually restarting container never grows the document unbounded.
  *
  * @param {string} identifier
+ * @param {boolean} crashed - Docker reported a fault (non-zero exit or OOM kill)
  */
-async function recordRestart(identifier) {
+async function recordRestart(identifier, crashed = true) {
   try {
     const state = await getState(identifier);
-    const history = (state && state.restartHistory) || [];
-    history.push(Date.now());
-    if (history.length > MAX_HISTORY) {
-      history.splice(0, history.length - MAX_HISTORY);
+    const fields = {
+      autoRestartWindow: [...((state && state.autoRestartWindow) || []), Date.now()].slice(-RESTART_BURST_COUNT),
+    };
+    if (crashed || burstExceeded(state)) {
+      fields.restartHistory = [...((state && state.restartHistory) || []), Date.now()].slice(-MAX_HISTORY);
     }
-    await setFields(identifier, { restartHistory: history });
+    await setFields(identifier, fields);
   } catch (err) {
     log.error(`appsRuntimeState - failed to record restart for ${identifier}: ${err.message}`);
   }
@@ -150,10 +181,16 @@ async function recordRestart(identifier) {
  * @param {string} identifier
  * @param {number|null} lastFinishedAtMs - docker State.FinishedAt of the
  *        stopped container (ms epoch), when the caller has inspect data
+ * @param {boolean} crashed - Docker reported a fault (non-zero exit or OOM kill)
  * @returns {Promise<number>}
  */
-async function restartWaitMs(identifier, lastFinishedAtMs = null) {
+async function restartWaitMs(identifier, lastFinishedAtMs = null, crashed = true) {
   const state = await getState(identifier);
+  // A clean exit is the operator's own restart far more often than it is a
+  // fault, and pacing it makes a deliberate restart look like an outage. It
+  // goes back immediately - unless the restarts are arriving fast enough to
+  // fill the burst window, which is a fault however the exit code reads.
+  if (!crashed && !burstExceeded(state)) return 0;
   const history = (state && state.restartHistory) || [];
   if (history.length === 0) return 0;
 
@@ -331,6 +368,7 @@ async function prepareCollection() {
           networkHealRemoval: twins.some((t) => t.networkHealRemoval === true),
           networkHealHistory: [...new Set(twins.flatMap((t) => t.networkHealHistory || []))].sort((a, b) => a - b).slice(-MAX_HISTORY),
           restartHistory: [...new Set(twins.flatMap((t) => t.restartHistory || []))].sort((a, b) => a - b).slice(-MAX_HISTORY),
+          autoRestartWindow: [...new Set(twins.flatMap((t) => t.autoRestartWindow || []))].sort((a, b) => a - b).slice(-RESTART_BURST_COUNT),
           updatedAt: Math.max(...twins.map((t) => t.updatedAt || 0)),
         };
         const newestExit = twins.filter((t) => t.lastDiedAt !== undefined).sort((a, b) => b.lastDiedAt - a.lastDiedAt)[0];
@@ -368,4 +406,6 @@ module.exports = {
   BACKOFF_DELAYS_MS,
   STABLE_RUN_MS,
   MAX_HISTORY,
+  RESTART_BURST_COUNT,
+  RESTART_BURST_WINDOW_MS,
 };

services/appMonitoring/appReconciler.js

@@ -305,6 +305,9 @@ async function dockerActual(identifier) {
       exists: true,
       running: !!(info.State && info.State.Running),
       exitCode: everRan ? (info.State.ExitCode ?? null) : null,
+      // the kernel's verdict, not the entrypoint's: an image that swallows its
+      // payload's status still cannot hide this one
+      oomKilled: !!(info.State && info.State.OOMKilled),
       finishedAt,
       // classified from THIS inspect so the running-branch network check needs no
       // second docker call (and no TOCTOU between two inspects).
@@ -861,12 +864,25 @@ async function reconcile(rawIdentifier) {
     return;
   }
 
-  // exists but stopped, should run -> backoff-paced restart (no sleeping; the
-  // worker re-enqueues when the backoff window elapses)
-  const wait = await appsRuntimeState.restartWaitMs(identifier, actual.finishedAt);
+  // exists but stopped, should run -> restart, paced by the ladder only when the
+  // stop carries evidence of a fault (no sleeping; the worker re-enqueues when
+  // the backoff window elapses). A clean exit goes back immediately: it is an
+  // operator restarting their own app far more often than it is a crash, and
+  // pacing that turns a deliberate restart into what looks like an outage.
+  // exitCode null is a container that has never run - an initial start, not a death.
+  const crashed = !!actual.oomKilled || (actual.exitCode !== null && actual.exitCode !== 0);
+  const wait = await appsRuntimeState.restartWaitMs(identifier, actual.finishedAt, crashed);
   if (wait > 0) {
-    log.warn(`appReconciler - ${identifier} stopped, backing off ${Math.round(wait / 1000)}s before restart`);
-    fluxEventBus.publish('reconciler:actuated', { identifier, action: 'backoff', waitMs: wait });
+    // name which of the two put it here: a reported fault, or restarts arriving
+    // fast enough to be one whatever the exit code said. Support cannot tell
+    // these apart from the outside, and the difference decides what they do next.
+    const cause = crashed
+      ? `exit ${actual.exitCode}${actual.oomKilled ? ' (OOM-killed)' : ''}`
+      : 'restarting too fast to be healthy';
+    log.warn(`appReconciler - ${identifier} stopped, ${cause}; backing off ${Math.round(wait / 1000)}s before restart`);
+    fluxEventBus.publish('reconciler:actuated', {
+      identifier, action: 'backoff', waitMs: wait, crashed,
+    });
     scheduleRetry(identifier, wait);
     return;
   }
@@ -887,14 +903,18 @@ async function reconcile(rawIdentifier) {
     return;
   }
 
-  await appsRuntimeState.recordRestart(identifier);
+  await appsRuntimeState.recordRestart(identifier, crashed);
   try {
     await dockerService.appDockerStart(identifier);
   } catch (err) {
     // No die event fires for a failed start (the container never ran), so a
     // dropped throw here leaves the component down until the hourly sweep.
-    // Schedule our own retry; pacing is free - the attempt was recorded above,
-    // so a persistent failure walks the backoff ladder instead of hammering.
+    // Schedule our own retry. A start that never ran carries no exit code, so it
+    // is not a fault and does not walk the ladder directly - it reaches the
+    // ladder by filling the burst window, which these retries do comfortably
+    // (restartBurstCount x MANAGED_RETRY_MS against restartBurstWindowMs). That
+    // relationship is what bounds a permanently failing start, and the config
+    // comment on the window is where it is stated.
     log.error(`appReconciler - failed to start ${identifier}: ${err.message}; retrying`);
     fluxEventBus.publish('reconciler:actuated', { identifier, action: 'startFailed', reason: err.message });
     scheduleRetry(identifier, MANAGED_RETRY_MS);