Skip to content

Commit 2e67ea3

Browse files
authored
fix: max pressure (canonical#743)
* fix : use max-virtual machines for max pressure * add logline for clamping * chore: update changelog and pyproject.toml * add input validation
1 parent 098e2a5 commit 2e67ea3

11 files changed

Lines changed: 118 additions & 2 deletions

File tree

docs/changelog.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ This changelog documents user-relevant changes to the GitHub runner charm.
55
## 2026-02-27
66

77
- Fix a bug where image labels (base,arch) are not propagated to the planner charm.
8+
- Fix a bug where max_total_virtual_machines is not propagated to the github-runner-manager when used with the planner.
89

910
## 2026-02-25
1011

github-runner-manager/pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
[project]
55
name = "github-runner-manager"
6-
version = "0.13.1"
6+
version = "0.13.2"
77
authors = [
88
{ name = "Canonical IS DevOps", email = "is-devops-team@canonical.com" },
99
]

github-runner-manager/src/github_runner_manager/configuration/base.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,11 +214,36 @@ class NonReactiveCombination(BaseModel):
214214
image: Information about the image to spawn.
215215
flavor: Information about the flavor to spawn.
216216
base_virtual_machines: Number of instances to spawn for this combination.
217+
max_total_virtual_machines: Maximum number of instances to spawn. 0 means no cap.
217218
"""
218219

219220
image: "Image"
220221
flavor: "Flavor"
221222
base_virtual_machines: int
223+
max_total_virtual_machines: int = 0
224+
225+
@root_validator(pre=False, skip_on_failure=True)
226+
@classmethod
227+
def check_max_ge_base(cls, values: dict) -> dict:
228+
"""Validate that max_total_virtual_machines is not below base_virtual_machines.
229+
230+
Args:
231+
values: Values in the pydantic model.
232+
233+
Raises:
234+
ValueError: if max_total_virtual_machines is set but lower than base_virtual_machines.
235+
236+
Returns:
237+
Values in the pydantic model.
238+
"""
239+
max_vms = values.get("max_total_virtual_machines", 0)
240+
base_vms = values.get("base_virtual_machines", 0)
241+
if 0 < max_vms < base_vms:
242+
raise ValueError(
243+
f"max_total_virtual_machines ({max_vms}) must be >= base_virtual_machines"
244+
f" ({base_vms})"
245+
)
246+
return values
222247

223248

224249
class ReactiveConfiguration(BaseModel):

github-runner-manager/src/github_runner_manager/manager/pressure_reconciler.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,13 @@ class PressureReconcilerConfig:
4747
reconcile_interval: Minutes between timer-based delete reconciliations.
4848
min_pressure: Minimum desired runner count (floor) for the flavor.
4949
Also used as fallback when the planner is unavailable.
50+
max_pressure: Maximum desired runner count (ceiling). 0 means no cap.
5051
"""
5152

5253
flavor_name: str
5354
reconcile_interval: int = 5
5455
min_pressure: int = 0
56+
max_pressure: int = 0
5557

5658

5759
class PressureReconciler: # pylint: disable=too-few-public-methods
@@ -228,7 +230,16 @@ def _desired_total_from_pressure(self, pressure: int) -> int:
228230
Returns:
229231
The desired total number of runners.
230232
"""
231-
return max(pressure, self._config.min_pressure, 0)
233+
total = max(pressure, self._config.min_pressure, 0)
234+
if self._config.max_pressure > 0 and total > self._config.max_pressure:
235+
logger.info(
236+
"Pressure %s exceeds max_pressure %s, clamping to %s",
237+
pressure,
238+
self._config.max_pressure,
239+
self._config.max_pressure,
240+
)
241+
total = self._config.max_pressure
242+
return total
232243

233244

234245
def build_pressure_reconciler(config: ApplicationConfiguration, lock: Lock) -> PressureReconciler:
@@ -260,6 +271,7 @@ def build_pressure_reconciler(config: ApplicationConfiguration, lock: Lock) -> P
260271
flavor_name=config.name,
261272
reconcile_interval=config.reconcile_interval,
262273
min_pressure=first.base_virtual_machines,
274+
max_pressure=first.max_total_virtual_machines,
263275
),
264276
lock=lock,
265277
)

github-runner-manager/tests/integration/factories.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,7 @@ def create_default_config(
236236
},
237237
"flavor": {"name": openstack_config.flavor or "small", "labels": ["small"]},
238238
"base_virtual_machines": 1,
239+
"max_total_virtual_machines": 0,
239240
}
240241
]
241242
},

github-runner-manager/tests/unit/manager/test_pressure_reconciler.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,35 @@ def _wait(_interval: int) -> bool:
158158
assert mgr.cleanup_called == 0
159159

160160

161+
@pytest.mark.parametrize(
162+
"pressure, min_pressure, max_pressure, expected",
163+
[
164+
pytest.param(5, 0, 10, 5, id="within_bounds"),
165+
pytest.param(15, 0, 10, 10, id="clamped_to_max"),
166+
pytest.param(1, 3, 10, 3, id="raised_to_min"),
167+
pytest.param(15, 3, 10, 10, id="max_wins_over_pressure"),
168+
pytest.param(5, 0, 0, 5, id="zero_max_means_no_cap"),
169+
pytest.param(-1, 0, 0, 0, id="negative_clamped_to_zero"),
170+
],
171+
)
172+
def test_desired_total_from_pressure_respects_bounds(
173+
pressure: int, min_pressure: int, max_pressure: int, expected: int
174+
):
175+
"""
176+
arrange: A reconciler with various min/max pressure configurations.
177+
act: Call _desired_total_from_pressure.
178+
assert: The result is clamped within the configured bounds.
179+
"""
180+
mgr = _FakeManager()
181+
planner = _FakePlanner()
182+
cfg = PressureReconcilerConfig(
183+
flavor_name="small", min_pressure=min_pressure, max_pressure=max_pressure
184+
)
185+
reconciler = PressureReconciler(mgr, planner, cfg, lock=Lock())
186+
187+
assert reconciler._desired_total_from_pressure(pressure) == expected
188+
189+
161190
def test_handle_timer_reconcile_uses_desired_total_not_raw_pressure():
162191
"""
163192
arrange: A reconciler with 4 runners and min_pressure=5.

github-runner-manager/tests/unit/test_config.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
non_reactive_configuration:
4444
combinations:
4545
- base_virtual_machines: 1
46+
max_total_virtual_machines: 2
4647
flavor:
4748
labels:
4849
- flavorlabel
@@ -142,6 +143,7 @@ def app_config_fixture() -> ApplicationConfiguration:
142143
labels=["flavorlabel"],
143144
),
144145
base_virtual_machines=1,
146+
max_total_virtual_machines=2,
145147
)
146148
]
147149
),
@@ -206,6 +208,38 @@ def test_load_configuration_from_yaml(app_config: ApplicationConfiguration):
206208
assert loaded_app_config == app_config
207209

208210

211+
def test_non_reactive_combination_rejects_max_below_base():
212+
"""
213+
arrange: A NonReactiveCombination where max_total_virtual_machines < base_virtual_machines.
214+
act: Construct the model.
215+
assert: A ValidationError is raised.
216+
"""
217+
with pytest.raises(
218+
ValueError, match="max_total_virtual_machines.*must be.*base_virtual_machines"
219+
):
220+
NonReactiveCombination(
221+
image=Image(name="img", labels=[]),
222+
flavor=Flavor(name="flv", labels=[]),
223+
base_virtual_machines=5,
224+
max_total_virtual_machines=3,
225+
)
226+
227+
228+
def test_non_reactive_combination_allows_zero_max():
229+
"""
230+
arrange: A NonReactiveCombination where max_total_virtual_machines is 0 (no cap).
231+
act: Construct the model.
232+
assert: No error is raised.
233+
"""
234+
combo = NonReactiveCombination(
235+
image=Image(name="img", labels=[]),
236+
flavor=Flavor(name="flv", labels=[]),
237+
base_virtual_machines=5,
238+
max_total_virtual_machines=0,
239+
)
240+
assert combo.max_total_virtual_machines == 0
241+
242+
209243
def test_configuration_allows_empty_planner_fields():
210244
"""Planner URL/token are optional for non-planner mode."""
211245
config = yaml.safe_load(StringIO(SAMPLE_YAML_CONFIGURATION))

src/charm_state.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,12 @@ def from_charm(cls, charm: CharmBase) -> "OpenstackRunnerConfig":
626626
"Both deprecated and new configuration are set for the number of machines to spawn."
627627
)
628628

629+
if 0 < max_total_virtual_machines < base_virtual_machines:
630+
raise CharmConfigInvalidError(
631+
f"max-total-virtual-machines ({max_total_virtual_machines})"
632+
f" must be >= base-virtual-machines ({base_virtual_machines})"
633+
)
634+
629635
flavor_label_config = cast(str, charm.config[FLAVOR_LABEL_COMBINATIONS_CONFIG_NAME])
630636
flavor_label_combinations = _parse_flavor_label_list(flavor_label_config)
631637
if len(flavor_label_combinations) == 0:

src/factories.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ def _get_non_reactive_configuration(state: CharmState) -> NonReactiveConfigurati
129129
image=image,
130130
flavor=flavor,
131131
base_virtual_machines=state.runner_config.base_virtual_machines,
132+
max_total_virtual_machines=state.runner_config.max_total_virtual_machines,
132133
)
133134
]
134135
return NonReactiveConfiguration(combinations=combinations)

tests/unit/test_charm_state.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -846,6 +846,12 @@ def test_parse_virtual_machine_numbers(
846846
1,
847847
"deprecated and new configuration are set for the number of machines to spawn",
848848
),
849+
(
850+
0,
851+
5,
852+
3,
853+
"max-total-virtual-machines (3) must be >= base-virtual-machines (5)",
854+
),
849855
],
850856
)
851857
def test_error_parse_virtual_machine_numbers(

0 commit comments

Comments
 (0)