feat: durable per-service Docker resource limits#95
Conversation
Declare optional mem_limit / mem_reservation / oom_score_adj per service in the YAML (the single source of truth) and thread them through the deploy chain to containers.run(), so earner containers keep durable cgroup limits across recreates instead of relying on out-of-band settings that revert. - services/_schema.yml: document the optional docker.resources block - 15 service YAMLs: right-sized mem_limit + oom_score_adj. Revenue/reputation -critical services (storj, mysterium, anyone-protocol) protected with negative oom_score_adj; expendable earners positive so the kernel sacrifices them first. - worker_api: ResourceSpec on DeploySpec + validation (mem size regex, oom_score_adj in [-1000,1000]); forwarded to deploy_raw. - orchestrator.deploy_raw: normalize resources and pass mem_limit / mem_reservation / oom_score_adj to containers.run() only when set (memswap left unset to avoid the cgroup-v2 swap-validation issue at create time). - main deploy route: forward YAML resources into the worker spec when present. - tests: spec->deploy_raw->containers.run flow + DeploySpec validation. Part of the 2026-07-05 fleet OOM remediation.
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds optional Docker container resource limits (mem_limit, mem_reservation, oom_score_adj) that flow from service YAML definitions through the deploy endpoint, worker API validation, and orchestrator container recreation, with schema documentation, per-service configuration, and new tests. ChangesDocker resource limits pipeline
Estimated code review effort: 3 (Moderate) | ~25 minutes Related issues: None specified. Related PRs: None specified. Suggested labels: enhancement, worker-api, orchestrator, config Suggested reviewers: GeiserX PoemA rabbit hops through YAML fields so neat, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #95 +/- ##
==========================================
+ Coverage 91.59% 92.09% +0.49%
==========================================
Files 30 30
Lines 3082 3085 +3
==========================================
+ Hits 2823 2841 +18
+ Misses 259 244 -15
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
tests/test_main_deploy_routes.py (1)
183-277: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTests are correct and consistent with
app/main.py's resources-forwarding logic.Minor DRY nit: the
captured = {}/_capture_deployboilerplate is now duplicated across four tests in this class (includingtest_deploy_repocket_emits_rp_env_keys). Consider extracting a small fixture/helper to reduce repetition.♻️ Optional fixture extraction
`@pytest.fixture` def capture_deploy(): captured: dict = {} async def _capture(worker_id, slug, spec): captured["spec"] = spec return {"container_id": "abc123"} return captured, _capture🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_main_deploy_routes.py` around lines 183 - 277, The deploy tests repeat the same captured-state and async deploy stub setup across multiple methods, including `_capture_deploy` and the `captured` dict. Extract that boilerplate into a small shared fixture or helper in the test class (for example, a capture fixture returning the dict and callback), then update the tests like `test_deploy_forwards_resources_from_yaml`, `test_deploy_omits_resources_when_absent`, and `test_deploy_storj_real_catalog_carries_resources` to use it consistently.app/orchestrator.py (1)
170-177: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant re-check of
Noneinresource_kwargs.
_normalize_resourcesalready dropsNonevalues (line 119), sores.get(key) is not Noneis always true for keys present inres. Harmless, just a minor readability nit.♻️ Simplify
res = _normalize_resources(resources) - resource_kwargs = { - key: res[key] for key in ("mem_limit", "mem_reservation", "oom_score_adj") if res.get(key) is not None - } + resource_kwargs = {key: res[key] for key in ("mem_limit", "mem_reservation", "oom_score_adj") if key in res}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/orchestrator.py` around lines 170 - 177, resource_kwargs in the orchestration resource setup is redundantly re-checking for None even though _normalize_resources already removes null values. Simplify the dict comprehension in the resource handling block to rely on the presence of keys from res (using the existing _normalize_resources helper and resource_kwargs construction) instead of testing res.get(key) is not None, while keeping the same mem_limit, mem_reservation, and oom_score_adj behavior.app/worker_api.py (2)
262-273: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
ResourceSpecsilently swallows typo'd/unknown keys.Pydantic v2 defaults
extra='ignore'. A typo in the service YAMLresourcesblock (e.g.oom_score_adjustinstead ofoom_score_adj) would pass validation silently, with the intended limit never applied and no error surfaced anywhere in the deploy pipeline.🛡️ Proposed fix: forbid unexpected keys
class ResourceSpec(BaseModel): """Optional Docker resource limits applied when the container is created. mem_limit / mem_reservation follow Docker's size syntax ("768m", "2g"); oom_score_adj biases the kernel OOM killer (-1000 = sacrificed last). """ + model_config = ConfigDict(extra="forbid") + mem_limit: str | None = None mem_reservation: str | None = None oom_score_adj: int | None = None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/worker_api.py` around lines 262 - 273, ResourceSpec currently accepts unknown fields silently because BaseModel defaults to ignoring extras, so typos in the service YAML resources block can be missed. Update the ResourceSpec model to reject unexpected keys by configuring Pydantic to forbid extra fields, and keep the validation behavior centered on ResourceSpec so deploy-time schema mistakes surface immediately instead of being ignored.
311-324: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winNo cross-field validation that
mem_reservation<mem_limit.Docker requires the soft limit to be lower than the hard limit; the daemon rejects (or behaves unexpectedly for) a reservation that meets or exceeds the limit. Since both fields pass independent regex checks here, a service YAML that sets
mem_reservation>=mem_limitwould clear worker validation but fail atcontainers.run(), surfacing as a generic 500 instead of the clearer 400 this validator is meant to provide. None of the current service YAMLs in this PR setmem_reservation, so this is latent rather than actively triggered.🛡️ Proposed fix
if resources.oom_score_adj is not None and not (-1000 <= resources.oom_score_adj <= 1000): raise HTTPException( status_code=400, detail=f"Invalid oom_score_adj '{resources.oom_score_adj}': must be between -1000 and 1000", ) + if ( + resources.mem_limit is not None + and resources.mem_reservation is not None + and _parse_mem(resources.mem_reservation) >= _parse_mem(resources.mem_limit) + ): + raise HTTPException( + status_code=400, + detail="mem_reservation must be lower than mem_limit", + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/worker_api.py` around lines 311 - 324, The _validate_resources validation only checks mem_limit and mem_reservation format independently, so add a cross-field check in _validate_resources to ensure mem_reservation is strictly lower than mem_limit before containers.run() is called. Use the existing ResourceSpec fields and the _MEM_LIMIT_RE parsing logic to compare the two values after validation, and raise HTTPException with a 400 status when the reservation is equal to or greater than the limit.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app/orchestrator.py`:
- Around line 170-177: resource_kwargs in the orchestration resource setup is
redundantly re-checking for None even though _normalize_resources already
removes null values. Simplify the dict comprehension in the resource handling
block to rely on the presence of keys from res (using the existing
_normalize_resources helper and resource_kwargs construction) instead of testing
res.get(key) is not None, while keeping the same mem_limit, mem_reservation, and
oom_score_adj behavior.
In `@app/worker_api.py`:
- Around line 262-273: ResourceSpec currently accepts unknown fields silently
because BaseModel defaults to ignoring extras, so typos in the service YAML
resources block can be missed. Update the ResourceSpec model to reject
unexpected keys by configuring Pydantic to forbid extra fields, and keep the
validation behavior centered on ResourceSpec so deploy-time schema mistakes
surface immediately instead of being ignored.
- Around line 311-324: The _validate_resources validation only checks mem_limit
and mem_reservation format independently, so add a cross-field check in
_validate_resources to ensure mem_reservation is strictly lower than mem_limit
before containers.run() is called. Use the existing ResourceSpec fields and the
_MEM_LIMIT_RE parsing logic to compare the two values after validation, and
raise HTTPException with a 400 status when the reservation is equal to or
greater than the limit.
In `@tests/test_main_deploy_routes.py`:
- Around line 183-277: The deploy tests repeat the same captured-state and async
deploy stub setup across multiple methods, including `_capture_deploy` and the
`captured` dict. Extract that boilerplate into a small shared fixture or helper
in the test class (for example, a capture fixture returning the dict and
callback), then update the tests like
`test_deploy_forwards_resources_from_yaml`,
`test_deploy_omits_resources_when_absent`, and
`test_deploy_storj_real_catalog_carries_resources` to use it consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f628b6b8-f050-4611-a3b6-6b9a7b2cd633
📒 Files selected for processing (21)
app/main.pyapp/orchestrator.pyapp/worker_api.pyservices/_schema.ymlservices/bandwidth/bitping.ymlservices/bandwidth/earnapp.ymlservices/bandwidth/earnfm.ymlservices/bandwidth/honeygain.ymlservices/bandwidth/iproyal.ymlservices/bandwidth/mysterium.ymlservices/bandwidth/packetstream.ymlservices/bandwidth/proxybase.ymlservices/bandwidth/proxylite.ymlservices/bandwidth/proxyrack.ymlservices/bandwidth/repocket.ymlservices/bandwidth/traffmonetizer.ymlservices/bandwidth/urnetwork.ymlservices/depin/anyone-protocol.ymlservices/storage/storj.ymltests/test_main_deploy_routes.pytests/test_worker_resources.py
Replace the duplicated captured / _capture_deploy boilerplate across the api_deploy tests with a shared `deploy_capture` pytest fixture. No behavior change. Addresses the CodeRabbit DRY nit on PR #95.
1 participant
What
Adds optional per-service Docker resource limits declared in the service YAML (the single source of truth) and threads them through the whole deploy chain down to
containers.run(). This makes currently-ephemeral cgroup limits durable across container recreates.Today the worker's
deploy_rawcallscontainers.run(...)with no memory limit oroom_score_adj, so earner containers have no durable limits — the production ones were set out-of-band and revert whenever a container is recreated. Part of the 2026-07-05 fleet OOM remediation.How it flows
services/<svc>.yml(docker.resources) → UI deploy route buildsspec["resources"]→ workerDeploySpec.resources(validated) →deploy_raw(resources=...)→containers.run(mem_limit=..., oom_score_adj=...).services/_schema.yml— documents the optionaldocker.resourcesblock (mem_limit, optionalmem_reservation,oom_score_adj).worker_api.py— newResourceSpecmodel onDeploySpec;_validate_deploy_specrejects a badmem_limit/mem_reservation(must match^\d+[bkmgBKMG]?$) or anoom_score_adjoutside[-1000, 1000]with a 400.orchestrator.deploy_raw— newresourcesparam, normalized to a dict;mem_limit/mem_reservation/oom_score_adjare passed tocontainers.run()only when set (Docker defaults preserved otherwise).memory-swapis deliberately left unset — at create timemem_limitalone avoids the cgroup-v2 swap-validation issue.main.py) — readsresourcesfrom the service YAML and includes it in the spec only when present.docker-py7.1.0 (pinned) supports all three kwargs directly on the high-levelcontainers.run()(they're inRUN_HOST_CONFIG_KWARGS/HostConfig).Right-sizing rationale
mem_limitis a ceiling, so it is safe on every host (the RAM-constrained host also has earlyoom as a backstop).oom_score_adjbiases the kernel OOM killer: negative = sacrificed last, positive = sacrificed first.mem_limitoom_score_adjRevenue/reputation-critical services are protected with a negative
oom_score_adj; expendable earners get a positive one so the kernel kills them first under pressure.Tests
tests/test_worker_resources.py(new):ResourceSpec/DeploySpecvalidation (valid + invalidmem_limit,mem_reservation,oom_score_adj),deploy_raw→containers.run()kwargs (pydantic + dict inputs,Nonedropped, omitted when unset), and the worker/deployendpoint threading resources through (incl. 400s on invalid input).tests/test_main_deploy_routes.py: UI route forwards YAMLresourcesinto the spec, omits when absent, and a real-catalogstorjguard against YAML key renames.ruff check .clean; fullpytestgreen (995 passed).Notes
app/; YAML stays the source of truth.compose_generator.py) is intentionally untouched — this PR scopes to the worker deploy path.Summary by CodeRabbit
New Features
Bug Fixes