Skip to content

fix(garm): deliver aproxy bootstrap as pre-install script so it runs before GARM's install wrapper#272

Merged
cbartz merged 5 commits into
mainfrom
fix/garm-aproxy-pre-install-bootstrap
Jul 9, 2026
Merged

fix(garm): deliver aproxy bootstrap as pre-install script so it runs before GARM's install wrapper#272
cbartz merged 5 commits into
mainfrom
fix/garm-aproxy-pre-install-bootstrap

Conversation

@cbartz

@cbartz cbartz commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Delivers the aproxy proxy bootstrap as a GARM pre_install_scripts extra-spec entry (00-aproxy) instead of injecting it into the per-scaleset runner template. When a proxy is configured, cloud-init package upgrades are also disabled on boot. Two related fixes: snap set aproxy proxy= now gets a bare host:port (aproxy rejects URLs), and all pre_install_scripts values are base64-encoded as GARM's map[string][]byte extra-spec type requires.

Why we need it

On proxy-only networks (no direct egress), runners never registered. When a scaleset uses a server-side template, GARM prepends a compiled-in wrapper as the cloud-init install script; that wrapper must reach the GARM metadata API to fetch the template before any template content runs. Proxy bootstrap living inside the template therefore can never help the wrapper's fetch succeed — the fetch times out (~134 s/attempt) with no egress. GARM's pre_install_scripts run as root before the wrapper, so moving aproxy there brings egress up in time. The apt upgrade runs even earlier (before any pre-install script), so it's disabled when a proxy is set rather than left to time out.

Test plan

  • Unit: retargeted the aproxy rendering tests at the new render_aproxy_pre_install_script, added happy-path + scheme-stripping (http:// / https:// / scheme-less → host:port) coverage, and added scaleset-reconciler tests for the 00-aproxy + disable_updates create path and three drift cases (encoded match → no-op; legacy raw-text → update; missing disable_updates → update). tox -c tox.toml -e fmt,lint,static,unit all pass; touched-module coverage 98% / 89%.
  • Integration: updated test_runner_options_render_into_scaleset_template to assert the aproxy script now lands in the scaleset's extra_specs (base64-decoded 00-aproxy, disable_updates: true) rather than the template. Not run here (needs a live Juju model).

Review focus

  • _effective_extra_specs() in scaleset_reconciler.py is the single source of truth feeding create, update, and drift detection — worth confirming the drift comparison (encoded scripts + disable_updates) is symmetric with what's sent.
  • The 00-aproxy key name is load-bearing: GARM runs pre-install scripts in lexicographic order, so it must sort before the configurator's pre_install.sh.

Potential breaking changes

Existing scalesets get a one-time update on first reconcile as their extra_specs converge (encoding change + aproxy entry). No config-surface change. Operational prerequisite: the egress proxy must allow CONNECT to the GARM API host, since all GARM API traffic from runners now rides through aproxy.

Checklist

  • Changes comply with the project's coding standards and guidelines (see CONTRIBUTING.md and STYLE.md)
  • CONTRIBUTING.md has been updated upon changes to the contribution/development process (e.g. changes to the way tests are run) — n/a
  • Technical author has been assigned to review the PR in case of documentation changes (usually *.md files) — n/a (changelog only)
  • I updated docs/changelog.md with user-relevant changes
  • I used AI to assist with preparing this PR
  • I added or updated tests as needed (unit and integration)
  • If integration test modules are used: I updated the workflow configuration — n/a (existing test_garm module, no new module)
  • If this PR involves a Grafana dashboard: I added a screenshot of the dashboard — n/a
  • If this PR involves Terraform: terraform fmt passes and tflint reports no errors — n/a
  • If this PR involves Rockcraft: I updated the version — n/a
  • If this PR adds/removes a charm, or changes a charm's base class, conventions, tooling, or repo structure: I updated the relevant AGENTS.md — n/a (no convention change)
  • If this PR changes .copilot-collections.yaml or .github/instructions/: I re-checked the AGENTS.md "12-factor divergences" guidance — n/a

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Moves aproxy bootstrap out of the per-scaleset runner template and into GARM scaleset extra_specs.pre_install_scripts (as 00-aproxy) so it runs before GARM’s compiled-in install wrapper can fetch template content—fixing runner provisioning on proxy-only networks. It also base64-encodes all pre_install_scripts values to match GARM’s map[string][]byte wire type and sets disable_updates when a proxy is configured.

Changes:

  • Deliver aproxy bootstrap via scaleset extra_specs.pre_install_scripts["00-aproxy"] (base64-encoded) and set extra_specs.disable_updates=true when proxying.
  • Update aproxy script rendering to configure snap set aproxy proxy= with a bare host:port form.
  • Retarget/add unit + integration assertions to validate extra-spec injection, encoding, and drift behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
docs/changelog.md Documents the operational change for proxy-only networks and the new delivery mechanism.
charms/tests/integration/test_garm.py Updates integration assertions to validate aproxy delivery via extra_specs (decoded) instead of template injection.
charms/garm/tests/unit/test_scaleset_reconciler.py Adds coverage for base64 encoding, disable_updates, aproxy script injection, and drift/no-op cases.
charms/garm/tests/unit/test_runner_template.py Retargets tests away from template-injected aproxy; adds direct tests for standalone aproxy script rendering and scheme stripping.
charms/garm/src/templates/pre_install.j2 Removes aproxy block from the template pre-install snippet.
charms/garm/src/templates/aproxy.j2 Converts aproxy template into a standalone best-effort bash script and switches aproxy config to proxy_host_port.
charms/garm/src/scaleset_reconciler.py Centralizes extra_specs generation via _effective_extra_specs() for create/update/drift and injects 00-aproxy + disable_updates.
charms/garm/src/runner_template.py Introduces render_aproxy_pre_install_script() and _proxy_host_port(); updates template injection scope to exclude proxy/aproxy.

Comment thread charms/garm/src/scaleset_reconciler.py
Comment thread charms/garm/src/runner_template.py Outdated
Comment thread charms/garm/src/runner_template.py
@cbartz cbartz force-pushed the fix/garm-aproxy-pre-install-bootstrap branch from f7ea017 to b7943c4 Compare July 8, 2026 21:36
@cbartz cbartz requested a review from Copilot July 8, 2026 21:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Comment thread charms/garm/src/scaleset_reconciler.py
@cbartz cbartz force-pushed the fix/garm-aproxy-pre-install-bootstrap branch from b7943c4 to 42ee6a0 Compare July 8, 2026 21:45
@cbartz cbartz requested a review from Copilot July 8, 2026 21:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Comment thread charms/garm/src/scaleset_reconciler.py
Comment thread charms/garm/tests/unit/test_runner_template.py Outdated
…before GARM's install wrapper

When a scaleset uses a server-side template, GARM injects a compiled-in
wrapper as the cloud-init install script; the wrapper must reach the
GARM API to fetch the template before any template content runs. Proxy
bootstrap living inside the template therefore can never work on
proxy-only networks: the wrapper's fetch has no egress until the proxy
is up.

The aproxy setup is now rendered as a standalone script and delivered
via the pre_install_scripts extra-spec (key "00-aproxy", sorting before
operator scripts), which GARM runs as root before the wrapper. When a
proxy is configured, apt updates are also disabled on boot
(disable_updates extra-spec): cloud-init's package upgrade runs before
any pre-install script, so it can never use the proxy and only burns
boot time on timeouts.

Two related fixes: `snap set aproxy proxy=` now receives a bare
host:port (aproxy rejects URLs; the new `snap set system
proxy.http/https=` calls take the full URL), and all pre_install_scripts
values are base64-encoded on the wire as required by GARM's
map[string][]byte extra-spec type.
@cbartz cbartz force-pushed the fix/garm-aproxy-pre-install-bootstrap branch from 42ee6a0 to ef4d1be Compare July 8, 2026 21:51
@cbartz cbartz requested a review from Copilot July 8, 2026 21:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

cbartz added 2 commits July 9, 2026 05:25
Add 'aproxy' to the docs wordlist and reword 'prepends' (absent from the
base dictionary) to 'injects' so the docs spellcheck passes.
The nftables DNAT sends :80/:443 to aproxy's :54969; applying it when the
aproxy snap failed to install or start would black-hole all egress into a
dead port rather than leaving the box on whatever path it had. Only install
the redirect once 'snap services aproxy' reports active, tolerate an
already-baked-in snap, and log every step (and both bail-out paths) to the
cloud-init console output so a broken proxy bootstrap is diagnosable.

Also correct the header comment: cloud-init runs each pre-install script as
its own runcmd entry and continues past a non-zero exit, so strict mode
would not have protected GARM's install sequence anyway.
@cbartz cbartz marked this pull request as ready for review July 9, 2026 05:51

@yanksyoon yanksyoon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor nits, thank you @cbartz !

Comment thread charms/garm/src/templates/aproxy.j2 Outdated
Comment thread charms/garm/src/scaleset_reconciler.py
Comment thread charms/garm/tests/unit/test_scaleset_reconciler.py Outdated
Comment thread charms/garm/src/templates/aproxy.j2 Outdated
cbartz added 2 commits July 9, 2026 06:18
Review follow-ups on the aproxy pre-install script:

- Runner VMs are single-use and never reboot, so persisting the ruleset to
  /etc/nftables.conf and enabling nftables.service is dead weight. Pipe the
  ruleset straight into the kernel with 'nft -f -' instead, keeping the
  aproxy-active guard around it.
- Hoist the repeated function-local test imports (RunnerConfig,
  _effective_extra_specs, build_template_data) to the module top.
…nstall-bootstrap

# Conflicts:
#	docs/changelog.md
@cbartz cbartz enabled auto-merge (squash) July 9, 2026 07:28
@cbartz cbartz merged commit 11b7322 into main Jul 9, 2026
62 checks passed
@cbartz cbartz deleted the fix/garm-aproxy-pre-install-bootstrap branch July 9, 2026 07:59
@gabriel-samfira

gabriel-samfira commented Jul 13, 2026

Copy link
Copy Markdown

Fyi, main now has proxy support added by this PR:

cloudbase/garm#822

Proxy settings get added to package managers using the cloud-init bootcmd section, so it runs before the package upgrade.

GARM itself has separate proxy settings in the config file. I will update docs before the next release.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants