Fail the application when CONFIG_SVC config load fails instead of continuing #26

Activity

1. birme

   self-assigned this

   on Sep 30, 2026
2. birme

   added

   Human NeededRequires human interventionRequires human intervention

   on Oct 2, 2026
3. birme commented on Oct 2, 2026

   

   birme

   on Oct 2, 2026

   ContributorAuthor

   More actions

   Architect Triage

   Classification: NEEDS_HUMAN. This changes the failure mode of a shipped runtime (contract gate), the issue is not labelled agent-actionable, and two statements in the ticket are wrong for python-runner. I verified them by running the real config block (see below), so the decisions below differ from the web-runner sibling.

   Sibling: Eyevinn/web-runner#64 was triaged NEEDS_HUMAN with decisions A-D (Eyevinn/web-runner#64). I did not re-research those. This comment covers only what is specific to python-runner (541ab74).

   What is actually true in python-runner (verified)

   I extracted scripts/docker-entrypoint.sh:203-236 and ran it under set -e (the file's line 2) with stub npx, curl on PATH:

   config-to-env result	Ticket says	Actual behaviour today
   exit 0, valid export lines	loads them	loads them (unchanged)
   exit 0, empty stdout (empty store)	"silent, continues"	silent exit 1, no log line, never reaches pip install
   exit 0, stdout with no valid exports	"silent, continues"	silent exit 1, same
   non-zero exit or timeout 124	logs ERROR and continues	logs ERROR and continues (matches ticket)

   Cause: entrypoint:227 is valid_exports=$(echo "$config_env_output" | grep "^export ..."). When grep matches nothing it returns 1, the command substitution is a simple command inside the if body (not a condition), and set -e kills the script. The existing comment at :216-224 covers the && config_exit=0 || config_exit=$? form on :225 only, not this line.

   Consequences:

   1. For python-runner the ticket's third failure condition ("exit 0 but no valid exports") is already terminal today, but accidentally and with no [CONFIG] message. An app bound to an empty store (a normal flow: the create dialog lists any existing store, osaas-app CreateAppModal.tsx) already ends in a container restart loop, with nothing in the logs after [CONFIG] Loading environment variables .... So decision A (empty store) is not "keep today's behaviour" here. "Empty store is not a failure" would turn a silent crash loop into a started app. "Empty store is a failure" would keep the crash and only add the missing log line.
   2. The ticket's AC "/healthz reports failure (500)" cannot be met. scripts/ holds only docker-entrypoint.sh (no loading-server.js), and osaas-deploy-manager/src/apps/runner_health.ts:9 documents the same ("python-/wasm-runner have no loading server at all"). The file's own precedent for terminal build failures is a non-zero exit (comment at :261-269, setup.sh timeout from fix: wrap setup.sh escape hatch with 300s timeout to prevent indefinite build hang #23: exit 124 / exit $setup_exit, and a failed pip install via set -e). The ticket's "exit non-zero" half of the suggested shape matches that precedent; the "/healthz 500" half does not exist.
   3. What a non-zero exit looks like to the platform: osaas-deploy-manager/src/apps/python.ts:271-300 (checkBuildStatus) maps /healthz 200 to running, 404 to running, 500 to failed, 503 to building, anything else (including a connection error) to unknown. A crash-looping pod has no ready endpoint, so the ingress should answer 503, which maps to building. Unverified: that exact response on the live Elastx ingress for a crash-looping python-runner pod (no prod kubectl in this session). If true, a config failure shows up as a perpetual "building" in My Apps rather than "failed", and the only evidence is the container log.
   4. validateConfigServiceReachable (osaas-deploy-manager/src/apps/util.ts:127, called from python.ts:108) deliberately fails soft: an unknown or unreachable store is logged and creation proceeds (#1529). So a dead store binding can already be created, and after this change such an app fails on every boot.

   Contract gate (task 5.5a)

   Fatal-on-config-failure is a new failure mode on a shipped runtime: every existing python-runner app whose store is unreachable, expired-token or deleted starts failing on its next boot once a release carrying the fix is remade in prod. That is rejected as an autonomous change and needs birme's approval. The ticket is birme-authored, so the direction is clear, but the decisions below must be settled first.

   Decisions needed from birme (recommendations for python-runner)

   • A. Empty store and "no valid exports". Two coherent answers, pick one:
     • A1 (recommended): not a failure. Capture stdout separately from stderr, log [CONFIG] Config service '<name>' has no parameters and continue. Fixes the silent crash loop. Treat as failure only: non-zero exit, 124, or non-empty stdout with zero valid exports (garbled output, e.g. an npm banner or a key like my-key that the entrypoint's identifier grep drops).
     • A2: empty store is a failure (as the ticket's AC says). Then the only change for that case is a [CONFIG] ERROR: ... line before the exit, and apps bound to empty stores stay broken.
   • B. Mechanism. Recommend exit non-zero (keep the set -e precedent; use exit 124 for timeout and exit "$config_exit" otherwise, like setup.sh). Drop the /healthz 500 AC for python-runner. Porting loading-server.js into python-runner (Node is already in the image for the OSC CLI) would meet the /healthz AC and make deploy-manager show failed, but it is a separate multi-file feature (new script, Dockerfile entry, entrypoint wiring, tests) and should be its own ticket if wanted, not folded into this one.
   • C. Retry. With B as an exit, kubelet restarts give a natural retry (CrashLoopBackOff), so an in-script retry is optional. A bounded in-script retry (3 attempts, 3s backoff like the token refresh) would still help the common transient npm-registry blip (npx -y @osaas/cli@latest hits the registry on every boot) without waiting for a restart. Recommend skipping it here and keeping the change minimal.
   • D. CONFIG_SVC set but OSC_ACCESS_TOKEN empty. The whole block (:204) is skipped silently. Recommend leaving as is (README documents the token as required).

   Contract-preserving direction (for release once A-D are settled)

   If birme chooses A1 + B (exit) + C (none) + D (unchanged), this is SMALL: one file section plus one test file.

   • scripts/docker-entrypoint.sh, config block :203-236 only. Keep the comment line # Load environment variables from config service if configured verbatim and in place: tests/test-entrypoint-subpath.sh:51 greps for the phrase Load environment variables from config service to check ordering against the SUB_PATH block.
   • Replace config_env_output=$(... 2>&1) with separate stdout and stderr capture (2>"$err_file"). Make the grep on :227 safe under set -e (valid_exports=$(... | grep ...) || valid_exports="") and extend the existing explanatory comment to cover this line.
   • On timeout, non-zero exit, or non-empty stdout with zero valid exports: keep [CONFIG] ERROR: Failed to load config (exit N): <output> and the refresh-app-config hint for auth failures, then exit 124 / exit "$config_exit" before pip install.
   • Apps without CONFIG_SVC or without OSC_ACCESS_TOKEN are unaffected (outer if unchanged).
   • Rewritten AC (the originals presuppose a /healthz mechanism that does not exist here and treat the already-fatal empty case as "continues"):
     • With CONFIG_SVC + OSC_ACCESS_TOKEN set, timeout (124), non-zero exit, and garbled non-empty stdout each print a [CONFIG] ERROR line and exit non-zero before pip install.
     • Exit 0 with empty stdout prints the no-parameters line and continues to pip install (A1) instead of exiting silently.
     • Success path unchanged: valid exports are evaled and the Loaded N environment variable(s) line is printed.
     • Apps without CONFIG_SVC or without OSC_ACCESS_TOKEN behave exactly as today.
     • New tests/test-entrypoint-config-load-failure.sh, in the style of tests/test-entrypoint-setup-timeout.sh (extract the real block from the entrypoint and run it with stub npx, curl, timeout on PATH; do not re-implement the handler inside the test as web-runner does), covering timeout, non-zero exit, empty stdout, garbled stdout, success.
   • The ticket's last AC (a comment about set -e and x=$(cmd)) is already satisfied for :225 by the comment at :216-224; the work is extending it to :227.

   Existing test coverage

   • Repo: tests/ has 5 shell tests and none runs or asserts the config block. tests/test-entrypoint-subpath.sh:47-57 only checks that the config-load comment appears after the SUB_PATH block, which the change must preserve (see above).
   • e2e (osaas-e2e-tests, pulled today): tests/test-my-apps-runtimes.ts only asserts the runtime list (Python among the options, line 119). tests/test-ui-myapp-with-paramstore.ts, tests/test-mcp-myapps-paramstore.ts, tests/test-mcp-myapps-encrypted-params.ts bind a store but never select the Python runtime (no python or runtime token in them), so they exercise web-runner. tests/test-mcp-myapps-paramstore-large.ts runs config-to-env directly. None covers python-runner plus a config store, so none can act as a safety net for decisions A or B.

   E2E Impact

   Verdict: None
   Grepped tests/ and utils/ for python-runner|pythonrunner|python_runner|python.*runtime|runtime.*python|configService|CONFIG_SVC|config-to-env: 5 specs hit (listed above). pr-projects.mjs on those hits collapses to the default e2e chain, so there is no narrow project list. None asserts python-runner config-load behaviour, none would turn red from this change, and none would catch a regression of it. The failure paths are container shell logic and are covered at the right layer by the new behavioural test. An e2e would need a deleted store plus a pip install per run. No e2e sub-ticket is filed.

   Not verified

   • Unverified: how many existing prod python-runner apps are bound to an empty, dead or expired store, so the blast radius at remake time is unquantified. GRAFANA_API_KEY is set in this session but Grafana returns 401 (/api/org), so no Loki query was possible, and there is no prod kubectl here. A query for pods that log [CONFIG] Loading environment variables with no following Installing Python dependencies... in {namespace="eyevinn-python-runner"} would identify the already-silently-crashing empty-store apps.
   • Unverified: ingress response and deploy-manager buildStatus for a crash-looping python-runner pod on Elastx (point 3 above).
   • Rollout: a merged PR rebuilds the dev fork automatically. Prod needs a release plus remake by a human, which is the point where existing apps start failing.

   Add the agent-actionable label once A-D are decided and automation will pick it up. If you want to ship only the safe part first (the silent empty-store exit becomes a logged, continuing start, per A1), say so and I will split that out as its own ticket.

   Automated triage by architect agent
4. birme commented on Oct 4, 2026

   

   birme

   on Oct 4, 2026

   ContributorAuthor

   More actions

   birme approved the architect's recommendations on this ticket: A1 (an empty store is not a failure); B exit non-zero on config-load failure (the /healthz 500 requirement is dropped, python-runner has no loading server); C no retry; D unchanged. Implement per the architect triage; verify premises against current main first; add failing-path AND passing-path tests (empty store must still start). Moving to Backlog.
5. birme

   removed

   Human NeededRequires human interventionRequires human intervention

   on Oct 4, 2026
6. birme

   closed this as completedin #28on Oct 4, 2026
7. birme

   added

   Waiting for deployImplementation done, waiting for production deployImplementation done, waiting for production deploy

   on Oct 4, 2026
8. birme commented on Oct 24, 2026

   

   birme

   on Oct 24, 2026

   ContributorAuthor

   More actions

   This repository has no release tags, so a tagged release cannot be awaited; removing Waiting for deploy.
9. birme

   removed

   Waiting for deployImplementation done, waiting for production deployImplementation done, waiting for production deploy

   on Oct 24, 2026