[WIP] feat(pressure reconciler): Support no planner integration#759
[WIP] feat(pressure reconciler): Support no planner integration#759
Conversation
aa7d274 to
ddd61be
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends the pressure reconciler to operate without a planner integration (using a configured minimum runner count as fallback/static pressure), and refactors the HTTP server endpoints to use RunnerManager directly.
Changes:
- Add
RunnerInfo+RunnerManager.get_runner_info()for aggregated runner status reporting. - Allow
PressureReconcilerto be built/run withplanner_client=Noneand update unit tests accordingly. - Refactor
/runner/checkand/runner/flushHTTP endpoints to callRunnerManagerinstead ofRunnerScaler; bump package version and update changelog.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| github-runner-manager/src/github_runner_manager/manager/pressure_reconciler.py | Makes planner optional, adds no-planner branch in create loop, and exposes build_runner_manager. |
| github-runner-manager/src/github_runner_manager/manager/runner_manager.py | Introduces RunnerInfo and get_runner_info() aggregation on RunnerManager. |
| github-runner-manager/src/github_runner_manager/http_server.py | Switches HTTP endpoints from RunnerScaler to RunnerManager and adjusts response serialization. |
| github-runner-manager/src/github_runner_manager/cli.py | Builds a RunnerManager for the HTTP server and wires pressure reconciler threads conditionally. |
| github-runner-manager/tests/unit/test_http_server.py | Updates tests to mock/use RunnerManager directly. |
| github-runner-manager/tests/unit/manager/test_runner_manager.py | Adds unit tests for RunnerManager.get_runner_info(). |
| github-runner-manager/tests/unit/manager/test_pressure_reconciler.py | Adds tests for no-planner behavior and build function behavior. |
| github-runner-manager/pyproject.toml | Bumps application version to 0.16.0. |
| docs/changelog.md | Documents no-planner support and HTTP endpoint refactor. |
| runner_scaler = get_runner_scaler(app_config) | ||
| app.logger.info("Flushing busy: %s", flush_busy) | ||
| flush_mode = FlushMode.FLUSH_BUSY if flush_busy else FlushMode.FLUSH_IDLE | ||
| try: |
There was a problem hiding this comment.
Switching /runner/flush from RunnerScaler.flush() to RunnerManager.flush_runners() changes behavior: it no longer runs RunnerManager.cleanup() first and it no longer flushes reactive processes (which RunnerScaler.flush() did when reactive config is present). If this endpoint is still expected to fully flush a deployment in legacy/ reactive mode, consider either restoring the cleanup + reactive flush steps (e.g., by keeping RunnerScaler here or by passing a higher-level façade into the HTTP server).
| try: | |
| try: | |
| runner_manager.cleanup() |
There was a problem hiding this comment.
The behaviour change is fine - we no longer support legacy/reactive mode
Applicable spec: ISD-244
Overview
Support using no planner in the pressure reconciler.
Rationale
Currently we need one planner deployment per Github organisation. For some organisations, that only require a fixed pool of runners, a planner integration is overkill. As we are going to drop the legacy reconcile in a future PR, we need to let the pressure reconciler support no planner integration.
Checklist
urgent,trivial,complex).github-runner-manager/pyproject.toml.