Repository navigation
Conversation
pimlock
added
gator:approval-needed
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
🌿 Preview your docs: https://nvidia-preview-pr-4320.docs.buildwithfern.com/openshell |
pimlock
removed
gator:approval-needed
pimlock
changed the title
b4bc6e9 to
0768489
Compare
This comment was marked as outdated.
This comment was marked as outdated.
b9972d5 to
f288455
Compare
This comment was marked as outdated.
This comment was marked as outdated.
|
/ok to test f288455 |
Supervisors poll the gateway for configuration every 10 seconds. Add an opt-in push path: with config_delivery_mode = "push", the gateway delivers sandbox configuration and provider environments over the existing ConnectSupervisor session, and supervisors apply and acknowledge them. Poll stays the default, and older supervisors and gateways keep polling. The gateway builds a streamed bootstrap that the supervisor applies before starting the workload, then runs a per-session delivery task that coalesces publications, takes FIFO build permits sized from the database pool, retries failed components with backoff, and reconciles periodically. Both components build from one shared input load, and the bootstrap checks that they agree on provider revision, attachments, and effective policy. In HA, replicas notify the session owner, which rebuilds from shared state. Builds also get cheaper in poll mode: interceptor provider-profile sources are served from a snapshot refreshed every 10 seconds, and provider environment builds read refresh states once and load providers concurrently. The supervisor applies polled and streamed configuration through one runtime, now in its own module, and stops polling once a push session is applying configuration. New metrics cover builds, deliveries, apply results, bootstraps, and peer notifications. Part of #1731. Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
961f154 to
cfe305e
Compare
- Log sandbox policy load results from both the polling RPC and stream acknowledgements, so push mode keeps the gateway log line poll mode had. - Warn on the gateway when a supervisor reports a failed or unsupported pushed configuration apply, including the failure code. - Emit an OCSF event when a pushed sandbox configuration errors during apply; polling surfaced this by ending its loop. - Record sandbox_id on the config_delivery.build span. - Add openshell_supervisor_config_apply_duration_seconds, the time from queuing an update on the session to its apply result, and document it with the previously undocumented policy apply results counter. Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
pimlock
left a comment
pimlock
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
This maintainer-authored PR is project-valid as the implementation of accepted issue #1731, and its documentation and E2E label coverage match the user-visible gateway and supervisor changes. The initial review found three blocking startup and HA state-transition defects.
Action required: @pimlock, please address the three inline findings and push an updated head for a focused follow-up review.
Blocking findings:
GATOR-3b8ca9a8-01: bootstrap admission can overtake a newer committed configurationGATOR-3b8ca9a8-02: runtime readiness can be sent to a superseded sessionGATOR-3b8ca9a8-03: a remote owner record is mistaken for runtime readiness
Carried findings:
- None
Non-blocking suggestions:
- None
Gator metadata
- Validation: Maintainer-authored implementation of accepted issue #1731; the superseded stage PRs are closed.
- Docs: Fern documentation covers gateway configuration, interceptor behavior, and metrics; no navigation change is needed for the existing pages.
- Checks: Pipeline handoff is deferred while blocking review findings remain.
- E2E:
test:e2eandtest:e2e-kubernetesare present and current-head workflow runs exist; results will be gated after review feedback is resolved. - Head SHA:
3b8ca9a802d55db014e2c8ea30c96db57c1d5aae - Base SHA:
d789ec613b829044e4580f6cda7f5d4648a31285 - Merge base SHA:
d789ec613b829044e4580f6cda7f5d4648a31285 - Patch ID:
2e40dd966db0b5d600e81d4c7a949b13c6355b8d - Gator payload:
11 - Review mode:
initial - Previous reviewed SHA: none
- Review budget exhausted: no
- Maintainer decision required: no
- Next state:
gator:in-review
pimlock
added
the
gator:in-review
Rename SupervisorHello.supports_config_apply to supports_config_push and SessionAccepted.config_apply_enabled to config_push_enabled. Polled configuration is applied through the same supervisor runtime, so apply no longer distinguishes the modes; the flag selects push delivery, matching config_delivery_mode = "push". Field numbers are unchanged. Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
pimlock
left a comment
pimlock
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Thanks @pimlock. I reviewed the new head and its capability-rename delta against the three open findings. The rename is internally consistent, but it does not change the startup or HA state transitions behind those findings, so all three remain open and no new finding was added.
Action required: @pimlock, please address the three existing inline findings and push an updated head for another focused follow-up review.
Blocking findings:
- No new blocking findings in this delta
Carried findings:
GATOR-3b8ca9a8-01: defer startup admission when a newer configuration generation is pendingGATOR-3b8ca9a8-02: deliver runtime readiness through the active replacement session after reconnectGATOR-3b8ca9a8-03: fence cross-replica readiness separately from owner-record freshness
Gator metadata
- Validation: Maintainer-authored implementation of accepted issue #1731; project validity is unchanged.
- Docs: Existing Fern documentation updates remain sufficient; the rename does not create a new navigation obligation.
- Checks: Current-head checks are running, but pipeline handoff remains deferred while carried review findings are open.
- E2E:
test:e2eandtest:e2e-kubernetesremain applied and current-head workflows are running. - Head SHA:
54d415fa0d93dbbad6ecd698390ee98db0d234f2 - Base SHA:
d789ec613b829044e4580f6cda7f5d4648a31285 - Merge base SHA:
d789ec613b829044e4580f6cda7f5d4648a31285 - Patch ID:
9c806ec8e195230d37eb4454adde464bd477c91b - Gator payload:
11 - Review mode:
follow_up - Previous reviewed SHA:
3b8ca9a802d55db014e2c8ea30c96db57c1d5aae - Review budget exhausted: no
- Maintainer decision required: no
- Next state:
gator:in-review
Withhold accepted startup admission when a newer configuration generation was committed before the workload started, as polling startup reports already do. The newer generation's own admission unlocks startup. Report runtime readiness through whichever supervisor session is current, so a reconnect between admission and workload activation still reaches the gateway. Treat a remote session owner as ready only once its supervisor instance has durably reported runtime readiness, both in driver reconciliation and when restoring state after a disconnect. Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
Maintainer Approval NeededThanks @pimlock. I checked your replies and the new code for startup admission, readiness after reconnect, and cross-replica readiness. The current generation is checked before startup acceptance, each active session reports readiness through its own stream, and remote readiness requires the matching durable supervisor instance. All three prior findings are fixed, and their Gator threads are now resolved. The focused follow-up review found no new blocking findings. Gator validation and PR monitoring are complete. Branch Checks, Helm Lint, Trivy Changes, DCO, and required E2E are green on the current head. Push-mode E2E, Kubernetes HA, and credential-driver E2E also passed. Action required: a human maintainer must review and approve this PR; the maintainer approval gate is still unmet. Gator metadata
|
pimlock
added
gator:approval-needed
Every push acknowledgement persisted its configuration admission, and each sandbox policy result, while holding the compute sandbox lock. User mutations such as UpdateConfig and provider attachment take the same FIFO lock, so a fanout acknowledged by 250 sessions queued about 500 writes ahead of them: a single global setting write took up to 5.7 seconds, and 20 consecutive writes took 54 seconds. Once a workload runs, its admission no longer gates startup. Results for running workloads are now acknowledged at once and persisted by one background writer, so a user write waits for at most one result write. Queued admissions for a sandbox collapse into the newest, policy results are all kept, admissions from replaced sessions are skipped, and failed writes are retried. Startup admission stays synchronous. Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
The provider environment reads global settings only for the global policy, so a change to any other global setting no longer rebuilds every session's provider environment and resolves its credentials. Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
Attach and detach send the resource version the command itself just read, so a concurrent status write, such as a supervisor configuration result, could fail the command with "sandbox was modified by another operation". Retry those conflicts with a fresh read, up to five times. Mutations whose save is uncertain still never retry. Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
pimlock
left a comment
pimlock
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
@pimlock, I checked your fixes for startup admission, reconnect readiness, and cross-replica readiness; all three prior findings remain resolved. The focused follow-up review checked the new background result writer, selective global-setting publication, and bounded provider attachment retries. It found one new recovery defect in the result writer.
Action required: @pimlock, preserve recovery of acknowledged results after persistence retries are exhausted, and push an updated head with focused recovery coverage.
Blocking findings:
GATOR-685e7007-01: dropped acknowledged results cannot be recovered by unchanged reconciliation.
Carried findings:
- None;
GATOR-3b8ca9a8-01,GATOR-3b8ca9a8-02, andGATOR-3b8ca9a8-03remain resolved.
Gator metadata
- Validation: Maintainer-authored implementation of accepted issue #1731; project scope remains unchanged.
- Docs: Existing Fern configuration, interceptor, and metrics updates cover the changes, including result-writer metrics.
- Checks: Branch Checks, Helm Lint, Trivy Changes, and DCO are green; required E2E is running. Pipeline handoff waits for review feedback to be addressed.
- E2E:
test:e2eandtest:e2e-kubernetesare applied; current-head Branch E2E Checks is in progress. Push-mode Docker E2E passed. - Head SHA:
685e700768443e560607803ce470d2201db4fca6 - Base SHA:
d789ec613b829044e4580f6cda7f5d4648a31285 - Merge base SHA:
d789ec613b829044e4580f6cda7f5d4648a31285 - Patch ID:
c729d64e20f70257673573893d9846676407a9ec - Gator payload:
11 - Review mode:
follow_up - Previous reviewed SHA:
54d415fa0d93dbbad6ecd698390ee98db0d234f2 - Review budget exhausted: no at cycle start; this is the third finding-bearing round under the ledger's accounting.
- Maintainer decision required: no convergence decision this cycle; author correction is required.
- Next state:
gator:in-review - Review method: Code-only independent review; no local tests were run.
| retry.policy_results.push(queued); | ||
| } else { | ||
| warn!(sandbox_id, version = result.version, error = %error, "failed to persist supervisor policy result"); | ||
| record_write("policy_result", "dropped"); |
There was a problem hiding this comment.
gator-agent
Warning — GATOR-685e7007-01 · Dropped acknowledged results cannot recover
Summary: When a policy update on a running push-mode supervisor succeeds but result persistence fails five times, this branch permanently discards the result. The session has already acknowledged its fingerprint, so periodic reconciliation suppresses the unchanged snapshot after the store recovers. Policy status can remain pending and policy set --wait times out despite successful application. The admission-drop branch has the same recovery gap.
Fix: Retain failed writes with capped backoff, or invalidate the matching acknowledged delivery state and republish on exhaustion. Preserve admission coalescing and stale-session/missing-revision handling.
Verify: Fail policy-result persistence exactly five times, restore writes, and reconcile the unchanged snapshot without reconnecting. Require loaded status and the updated current policy version. Cover admission recovery through the same failure sequence.
Agent context
- Agent path: policy update → matching supervisor result → queued persistence → retry exhaustion → unchanged reconciliation.
- Ownership: The new writer acknowledges before persistence and removes the previous failed-write redelivery path.
- Related location: admission retry exhaustion at
result_writer.rs:251.
pimlock
added
gator:in-review
This branch has not been deployed
1 participant
Summary
Supervisors poll the gateway for configuration every 10 seconds. This PR adds an opt-in push mode (
config_delivery_mode = "push"): the gateway delivers configuration over the existingConnectSupervisorsession, and supervisors apply it and acknowledge it.pollstays the default, and older supervisors and gateways keep polling.Tip
See interactive walk-through of main parts of this PR.
Related Issue
Part of #1731. Durable operations are deferred to a follow-up PR. This replaces the stacked PRs #3244, #3265 and #3273.
Changes
Testing
mise run pre-commitmise run e2e:rust:pushandmise run e2e:rust(poll)mise run go:ci,mise run sdk:ts:ciChecklist