Repository navigation
Infra: add deterministic Render deploy pipeline and separate worker services - #172
Vishnu2707 merged 10 commits into
Conversation
e9d4222 to
1337177
Compare
|
CI note: all code, test, lint, frontend, SAST/SCA, secret-scan, SBOM, and validation checks are passing. The only failing check is This PR does not modify Recommended follow-up: handle the Trivy findings in a separate dependency/base-image security PR. |
ritiksah141
left a comment
There was a problem hiding this comment.
I reviewed PR #172 end to end against issue #157.
The direction is good: the fixed sleep is replaced by Render deploy polling, startup.sh no longer backgrounds the worker, staging and production services are defined, and the worker is modeled as a separate Render background worker. Most checks pass. I do think this needs changes before merge because the deploy controls still allow the wrong commit to reach the wrong environment, and the worker is not actually part of the deterministic deployment.
Findings:
-
.github/workflows/deploy.yml:67can deploy a non-main ref to production.The workflow is manual and lets the operator choose
environment, then deploys${{ github.sha }}to whichever Render service id matches that input. There is no guard tyingproductiontomainorstagingtodev. In GitHub Actions, a manually dispatched workflow can be run from a selected branch/ref, so someone can selectenvironment=productionwhile running the workflow fromdevor this PR branch and the script will send that SHA toRENDER_PRODUCTION_SERVICE_ID.That breaks the issue acceptance goal that production serves
mainand staging servesdev. Please add an early guard before triggering Render, for example:if [ "${{ inputs.environment }}" = "production" ] && [ "${{ github.ref_name }}" != "main" ]; then echo "ERROR: production deploys must be dispatched from main." exit 1 fi if [ "${{ inputs.environment }}" = "staging" ] && [ "${{ github.ref_name }}" != "dev" ]; then echo "ERROR: staging deploys must be dispatched from dev." exit 1 fi
It would also be worth setting the job
environment: ${{ inputs.environment }}so GitHub Environment protections can be applied later. -
render.yaml:65and.github/workflows/deploy.yml:72leave the worker outside the deterministic deploy.The workflow only accepts one service id per environment, and the body says those are the API services. The worker services in
render.yamluseautoDeployTrigger: commit, so they can deploy independently of the manual deploy workflow. That creates two problems:- The workflow can pass after deploying and smoke-testing only the web service while the worker is still on an older commit.
- Or the worker can auto-deploy a newer commit while the web service remains pinned to an older manually deployed commit.
Issue #157 is explicitly about deterministic deploys and moving the worker into its own service. To keep those goals together, the deploy workflow should deploy both the selected API service and its matching worker service to the same
GITHUB_SHA, then run the health/smoke gates. That means adding worker service id secrets, setting workerautoDeployTrigger: "off", and callingscripts/render_deploy.pyfor both selected services.
Other notes:
docs/deployment/render.mdsays there is no offline Render schema validator. Render now exposes a Blueprint validate API endpoint, so the docs can be updated to mention that as an optional maintainer validation path. This is not blocking the PR.- Current PR checks are green except
Container Scan (Trivy), which is the existing image vulnerability gate issue and not introduced by this PR.
Validation I ran locally:
python -m py_compile scripts/render_deploy.py
bash -n startup.sh
python - <<'PY'
import yaml
for path in ['render.yaml', '.github/workflows/deploy.yml']:
with open(path) as f:
yaml.safe_load(f)
PY
ruff check scripts/render_deploy.py
ruff format --check scripts/render_deploy.py
git diff --check dev...HEAD
pytest -qResults:
- Python compile, Bash syntax, YAML parse, Ruff, format check, and diff check passed.
- Full pytest result:
143 passed, 2 skipped, 1 failed. - The one pytest failure is the known local ONNX Runtime temp-dir failure in
tests/test_ai_hallucination_guard.py::TestHallucinationGuard::test_vector_store_purity, not related to this deployment PR.
m-khan-97
left a comment
There was a problem hiding this comment.
Reviewed end to end against #157. Agree with both of Ritik's blocking findings — the missing environment/ref guard (a manual dispatch can currently send main's SHA to staging or a feature branch's SHA to production) and the worker sitting outside the deterministic path (autoDeployTrigger: commit lets it drift to a different commit than the web service he just deployed). Both need to land before this merges; the whole point of #157 was that web and worker deploy together, deterministically.
A few additive notes from my own pass:
-
The stale-looking
startup.shdiff is not actually a problem. This PR branched before #164 (Alembic) merged, so the diff shows the olddb.init_db()Python block as unchanged context rather thanalembic upgrade head. I checkedmergeablevia the API and it's clean (MERGEABLE, no conflict) — since #164 and this PR touch non-overlapping regions of the file, git's merge will correctly combine both changes. Worth rebasing before merge anyway for a clean diff, but it won't produce the actual breakage that #161 has right now from the same kind of base-branch drift. -
scripts/render_deploy.pytreats any transient network blip as fatal._request()calls_fail()(hard exit) on the firstURLErrorduring polling. Over a ~30 min / ~120-poll window againstapi.render.com, a single hiccup fails the whole verification even though the actual Render deploy might be proceeding fine. Worth a small retry/backoff (e.g. 2-3 retries) on transient errors before giving up — low priority, not blocking. -
Verified the Trivy failure is genuinely pre-existing and unrelated to this PR — pulled the job logs directly. It's flagging real CVEs in the built image:
zlib1g(CRITICAL),util-linux, and on the Python sidetransformers4.57.6 has a CVE (2026-4372, RCE) fixed in 5.3.0, plusjaraco.contextandwheel. This PR doesn't touchrequirements.txtor the Dockerfile, so correctly out of scope here — but there's no issue yet actually tracking remediation of these specific findings (#156 set up the scanning, nobody's filed the fix-the-findings follow-up). I'll open one separately so it doesn't get lost. -
render.yaml's "web and worker must share identical
DATABASE_URL/JWT_SECRET, enforced by convention" is a real configuration-drift risk (no automated check that the two services in one environment actually agree) — related to but distinct from Ritik's point 2. Not blocking, but worth a follow-up once the worker is wired into the deterministic path.
ee40ed6 to
b5f5c7b
Compare
|
Addressed blocker 1 (branch/environment enforcement) in commit 7a69a1f. The workflow now runs scripts/render_deploy_preflight.py before either Render create step, requires staging from dev and production from main via github.ref_name, validates all selected deployment values and conditional smoke credentials, and sets the job GitHub Environment. tests/test_render_deploy_config.py proves valid mappings and that invalid branch or missing configuration paths fail before any POST; targeted deployment tests pass 42/42. |
|
Addressed blocker 2 (worker outside the deterministic path) in commit 7a69a1f. .github/workflows/deploy.yml creates both API and worker deployments at the identical github.sha, retains both exact deployment IDs, waits for each independently, and gates health/smoke on both succeeding. render.yaml now sets autoDeployTrigger to off for all four services. tests/test_render_deploy_config.py verifies same-SHA creation, both ID-specific waits, gate ordering, branches, service types/start commands, and disabled auto-deploy. Polling resilience was added separately in f7b83a0 and covered by tests/test_render_deploy.py. |
a5144f6 to
fcdfc35
Compare
|
@ritiksah141 Follow-up fixes are in 459fb1d (with functional changes in 8c0e1de). Local create mode now succeeds without GITHUB_OUTPUT and emits the stable deploy_id= line; GitHub Actions still receives the same output file value. The Blueprint now adds ALLOWED_ORIGINS only to the two web services, and the obsolete single-service Render guide is deprecated in favor of docs/deployment/render.md. New CLI and Blueprint coverage passes: 47 targeted tests passed. Full local suite: 190 passed, 2 skipped, 1 environmental chromadb failure unrelated to deployment changes. |
|
@m-khan-97 Follow-up fixes are in 459fb1d (with functional changes in 8c0e1de). Rollback documentation now correctly states that workflow_dispatch deploys only the current selected branch SHA; historical rollback uses the client or Render dashboard/API for both API and worker at the same known-good SHA. The guide also documents API-owned Alembic migrations, worker retry behavior during migration completion, and the need to monitor worker logs after schema changes. New CLI and Blueprint coverage passes: 47 targeted tests passed. Full local suite: 190 passed, 2 skipped, 1 environmental chromadb failure unrelated to deployment changes. |
Vishnu2707
left a comment
There was a problem hiding this comment.
The PR addresses all the fixes that are mentioned, hence approving!
|
@m-khan-97 @ritiksah141 , the review changes are addressed, please do approve the PR. |
1594913
Summary
Addresses #157.
This PR provides a manual, deterministic Render workflow that deploys API and
worker services to the same immutable GitHub SHA, captures and monitors both exact
deployment IDs, and gates health and optional smoke tests on both services being
live. The branch is rebased on current
devat3443a85and preservesalembic upgrade headin the API startup path.Deployment controls and review fixes
stagingcan be dispatched only fromdev;productiononly frommain.before any Render deployment POST.
github.sha; both IDs are independentlymonitored before health or smoke validation.
autoDeployTrigger: "off"so GitHub Actions isthe sole deployment controller.
with bounded 1/2/4-second backoff inside the overall deadline.
Follow-up corrections
python scripts/render_deploy.py createnow succeeds withoutGITHUB_OUTPUTand printsdeploy_id=<deployment-id>; in Actions it also writesthat value to
GITHUB_OUTPUT.historical manual rollback through the deployment client or Render dashboard/API.
ALLOWED_ORIGINSis declared withsync: falseon the two API services only.current guide.
Validation
tests/test_ai_hallucination_guard.py::TestHallucinationGuard::test_vector_store_purity, because the local Python 3.13 environment has no usablechromadbinstallation. No deployment test failed.Maintainer setup and limitations
Provision the four Render services and configure their service IDs, API URLs,
matching
DATABASE_URL/JWT_SECRET, Azure credentials, and API-specificALLOWED_ORIGINSvalues. Live Blueprint validation and deployment have not beenperformed. Worker operational health remains a separate live verification step.
The unrelated Trivy findings remain outside this PR and are tracked by #173. This
branch does not modify dependencies, Docker/base images, scanner rules, API
contracts, or frontend behavior.