Skip to content

feat(labelfilter): synthesize the pool's platform label so platform-keyed tasks scale the pool (RIG-1471) - #5

Open
rigel-mintaka wants to merge 1 commit into
mainfrom
runners/rig-1471-pool-platform-filter
Open

feat(labelfilter): synthesize the pool's platform label so platform-keyed tasks scale the pool (RIG-1471)#5
rigel-mintaka wants to merge 1 commit into
mainfrom
runners/rig-1471-pool-platform-filter

Conversation

@rigel-mintaka

@rigel-mintaka rigel-mintaka commented Sep 8, 2026

Copy link
Copy Markdown

Makes the autoscaler's modeled pool filter synthesize the Woodpecker platform label, and makes the agent filter read each agent's self-reported one. First implementation slice of the runner label taxonomy (orion #2292, docs/designs/platform/ci-runners/runner-label-taxonomy.md, task T5). Must merge and publish an image before the orion cutover PR, which pins the digest and emits the new env var.

Why

NewPoolFilter models what a future pool agent could run, and calcAgents uses that to count eligible pending work. It synthesized repo="*" and org-id="*" but never platform — a label every real agent self-reports at registration.

Once the taxonomy keys pool selectors on platform, a pending task carrying it is unsatisfiable in the model, so calcAgents skips it. With MIN_AGENTS=0 a cold pool then never scales up for that work: the tasks sit unscheduled forever, with no error anywhere.

What changed

  • NewPoolFilter takes the pool's platform and synthesizes it before the custom labels, so an explicit ExtraAgentLabels entry still overrides it. org-id="*" stays applied last and non-overridable. An empty value synthesizes no key at all, preserving today's behaviour exactly for an unconfigured deployment.
  • New config field PoolPlatform, CLI flag pool-platform, env var WOODPECKER_POOL_PLATFORM. These names are a cross-repo contract — orion's T4 emits exactly this env var.
  • AgentFilter now models the agent's own reported platform (review finding, see below).
  • A startup Warn when PoolPlatform is unset, because the failure mode it guards is silent.

Review round

The review agent returned request-changes with 0 high, 3 medium. All are addressed.

The one that mattered. AgentFilter models a static agent from CustomLabels only, but a real agent's platform is not a custom label — it is a first-class API field, Agent.Platform (woodpecker-go/types.go:280), which this code ignored. Before this PR the gap was unreachable, because platform-keyed tasks were rejected by the pool filter and never got as far as the netting step. This PR makes them reachable, which activates it.

Verified against the live fleet rather than argued: every one of the 20 connected agents reports a truthful platform, and not one carries platform in custom_labels. So the netting step would have stopped crediting an idle static's free slots for exactly the work it is best suited to run.

The cost is real and now pinned by a test: an idle arm64 builder with two free slots, two pending arm64 image builds, and the pool boots 2 Spot agents beside the idle machine. Correct answer is 0. Test_calcAgents/an_idle_static_nets_out_platform-keyed_work_via_its_self-reported_platform asserts that, and reverting the AgentFilter fix reddens it with expected: 0, actual: 2.

Unlike the pool's value this one needs no configuration — the agent already told the server, so it is truthful by construction rather than operator-asserted.

Also fixed: two new misspell violations (US locale is pinned in .golangci.yaml), and an over-long table-case name whose rationale moved into a comment.

Deliberately not fixed: mnd fires on the len(extra)+3 capacity hints. main already trips it at both those exact sites with +2, so this renumbers a pre-existing violation rather than adding a class of one. Fixing it here would be scope creep.

Verification

go build ./... clean. go test ./... -count=1 → 0 failures. golangci-lint runmisspell count 0; the remaining findings are all pre-existing (mnd, unparam, plus generated-mock gci/gofmt/staticcheck).

Red-green by mutation, run independently rather than taken from the subagent's report — each mutation applied, observed, and reverted byte-identically:

  • Removing the pool synthesis reds TestNewPoolFilterPlatform/matching_platform_request_matches_when_PoolPlatform_is_set and Test_calcAgents/platform-keyed_pending_⇒_scales_when_PoolPlatform_matches.
  • Removing the AgentFilter synthesis reds both the new TestAgentFilter subtest and the netting test above.

The reviewer additionally mutated the label precedence, the empty-value guard, a platform="*"-when-empty variant, and the config plumbing; all four reddened their intended tests.

CI is red here for an unrelated pre-existing reason

Every pipeline in this fork — pull_request and push — errors at config-lint before producing any workflow:

status: error   event: pull_request   workflows: None
The formerly privileged plugin `docker.io/woodpeckerci/plugin-docker-buildx:6.1.0`
is no longer privileged by default, if required, add it to `WOODPECKER_PLUGINS_PRIVILEGED`

It comes from .woodpecker/publish-sealed.yaml, which this PR does not touch, and it hits main the same way. The consequence worth flagging: lint and test never run, so this repo currently has no CI gate, which is how the misspell violations reached review in the first place. Filed as RIG-3521 with the server-config fix; not fixed here because it is server config, not repo config.

The image publish is unaffected — publish-image.yml is a GitHub Actions workflow on a tag push or manual dispatch, and it last succeeded on main. So the cutover's image dependency is intact. Gate evidence above is local, and stands in for the absent CI.

Spec-impact: none — implements the frozen T5 contract in orion #2292 without changing it.
Ledger-impact: none — no design record on a ledgered surface is touched by this fork PR.

@linear-code

linear-code Bot commented Sep 8, 2026

Copy link
Copy Markdown

RIG-1471

…eyed tasks scale the pool (RIG-1471)

`NewPoolFilter` models which pending tasks the elastic pool could run. It
synthesized `repo="*"` and `org-id="*"` but never `platform` — a label every
real agent self-reports at registration (`cmd/agent/core/agent.go`) and the
model has no agent to ask.

Once the runner-label taxonomy keys pool selectors on `platform` (e.g.
`linux/arm64`), a pending task carrying `platform` is unsatisfiable in the
model, so `calcAgents` skips it when counting eligible pending work. With
`MIN_AGENTS=0` a cold pool would then **never** scale up for those tasks: they
strand forever, with no error.

Adds `config.PoolPlatform`, sourced from `WOODPECKER_POOL_PLATFORM`
(`--pool-platform`), and synthesizes `platform=<value>` **before**
`maps.Copy(labels, extra)` so an explicit `ExtraAgentLabels` entry still wins.
`org-id` stays last and non-overridable. An empty value synthesizes no
`platform` key at all, so an unconfigured deployment keeps today's behaviour
instead of asserting a platform it cannot know.

Co-authored-by: Matt Wilkinson <matt@rigel.build>
@rigel-mintaka
rigel-mintaka force-pushed the runners/rig-1471-pool-platform-filter branch from 4ca110d to 4657931 Compare September 8, 2026 03:19
@rigel-mintaka
rigel-mintaka marked this pull request as ready for review September 8, 2026 03:21
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.

1 participant