Repository navigation
feat(deploy): scope default sandbox memory budgets to Helm - #154
Conversation
Enable the aggregate sandbox memory policy with a 16 GiB total reserve in the standalone, Helm, Aliyun and Huawei templates. Document the cgroup v2 and effective-memory requirements, explicit overrides and migration of existing sandbox subtrees. Keep the policy enabled in small CI and PVM fixtures with an explicit 2 GiB reserve, and verify that the parent hard limit matches resource reporting. Add configuration-generation and override regressions across deployment paths. The default is deployment policy, not a host OOM safety guarantee. Signed-off-by: wanghongtao.wangho <wanghongtao.wangho@antgroup.com>
Document the observed standalone ADX capacity-source mismatch and the cloud operating-system requirements before treating the new memory policy as a deployment-wide default. Signed-off-by: wanghongtao.wangho <wanghongtao.wangho@antgroup.com>
|
Reviewed The aggregate sandbox memory limit and a single deployment reserve are a reasonable design direction. The current default-on rollout should wait for the two P1 fixes below. These are also acknowledged in the PR description; documenting them as prerequisites does not resolve the production configuration.
Before a broad default rollout, also requalify the new enabled, finite-cap PVM fixture on the maintained matching host profile. The 16 GiB reserve still needs profile-specific sizing/qualification evidence; it is deployment policy rather than a measured universal host-OOM safety budget. Keeping unqualified profiles opt-in is a reasonable way to reduce the initial scope. The existing documentation already explains smaller-node overrides, cap preservation, draining before adding/lowering a cap, and the independent distill-fs budget; preserve those contracts and the canary/drain upgrade procedure. Independent validation on this exact head:
CI run 42009405813 has ten successful checks and an expected skipped publication job. Its merge commit and this head have identical source trees. The successful Standalone E2E uses the 2 GiB reserve, and the new memory assertion checks parent/socket equality, not public ADX scheduling capacity. Green CI therefore does not close R1 or qualify the 16 GiB default/PVM path. My independent checks used isolated source worktrees, the verified release CLI, rendered configurations and temporary-file fixtures. They did not start a live standalone/cloud deployment, modify host cgroups, run real PVM/default-storage workloads, or perform OOM-pressure testing. The PR author's ECS runtime measurements are separate evidence. Please land R1/R2, add the production-path regressions and remaining qualification evidence (or explicitly narrow the rollout), then rerun the affected checks and CI on the resulting head before merging. |
Resolve the bundled standalone ADX profile at image build time before replacing its complete capacity source with sandboxd. Preserve explicit custom sources and verify resolved standard, PVM, trace and shared-storage configurations. Keep unqualified cloud OS profiles opt-in, exposing enabled and reserve inputs through Terraform and guided configuration. Add public scheduling capacity and allocation-release E2E coverage alongside the parent/socket assertion. Signed-off-by: wanghongtao.wangho <wanghongtao.wangho@antgroup.com>
…ew-followup Signed-off-by: wanghongtao.wangho <wanghongtao.wangho@antgroup.com>
Match public whole-MiB memory reporting and exercise fractional CPU allocation in isolated qualification. Pass the measured parent cap to PVM tests and retain strict skipped-test checks. Keep the shared ACK fixture importable on Python 3.10, and include its current policy file and boolean input in the cloud fixtures. Signed-off-by: wanghongtao.wangho <wanghongtao.wangho@antgroup.com>
Document whole-MiB public memory reporting and the optional CPU budget used by isolated qualification. Retain byte-level parent and socket checks alongside public allocation and release assertions. Signed-off-by: wanghongtao.wangho <wanghongtao.wangho@antgroup.com>
|
@rcyw The fixes have been pushed to
Validation completed:
Remaining limitations: The fractional-CPU results used independent local sandboxd/ADX candidates and a qualification overlay. The component versions pinned by this PR do not yet include that protocol fix; the released pair still loses fractional CPU precision through the legacy integer-core field. Integrating a matching formal ADX release and sandboxd gitlink remains necessary, so this is not claimed as a fix delivered by the current PR. The full sandboxd Go suite also had one unmount-permission failure without The enabled finite-cap PVM fixture still requires runtime qualification on the maintained matching host profile. Production component peaks and OOM-pressure behavior have not been qualified; ordinary Linux runsc/script tests and zero OOM events in bounded tests do not establish those guarantees. Please re-review the R1/R2 implementation and regression coverage. The remaining integration and qualification work is explicitly retained as a limitation before merge or broader rollout. |
|
Follow-up review of
My independent regression exercised the pinned producer’s real refresh, memory ceiling and Unix HTTP socket with a fake capacity provider. The downstream values below follow the checked released ADX parser; this was not a full ADX/SDK deployment:
Memory was correct in all cases: Before merge, integrate the backward-compatible millicore producer/consumer fix, update the sandboxd gitlink and matching formal ADX/component archive pins, and verify public capacity plus allocation/release at 500m and 1500m using those actual delivered artifacts. The independent locally compiled candidates described in the PR establish useful evidence, but the current pins still lack that fix. The acceptance regression should exercise the new consumer’s actual precision-field selection and legacy fallback, not reinterpret the old integer-core field with different units.
Verified progress: R1 now resolves the complete bundled deployment with the packaged CLI at image-build time and replaces the whole tagged resource-source mapping. Standard/PVM × plain/telemetry/AFS configuration comparisons pass without unrelated resolved-configuration changes; explicit custom sources remain preserved. R2 now leaves both cloud Terraform profiles disabled by default, with explicit enabled/reserve inputs wired through guided configuration and templates; OS defaults are unchanged. Independent checks on this head:
The configuration suite initially lacked fixed ADX example files; after supplying the exact Reproduction against the currently pinned sandboxd and legacy released consumer contractIn a disposable checkout of sandboxd The two fractional cases fail and the integer control passes on the shipped pins. This reproduces the frozen legacy pair; after a protocol upgrade, verify the actual new consumer as described above. Source SHA-256: Please keep this PR unmerged until these integration/qualification requirements are satisfied and the resulting head’s CI is green. No approval or merge performed in this follow-up. |
Preserve the maintained PVM guide and its qualification boundaries. Retain the sandbox capacity source and document the deferred PVM memory policy qualification. Signed-off-by: wanghongtao.wangho <wanghongtao.wangho@antgroup.com>
Keep the aggregate parent policy disabled for PVM unless an operator explicitly enables it and qualifies the selected reserve. Retain the ordinary KVM defaults and keep PVM smoke tests free of unsupported budget qualification claims. Advance sandboxd to preserve millicores in its resource observations. The matching ADX consumer candidate is qualified through release overrides; its formal archive pins must be published before integration. Signed-off-by: wanghongtao.wangho <wanghongtao.wangho@antgroup.com>
Keep older custom PVM profiles valid when the disabled memory policy is omitted. Normalize supported quoted boolean keys while retaining strict validation for duplicate fields, invalid types and explicit opt-in. Signed-off-by: wanghongtao.wangho <wanghongtao.wangho@antgroup.com>
Leave standalone memory enforcement disabled and retain its existing auto capacity source. Defer the source switch and remove the independent CPU candidate from the parent dependency pins. Keep cloud and PVM profiles explicitly opt-in. Restore the ordinary standalone CI and SDK contracts, and verify with the packaged CLI that all maintained standalone variants preserve their original auto source. Signed-off-by: wanghongtao.wangho <wanghongtao.wangho@antgroup.com>
|
@rcyw Updated to
This removes the new standalone triggering paths behind R1/R3. It does not claim to fix upstream fractional-CPU handling; existing Helm sandboxd-source CPU behavior is unchanged. Verification: 186 focused ECS checks passed with zero skips. The released e611 CLI confirms that all six standard/PVM × plain/telemetry/shared-storage variants preserve the complete original
The initial exact-1500m assertion failed and was retained; the exact develop control reproduced the same result. This is unchanged released-ADX behavior in this environment, not a 1500m precision pass. At the observed schedulable budgets, both SDK create/exec/delete tests passed, with CPU and the 1024 MiB allocation fully restored after deletion. TLS verification remained enabled, the default ext4 loop filestore was retained, OOM counters stayed zero, and original host services were preserved. The ECS image was a qualification assembly using the unmodified released e611 payload/rootfs, freshly compiled fbe69 binaries and final configuration. Separately, CI on this head passed all ten checks, including the full production Dockerfile build and Standalone E2E; publication was skipped as expected. Please re-review the reduced scope. |
|
Follow-up review of The reduced rollout is reasonable: ordinary direct Helm retains its existing sandboxd resource source; standalone retains its original P2 — fix the PVM false/opt-out contract in both Helm and standalone before merging. The Helm guard and standalone guard/rewrite recognize only the literal Or, inside the existing Both inputs parse to exactly the same complete configuration as the canonical table. However, the guard sees zero matching memory fields, treats the policy as omitted and skips its rewrite. With PVM selected and the switch explicitly false, the effective output remains enabled=true, contrary to the documented override contract. I reproduced this through real Helm rendering and the actual standalone
The canonical/default controls pass. This defect is limited to custom PVM configurations; it can preserve finite parent enforcement even when the operator explicitly disables it. These are configuration results, not a live PVM/cgroup runtime reproduction. Before merge:
Independent validation retained:
Runnable configuration-only reproducer for the reviewed headRequires Python 3.12, PyYAML and Helm on PATH. Save as It asserts complete input TOML equivalence, uses the repository's real rendering/generator fixtures, prints the six effective values and exits 1 when any false switch leaves enforcement enabled. Please keep approval/merge on hold until the P2 and the resulting-head/latest-develop CI are resolved. |
Bring the native build and runtime rootfs contracts into the review base. Retain the component revisions selected by develop. Signed-off-by: wanghongtao.wangho <wanghongtao.wangho@antgroup.com>
Resolve Helm policies by their parsed keys so equivalent table forms cannot bypass an explicit opt-out. Verify standalone rewrites against the full parsed input and reject unsupported overrides before writing. Exercise both switch values through the deployment entry points and retain compatibility with omitted disabled policies and KVM. Signed-off-by: wanghongtao.wangho <wanghongtao.wangho@antgroup.com>
|
@rcyw I addressed P2 in
ECS validation passed all 93 standalone tests on Python 3.10 with Full CI passed all 11 jobs on this head, including the production image build and Standalone E2E, after retries on the same commit. CI run Could you re-review P2 on this head? |
rcyw
left a comment
There was a problem hiding this comment.
Approve at 47d8563687ee738e39875b3aeb922049eae330f2. The previous P2 is resolved in both entry points. No remaining blocking finding within the reduced rollout scope.
The design remains reasonable: ordinary direct Helm enables the existing aggregate cap with a 16 GiB reserve and retains its sandboxd capacity source. Standalone retains its original auto source, and standalone/cloud/PVM standard defaults keep enforcement disabled. This scope requires no new ADX release.
P2 verification
Helm now parses the effective TOML and applies the requested boolean. Standalone parses before rewriting, verifies the complete parsed result and preserves unrelated fields; unsupported rewrites are explicitly rejected before replacing the existing configuration. This satisfies the previously requested safe-rejection alternative.
Independent rerun of the original six cases, with the PVM switch explicitly false:
| Entry point | Canonical table | Quoted table | Inline table |
|---|---|---|---|
| Helm | effective enabled=false | effective enabled=false | effective enabled=false |
| Standalone | effective enabled=false | explicit rejection; previous file unchanged | explicit rejection; previous file unchanged |
The product regressions also cover true/false switches, escaped/dotted forms, omitted disabled policies, invalid/duplicate fields and KVM behavior. The new version prerequisites are documented.
Independent validation
- 198 local checks passed with zero skips: 93 standalone, 24 Helm memory/storage, 4 PVM static-profile and 77 build-helper tests; deployment syntax passed.
- The full 24-test Helm memory/storage suite also passed with official Helm 3.17.0.
- Eight native version/profile checks passed across core/umbrella × KVM/PVM: Helm 3.16.0 renders ordinary KVM and clearly rejects PVM with the required minimum version; Helm 3.17.0 renders both. Official Helm archives were checksum-verified.
- Local checks used Python 3.12. Separately, CI deployment logs confirm Python 3.10.22 + tomli 2.5.0 and all 93 standalone tests passing.
Final-head/latest-develop CI
CI 42554549954 is completed/success: 11 successful jobs, including Python 3.10–3.14, SDK quality/distributions, managed helper checks, deployment contracts, the production image build and Standalone/runsc E2E; image publication was skipped as expected.
I verified the checked-out merge 84bca6e4674cc8f01e113bfbbeba31e0ef858874 has parents latest develop f81d033765335260fa74d4c583a27ccd29bce452 and this head. Its full tree and the head tree are identical: f6fc2861ccb88631861b58d93ab6c7f1e9f73c31. This closes the prior requirement for CI on the latest-develop combination. SDK, component gitlinks and platform/Execd Dockerfile inputs have no net changes against that base.
Non-blocking follow-ups:
- Update the PR description's old final-source/CI references from bc3aabd/42197801592 to this head/run, while preserving the actual source attribution of older ECS runtime evidence.
- Retain the CI retry history: attempt 1 observed 147 rather than 0 in the SDK service-process thaw test; attempt 2 hit an Ubuntu archive connection failure while downloading tini. Final same-commit retries passed. The relevant SDK code is unchanged from develop; the SDK failure's root cause was not diagnosed in this review and deserves separate tracking.
Local checks establish configuration behavior, not real finite-cap PVM/OOM qualification. PVM enforcement remains disabled by default; qualification on the matching maintained host remains required before explicit enablement. The documented reserve-sizing, drain/canary and existing-cap preservation contracts continue to apply.
Enable the existing aggregate sandbox parent memory policy with a 16 GiB reserve in the ordinary direct Helm profile. Keep standalone, both cloud Terraform profiles and PVM disabled by default, with explicit operator configuration for qualified deployments.
Changes
plugin.resource.memory_limitwithtotal_reserve="16GiB". Its existing sandboxd capacity source is unchanged.enabled=falseand the bundled ADXautocapacity source. The default source conversion, enabled-memory CI fixture and SDK budget qualification added in earlier revisions have been removed.sandbox_memory_limit_enabled=false, expose the reserve and opt-in through Terraform/guided configuration, and retain their OS defaults.AKERNEL_PVM_MEMORY_LIMIT_ENABLED=false/ Helmnode.config.sandboxd.pvmMemoryLimitEnabled=false. Explicit opt-in preserves the selected reserve. Older valid custom profiles with an omitted memory table/field remain supported. Its qualification fixture keeps enforcement disabled and retains strict no-skip checks.724238c, preserving its maintained PVM guide and resolving the documentation conflict.A packaged-CLI regression verifies that standard/PVM × plain/telemetry/shared-storage preparation preserves the complete original
autoresource-source mapping.Validation
Final source:
bc3aabdd01a8be7176cbcd6e89f56827b707cef7.Final-head CI passed: ten successful jobs, including the complete production image build and Standalone E2E; image publication was skipped as expected.
ECS focused checks: 186 passed, zero skipped, covering standalone configuration/platform, PVM profile contracts, Helm storage/memory rendering, evaluated Terraform/cloud contracts, build helpers, real released ADX CLI configurations and the SDK runner.
The isolated runtime comparison used the released e611 ADX payload and unchanged matching runtime rootfs, freshly compiled fbe69 sandboxd/sbox/logger binaries and the final production configuration. This was a qualification assembly, not a complete node Dockerfile build.
Both SDK lifecycle checks created a sandbox with a 1024 MiB request, executed a command, deleted it and verified full allocation recovery. Memory followed 8192 → 7168 → 8192 MiB. TLS/hostname verification was enabled, the default loop-backed ext4 filestore was retained, the sandbox parent remained unlimited (
memory.max=max), and OOM counters remained zero. Owned runtime resources were cleaned up and all seven existing host services, original loop mapping and listeners were preserved.The initial exact-1500m precision assertion failed. The exact develop control reproduced the same 1000m result. This is an existing released ADX
autolimitation in this environment; the comparison establishes unchanged behavior, not a fractional-CPU fix. Existing sandboxd-source CPU precision limitations are also outside this PR's reduced scope.Rollout boundaries
Enabling the policy requires a qualified cgroup v2 node/image and effective memory above the selected reserve. Standalone operators must also select and qualify a compatible scheduling resource source before manually enabling a parent cap. PVM finite-cap runtime qualification remains separate; this PR defers that default enablement.
Adding or lowering a cap requires draining the sandbox subtree; an existing stricter cap is retained. Preserve canary rollout and rollback planning. Distill-fs keeps its independent limit. The reserve is deployment policy, not a measured universal host-OOM safety guarantee; production peaks and OOM-pressure qualification remain operator/profile-specific.