feat(garm): support runner config via scaleset templates#265
Merged
Conversation
…on (ISD-5728) Add six operator-facing runner config options (dockerhub-mirror, runner-http-proxy, aproxy-exclude-addresses, aproxy-redirect-ports, otel-collector-endpoint, pre-job-script) to the garm-configurator charm. The GARM charm processes these via the relation databag, renders them into per-scaleset runner-install templates, and applies changes through the reconciler without requiring a charm upgrade. Configurator side: - charmcraft.yaml: add six config options - charm_state.py: add RunnerConfig with validation (http(s) URLs, IPv4 CIDRs, port ranges, aproxy-requires-proxy cross-check) - charm.py: publish runner config to garm-configurator relation databag GARM side: - runner_template.py: render runner options into a GARM runner-install template (aproxy nft rules, docker registry mirror, otel env, pre-job hook with heredoc-safe delimiters) - garm_api.py: add template API wrapper methods (list/get/create/update/ delete) over the generated TemplatesApi client - scaleset_reconciler.py: add runner_config to ScalesetSpec and template lifecycle (ensure/create/update/detach/delete) to the reconcile flow - charm.py: build runner_config from the relation databag in _build_desired_scalesets Tests: - Configurator: 13 new unit tests (invalid config validation, relation data publishing, aproxy-requires-proxy) - GARM: 10 new runner_template unit tests (injection, per-option, malformed tokens, heredoc collision, otel newline stripping, from_databag) - GARM: 4 new scaleset_reconciler template lifecycle tests (create, update, detach, keep-when-system-missing) - Integration: test_runner_options_render_into_scaleset_template
Remove the duplicated set of five template methods appended to GarmAuthenticatedClient (they shadowed the pre-existing ones); fold the new behaviour into the originals: optional partial_name/os_type filters on list_templates and defaults on create_template. In the scaleset reconciler, type observed templates as dict[str, Template], drop the shallow _template_id helper for attribute access, and remove the redundant template_changed check already covered by _needs_update. Simplify the configurator's pre_job_script handling and drop a redundant re-assertion loop in the garm integration test.
runner_template.py is an imported module, never executed directly, so the `#!/usr/bin/env python3` shebang was misleading. Align it with the other pure modules (charm_state.py, garm_api.py) that start with the copyright header.
…dators Move RunnerConfig validation off the imperative from_charm and onto the model: a model_validator normalises empty/whitespace values to None, field validators check the URL / IPv4-list / port-list options, and an after model_validator enforces the aproxy-requires-proxy rule. from_charm now just reads the config strings and converts ValidationError to CharmConfigInvalidError. Drops the two module-level validation helpers.
The runner-option rendering was a hand-written approximation with wrong values and missing pieces. Port it faithfully from the production github-runner-manager templates (env.j2, openstack-userdata.sh.j2, pre-job.j2): - env: emit DOCKERHUB_MIRROR + CONTAINER_REGISTRY_URL, and fix the OTEL variable name to ACTION_OTEL_EXPORTER_OTLP_ENDPOINT. - aproxy: listen on :54969, write /etc/nftables.conf with the default-ipv4 gateway lookup, a named exclude set (always including 127.0.0.0/8), and both prerouting and output DNAT chains; enable nftables.service. - docker mirror: add systemctl daemon-reload before restart. - static host prep: adduser the runner account to lxd/adm. - pre-job hook: drop the fabricated proxy-pinning snippet; set up the OpenTelemetry collector config and run the operator pre-job-script via the production temp-file pattern. Removes the TODO(ISD-278) stubs. Tests updated to assert the ported output.
Describe what each rendered block does directly, instead of citing the legacy github-runner charm templates and line numbers. The new charm should stand on its own; readers shouldn't need the legacy charm to understand this module.
Add aproxy-exclude-addresses (previously untested) and tighten the asserted markers so each config option has a distinct, specific marker in the rendered scaleset template (docker env var, aproxy listener + nft ruleset, otel env var, pre-job hook).
RunnerConfig is relation-derived desired state, so per ISD014 (managing charm complexity) it belongs in charm_state.py alongside CharmState and SSHDebugInfo — not in the rendering module. runner_template.py now imports it and only renders it; charm_state.py stays free of rendering/reconciler imports to avoid a cycle. Also hoist the large _OTEL_COLLECTOR_SETUP constant to the top of runner_template.py with the other module constants, and document the value-object placement rule in the garm AGENTS.md.
cbartz
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Adds runner-level configuration propagation from garm-configurator into the GARM charm’s scaleset reconciliation by generating per-scaleset runner-install templates, so runner behaviour changes take effect via live reconcile (no manual upgrade/restart cycle).
Changes:
- Extend the configurator→garm relation contract with runner options and validate them in
garm-configurator. - Add a runner-install template renderer and template lifecycle management in the GARM scaleset reconciler.
- Add unit + integration coverage for template creation/update/detach and rendered script contents.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| charms/tests/integration/test_garm.py | Adds an integration test that asserts runner options appear in the rendered per-scaleset template. |
| charms/garm/tests/unit/test_scaleset_reconciler.py | Extends reconciler unit coverage for enabled scalesets and template lifecycle operations. |
| charms/garm/tests/unit/test_runner_template.py | New unit tests for runner_template rendering, including sanitisation and delimiter collision cases. |
| charms/garm/src/scaleset_reconciler.py | Introduces per-scaleset template management and enables scalesets on reconcile. |
| charms/garm/src/runner_template.py | New template renderer that injects docker mirror, aproxy, OTEL, and pre-job hook logic into a base template. |
| charms/garm/src/garm_api.py | Extends list_templates to support server-side filtering; adds defaults for create_template. |
| charms/garm/src/charm.py | Threads runner-config databag values into ScalesetSpec during desired-state construction. |
| charms/garm/src/charm_state.py | Adds RunnerConfig value object and defines the runner-option relation databag key contract. |
| charms/garm/AGENTS.md | Documents the new “value objects live in charm_state.py” convention for runner options. |
| charms/garm-configurator/tests/unit/test_charm.py | Adds unit tests for runner-config validation and for writing runner-config fields into the relation databag. |
| charms/garm-configurator/src/charm.py | Writes runner-config fields into the garm-configurator relation databag. |
| charms/garm-configurator/src/charm_state.py | Adds RunnerConfig (Pydantic) with URL/IP/port validation and aproxy dependency validation. |
| charms/garm-configurator/charmcraft.yaml | Adds new charm config options for runner behaviour (mirror/proxy/aproxy/otel/pre-job). |
cbartz
force-pushed
the
feat/configurator-app-config-ISD-5728-rebase
branch
from
96bf938 to
1a08ab9
Compare
cbartz
force-pushed
the
feat/configurator-app-config-ISD-5728-rebase
branch
from
1a08ab9 to
6305fb6
Compare
cbartz
force-pushed
the
feat/configurator-app-config-ISD-5728-rebase
branch
from
6305fb6 to
7d0654c
Compare
cbartz
force-pushed
the
feat/configurator-app-config-ISD-5728-rebase
branch
from
d78eb48 to
1c7e8c3
Compare
cbartz
requested review from
javierdelapuente,
weiiwang01,
yanksyoon and
yhaliaw
as code owners
yanksyoon
approved these changes
Jul 7, 2026
Split ScalesetReconciler.reconcile into _load_templates (the conditional template fetch) and _reconcile_one (the per-spec body), dropping its cyclomatic complexity from 12 to 6. Pure extraction, no behaviour change.
…nja2 Move port/address/env-value sanitisation from the renderer into RunnerConfig.from_databag so relation-sourced values are cleaned at the boundary. Render the runner-install script from Jinja2 templates in src/templates/*.j2 (FileSystemLoader) instead of inline string concatenation.
mypy cannot infer the generic parameter of TypeAdapter(IPvAnyNetwork) (IPvAnyNetwork is a union special form), so the static check failed with var-annotated. Add the explicit TypeAdapter[IPvAnyNetwork] annotation.
_detach_synced_credential could not delete test-org once a prior test left a scaleset attached: GARM 400s an org delete while a scaleset references it. Remove scalesets first; with min-idle-runners at zero they hold no runners and delete cleanly, so the org (and then the credential) can be removed to repoint github.com.
…anup The scaleset delete added in the previous commit 400s while GARM still has the scaleset enabled/draining. Mirror the charm's _delete_orphaned pattern: disable (idle count to zero) to trigger the drain, then retry the delete until GARM drains it. Also log GARM's response body on a failed delete to aid diagnosis.
…mments Keep _detach_synced_credential and _delete_scalesets docstrings to their interface; put the GARM referential-integrity/drain rationale as technical comments at the call site and at the disable step where it applies.
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.
What this PR does
garm-configuratorinto GARM scaleset reconciliationWhy we need it
GARM stores runner-install behavior in templates, so exposing runner options from the configurator is only useful if the operator can reconcile those options into the scaleset's effective template without a manual upgrade or restart cycle.
Checklist
CONTRIBUTING.mdhas been updated upon changes to the contribution/development process (e.g. changes to the way tests are run)docs/changelog.mdwith user-relevant changes(e.g., in
.github/workflows/integration_tests.yaml, ensure themoduleslist is correct)terraform fmtpasses andtflintreports no errorsAGENTS.md.copilot-collections.yamlor.github/instructions/: I re-checked whether theAGENTS.md"12-factor divergences" guidance still matches the upstream copilot-collections guidanceTest plan
tox -c charms/garm/tox.toml -e unit(199 passed)pytest charms/garm/tests/unit/test_runner_template.py charms/garm/tests/unit/test_scaleset_reconciler.py(30 passed)tox -c charms/garm-configurator/tox.toml -e unitcurrently fails during tox env setup becauseops-scenario==8.7.1depends onops==3.7.1whilerequirements.txtpinsops==3.8.0Review focus
RunnerConfigvalidation and relation plumbing ingarm-configuratorscaleset_reconciler.py, especially create/update/detach behaviorrunner_template.pyPotential breaking changes
New dependencies, APIs, modules, or workflow changes
charms/garm/src/runner_template.pymodulecharms/tests/integration/test_garm.py