Skip to content

chore(garm): bump workload to fix runners stuck on hung database queries - #445

Open
cbartz wants to merge 1 commit into
mainfrom
chore/bump-garm-stuck-runners-fix
Open

cbartz wants to merge 1 commit into
mainfrom
chore/bump-garm-stuck-runners-fix

Conversation

@cbartz

@cbartz cbartz commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Bumps the pinned GARM workload commit to the merge of cloudbase/garm#926.

Why we need it

We regularly see runners stuck in a state, most likely because of SQL queries that hang. #926 addresses three causes in GARM's database layer:

  • Every PostgreSQL query now has a timeout (statement 60s, lock 30s, connect 10s by default). Migrations are exempt.
  • Runners left in creating are cleaned up when recording them as running fails.
  • Instance and scale set updates no longer write each other's rows, which removes an ABBA deadlock between them.

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)
  • Technical author has been assigned to review the PR in case of documentation changes (usually *.md files)
  • 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
    (e.g., in .github/workflows/integration_tests.yaml, ensure the modules list is correct)
  • If this PR involves a Grafana dashboard: I added a screenshot of the dashboard
  • If this PR involves Terraform: terraform fmt passes and tflint reports no errors
  • If this PR involves Rockcraft: I updated the version
  • 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
  • If this PR changes .copilot-collections.yaml or .github/instructions/: I re-checked whether the AGENTS.md "12-factor divergences" guidance still matches the upstream copilot-collections guidance

Not applicable or not ticked:

  • No new test here. Hung queries and the deadlock can't be reproduced reliably in the integration suite. Upstream #926 adds tests for them.
  • The rock version stays 0.2; it isn't bumped per workload pin (see fix(garm): bump pinned commit to fix scale set pool ID tag length #351).
  • CONTRIBUTING.md, Grafana, Terraform, AGENTS.md and copilot-collections are untouched.

Test plan

  • Built the rock with ./build-garm-rock.sh (Go 1.26 snap). The garm binary is statically linked and garm version reports 1ad3680.
  • CI integration run (garm) on the rebuilt rock.

Review focus

  • The pin is the #926 merge commit, not upstream main. main has three more commits, two of which change scale set deletion behaviour; they're left for a separate bump.
  • The new timeouts use GARM's defaults. The charm doesn't set the new statement_timeout_secs / lock_timeout_secs / connect_timeout_secs options, so no config change is needed.
  • Besides #926, the pin brings in one more functional commit (scale set jobs are listed in the job listing again) and two webapp dependency bumps.

AI-generated message.

@cbartz
cbartz marked this pull request as ready for review October 25, 2026 05:05
Copilot AI lite review requested due to automatic review settings October 25, 2026 05:05
Pin GARM to the merge of cloudbase/garm#926. It bounds every PostgreSQL
query with a statement, lock and connect timeout, reaps runners left in
creating when recording them as running fails, and omits associations
when writing instances and scale sets to remove an ABBA deadlock
between those updates.
@cbartz
cbartz force-pushed the chore/bump-garm-stuck-runners-fix branch from 988e00b to d6a87b6 Compare October 25, 2026 05:06

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.

🔵 Needs a closer look

It changes how PostgreSQL queries and runner cleanup behave in a production workload, and the integration test run that validates it is still pending.

1 open issue
What changed in this PR

This PR bumps the pinned GARM workload commit in garm-rockcraft.yaml to the merge of cloudbase/garm#926. That change targets runners that get stuck, which the PR links to hung PostgreSQL queries and a database deadlock. The changelog gets a dated entry describing the user-facing effect.

Changes:

  • Moves the GARM source pin to commit 1ad3680, and documents the reason in the pin comment.
  • Adds query timeouts for PostgreSQL (migrations are exempt), cleanup for runners left in creating, and a fix for an instance and scale set update deadlock.
  • Adds a 2026-10-25 changelog entry for the change.
File Description
garm-rockcraft.yaml Moves the GARM pin to the #926 merge commit and adds a comment explaining the fixes it brings in.
docs/​changelog.md Adds a dated entry describing the stuck-runner fixes for users.

Review effort: Lite

Comment thread docs/changelog.md
- `garm`: job steps now see `USER` and `LOGNAME` set to the account their UID resolves to, so `usermod -aG <group> $USER` followed by `sg <group>` works on GARM runners. This works around the duplicate-UID account layout of the runner images; removal is tracked in [#438](https://github.com/canonical/github-runner-operators/issues/438).
- `garm`: keep a removed configurator's provider published until the last scale set that uses it is gone, so removing a `garm-configurator` relation finishes instead of retrying until the five-day drain deadline. The charm records the provider set it starts GARM with, adds providers as configurator applications relate, and removes one only once no scale set refers to it — the provider is dropped on a later reconcile, after its scale sets are deleted.
- `garm` upgrade note: deployments already stuck before this release do not recover on upgrade, because the removed configurator's credentials are no longer available to the charm. Relate the same configurator application again — its unit name reproduces the missing provider name — so the provider is republished and the drain resumes; a scale set whose runner is stuck in the `deleted` status can also be cleared by hand.
- `garm`: bump the GARM workload so that runners no longer get stuck. GARM now bounds every PostgreSQL query with a timeout instead of waiting forever on a hung query or row lock, cleans up runners left in `creating` when recording their creation fails, and no longer deadlocks when an instance and its scale set are updated concurrently.

This branch has not been deployed

No deployments
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.

3 participants