Skip to content

(P4DEVOPS-15381) Fix ABS priority: CI runs no longer request priority 1 - #294

Open
isaac-jha wants to merge 1 commit into
mainfrom
P4DEVOPS-15381-abs-priority
Open

(P4DEVOPS-15381) Fix ABS priority: CI runs no longer request priority 1#294
isaac-jha wants to merge 1 commit into
mainfrom
P4DEVOPS-15381-abs-priority

Conversation

@isaac-jha

Copy link
Copy Markdown

Problem

tasks/abs.rb set the ABS request priority with:

priority = ENV['CI'] ? 1 : 2

ENV['CI'] is a string, so it is truthy for any value — including "false". Every run under CI (GitHub Actions always sets CI=true, and Litmus/module CI provisions through this task) therefore requested ABS priority 1.

Per the ABS README, "Jobs at Priority 1 ignore max_count" — priority 1 bypasses the pool's anti-swamping cap. So this made CI able to flood ABS and contributed to VM pile-up/orphans.

This is one of the orphan sources tracked in P4DEVOPS-15381 (observed via the complyci-ghactions user used by puppetlabs/comply-e2e).

Fix

priority = ENV['CI'] == 'true' ? 3 : 2
  • Guards on the actual value, so CI=false/unset → priority 2.
  • CI runs use priority 3 (lowest/default), local/interactive runs use 2. Neither claims priority 1, so both respect max_count.

Tests

Added coverage asserting the posted priority is 3 under CI=true and 2 under CI=false. Full spec/tasks/abs_spec.rb passes locally (13 examples, 0 failures).

Note on reaping

This stops CI from bypassing max_count, but the 12-hour ABS reaper backstop for these requests is being fixed separately in always-be-scheduling (the reaper was scoped only to user == always-be-scheduling). See P4DEVOPS-15381.

🤖 Generated with Claude Code

@isaac-jha
isaac-jha requested a review from a team as a code owner August 25, 2026 18:30
@isaac-jha
isaac-jha requested a balanced review from Copilot August 25, 2026 19:06

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Updates ABS provisioning priorities to prevent CI jobs from bypassing pool limits.

Changes:

  • Assigns priority 3 to CI requests and 2 otherwise.
  • Adds tests for CI=true and CI=false.

Reviewed changes

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

File Description
tasks/abs.rb Updates ABS priority selection.
spec/tasks/abs_spec.rb Adds priority behavior tests.
Suppressed comments (1)

spec/tasks/abs_spec.rb:143

  • This also uses with_env, whose cleanup unconditionally deletes CI rather than restoring the runner's original value. The example therefore leaks a global environment change to later specs. Avoid mutating the real environment here, or make the helper restore each key's previous value.
      with_env('CI' => 'false') do
        expect(abs.task(**params)).to eq({ status: 'ok', nodes: 1 })
      end

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tasks/abs.rb Outdated
Comment thread spec/tasks/abs_spec.rb
@isaac-jha
isaac-jha force-pushed the P4DEVOPS-15381-abs-priority branch from 487b0da to 13f0bcb Compare August 25, 2026 19:13
@isaac-jha
isaac-jha requested a balanced review from Copilot August 25, 2026 19:17

Copilot AI left a comment

Copy link
Copy Markdown

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 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

tasks/abs.rb:43

  • AppVeyor CI is still classified as a local run here: this same method explicitly handles AppVeyor with ENV['CI'] == 'True' above, so those requests will receive priority 2 rather than the documented CI priority 3. Compare the value case-insensitively (and add coverage for CI=True) so every supported CI provider gets the capped default priority.
    priority = ENV['CI'] == 'true' ? 3 : 2

@isaac-jha
isaac-jha force-pushed the P4DEVOPS-15381-abs-priority branch from 13f0bcb to f45d336 Compare August 25, 2026 19:39
`priority = ENV['CI'] ? 1 : 2` was always truthy because ENV['CI'] is a
string (even "false" is truthy), so every CI run requested ABS priority 1.
Priority 1 bypasses the ABS max_count cap, letting CI swamp the pool. Guard
on the actual value so CI uses the lowest priority (3) and local/interactive
runs use 2; neither claims priority 1.

This is one of the sources of orphaned ABS VMs tracked in P4DEVOPS-15381.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@isaac-jha
isaac-jha force-pushed the P4DEVOPS-15381-abs-priority branch from f45d336 to 1c2792a Compare August 25, 2026 19:49
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.

2 participants