Repository navigation
fix(controller): paginate Kubernetes rollout reconciliation - #704
Closed
JiahaoTanXX (JiahaoTanXX) wants to merge 1 commit into
Closed
JiahaoTanXX (JiahaoTanXX) wants to merge 1 commit into
JiahaoTanXX (JiahaoTanXX) wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The pagination test can pass despite duplicate job creation attempts because _create_job() catches and logs the mock’s AssertionError instead of failing the test.
1 open issue
What changed in this PR
This PR removes the Kubernetes controller’s fixed 500-row reconciliation window, complementing the local-controller capacity fix in #624.
Changes:
- Add optional offset pagination to the rollout listing API.
- Read all pages before reconciliation, preserving job-creation rate limits and falling back when older servers repeat a page.
- Add pagination regression tests and document server upgrade requirements.
| File | Description |
|---|---|
tests/server/test_k8s_pagination.py |
Adds API-backed pagination, reconciliation, rate-limit, and legacy-server tests. |
docs/30-controller-configuration.md |
Explains pagination behavior and server compatibility. |
agentlightning/server/routes/rollouts.py |
Adds validated offsets to filtered rollout listings. |
agentlightning/controller/k8s_reconciler.py |
Fetches all pages before reconciliation and detects repeated pages. |
Review effort: Balanced
|
|
||
| async def async_create(self) -> None: | ||
| name = self.raw["metadata"]["name"] | ||
| assert name not in jobs, "an existing Job must not be created twice" |
Contributor
|
It is a design choice to only look at the first 500 jobs. It can simplify the controller. The afterward jobs won't be blocks as we only query the active jobs. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

With 500 running rollouts ahead of queued work, the Kubernetes controller only reads the first 500 active rows. Later jobs are never admitted until those rows leave the active set; orphaned running rollouts beyond that window are also missed.
Add optional offset pagination to
GET /api/rolloutsand read all pages before reconciling jobs. Reading first avoids skipping rows when reconciliation marks earlier rollouts terminal. Job creation still respectsmax_jobs_per_minute. If an older server ignores offsets, detect the repeated page, warn about upgrading, and retain the previous first-page behavior instead of looping indefinitely. Concurrent external state changes remain eventually reconciled on later polls.Related: #624 addressed a similar fixed query window in the local controller; this PR covers the Kubernetes path. The terminal completion cursor API is unchanged.
Validation on macOS arm64 / Python 3.12.13 with frozen dev, verl-cpu and docs dependencies:
.venv/bin/pytest -q tests: 250 passed, 4 skipped (CUDA/vLLM-dependent cases)..venv/bin/pyright: 0 errors, 0 warnings.git diff --cached --check: passed..venv/bin/mkdocs build --strictanduv build --no-sources: passed.Tests use the real ASGI rollout API and controller, with Kubernetes operations simulated. No live Kubernetes cluster, model inference or GPU training was used; no throughput improvement is claimed.
AI assistance: implemented, tested and independently reviewed with OpenAI Codex.