Skip to content

[WIP] feat(pressure reconciler): Support no planner integration#759

Draft
cbartz wants to merge 5 commits intomainfrom
feat/pressure-reconciler-no-planner-ISD-4104
Draft

[WIP] feat(pressure reconciler): Support no planner integration#759
cbartz wants to merge 5 commits intomainfrom
feat/pressure-reconciler-no-planner-ISD-4104

Conversation

@cbartz
Copy link
Collaborator

@cbartz cbartz commented Mar 18, 2026

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

  • The charm style guide was applied.
  • The contributing guide was applied.
  • The changes are compliant with ISD054 - Managing Charm Complexity
  • The documentation for charmhub is updated.
  • The PR is tagged with appropriate label (urgent, trivial, complex).
  • The changelog is updated with changes that affects the users of the charm.
  • The application version number is updated in github-runner-manager/pyproject.toml.

@cbartz cbartz changed the base branch from main to chore/prepare-tests-for-legacy-removal-ISD-4104 March 18, 2026 10:07
Base automatically changed from chore/prepare-tests-for-legacy-removal-ISD-4104 to main March 20, 2026 11:43
@cbartz cbartz force-pushed the feat/pressure-reconciler-no-planner-ISD-4104 branch from aa7d274 to ddd61be Compare March 20, 2026 13:45
@cbartz cbartz requested a review from Copilot March 20, 2026 13:46
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 PressureReconciler to be built/run with planner_client=None and update unit tests accordingly.
  • Refactor /runner/check and /runner/flush HTTP endpoints to call RunnerManager instead of RunnerScaler; 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:
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Suggested change
try:
try:
runner_manager.cleanup()

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The behaviour change is fine - we no longer support legacy/reactive mode

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants