feat(prow-job-executor): abort gating E2E job on rollout cancellation (AROSLSRE-1339) by raelga · Pull Request #259 · Azure/ARO-Tools

raelga · 2026-06-26T16:25:07Z

Jira: AROSLSRE-1339 · Sub-task: AROSLSRE-1349

Problem

When an EV2 gating rollout step is cancelled, prow-job-executor receives SIGTERM but previously just exited, leaving the postsubmit Prow E2E job running to completion. That wastes CI capacity and lets a job for an abandoned rollout keep reporting status.

Goal

Propagate the rollout cancellation to the gating Prow job: when the executor is cancelled, abort the E2E job it submitted, without ever touching a sibling region's job.

What changes

Abort the running job via Gangway BulkJobStatusChange (POST /v1/bulk-job-status-update) — there is no per-execution abort endpoint.
Distinguish external cancellation (SIGTERM) from an internal monitor timeout in WaitForCompletion; only the former triggers an abort, sent with context.WithoutCancel + a 30s budget so it survives the shutdown grace period.
Region-aware uniqueness guard: one EV2 pipeline fans out to several regional E2E jobs that share the same job_name and refs, and metav1.Time only serializes to one-second precision. Before aborting, list concurrent same-job/same-state executions and skip the abort (fail-safe) if any sibling shares the target's start-second.
New --abort-on-cancel flag (default true) to gate the behaviour.

Example

flowchart TD
    A[EV2 gating step cancelled] --> B[executor receives SIGTERM]
    B --> C{WaitForCompletion:<br/>monitor ctx done}
    C -->|internal timeout<br/>parent not cancelled| D[return timeout error<br/>job left running]
    C -->|parent cancelled| E{--abort-on-cancel?}
    E -->|false| F[leave Prow job running]
    E -->|true| G[handleCancellation:<br/>fresh 30s ctx, WithoutCancel + logger]
    G --> H[AbortJob: GetJobStatus]
    H --> I{terminal state?}
    I -->|yes| J[no-op]
    I -->|no| K{StartTime recorded?}
    K -->|no| L[skip: cannot bound window]
    K -->|yes| M{Spec.Refs present?}
    M -->|no| N[skip: refs part of selector]
    M -->|yes| O[abortWindowIsIsolated:<br/>list same-job/same-state execs]
    O --> P{sibling shares<br/>StartTime second?}
    P -->|yes / probe error| Q[skip: fail-safe,<br/>avoid cancelling sibling region]
    P -->|no| R[BulkJobStatusChange<br/>StartTime, StartTime → ABORTED]

Validation

From tools/prow-job-executor:

go build ./...
go vet ./...
go test ./...        # all packages ok, incl. new prowjob/abort_test.go
go tool golangci-lint run ./...   # 0 issues

New tests cover abort of a running job, skip-on-region-collision, abort when regions differ in time, the no-op terminal paths, monitor wiring, and the bulk-URL/terminal-state helpers.

Follow-ups

After this merges, bump the github.com/Azure/ARO-Tools/tools/prow-job-executor pseudo-version in ARO-HCP test/go.mod (consumed without a replace). Feature is on by default, so no ARO-HCP code change is needed.
Cross-env collisions (different job_name, same refs) are out of scope and documented as a residual; the guard only protects same-job_name regional siblings.

Copilot

Pull request overview

This PR updates prow-job-executor so that when the executor is externally cancelled (e.g., EV2 rollout cancellation leading to SIGTERM), it makes a best-effort attempt to abort the corresponding gating Prow E2E job via Gangway’s bulk status-change API, while adding a safety guard to avoid accidentally aborting a sibling regional execution.

Changes:

Add cancellation-aware monitoring logic that distinguishes external cancellation from internal monitor timeout and triggers best-effort aborts on external cancellation.
Implement Gangway bulk abort support (BulkJobStatusChange) with an isolation guard based on concurrent executions and start-time second collisions.
Add --abort-on-cancel flag (default true) and introduce focused unit tests covering abort/skip paths and monitor wiring.

Reviewed changes

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

Show a summary per file

File	Description
tools/prow-job-executor/prowjob/monitor.go	Distinguishes parent cancellation vs internal timeout; triggers best-effort abort on cancellation.
tools/prow-job-executor/prowjob/client.go	Adds bulk-abort support, derives bulk endpoint URL, lists executions, and implements isolation guard logic.
tools/prow-job-executor/prowjob/abort_test.go	Adds unit tests for abort behavior, collision skip logic, and monitor cancellation behavior.
tools/prow-job-executor/options.go	Adds `--abort-on-cancel` flag and wires it into executor monitor creation.
tools/prow-job-executor/go.mod	Promotes protobuf dependency to direct requirement for protojson usage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

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

Copilot

Pull request overview

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

Copilot

Pull request overview

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

… (AROSLSRE-1339) When the EV2 gating step is cancelled, the executor receives SIGTERM but previously left the postsubmit Prow E2E job running to completion. Propagate the cancellation by aborting the job via Gangway BulkJobStatusChange. Because one EV2 pipeline fans out to several regional E2E jobs that share the same job_name and refs, the bulk selector is region-blind and metav1.Time only serializes to one-second precision. A region-aware uniqueness guard lists concurrent same-job executions and skips the abort (fail-safe) if any sibling region shares the target's start-second, so a sibling region is never aborted. Gated behind --abort-on-cancel (default true).

…ding Address review feedback on the cancellation-abort path: - handleCancellation now re-attaches the in-hand logger to the abort context via logr.NewContext. The client methods extract the logger with logr.FromContext and fail if it is absent, so the abort must not depend on the parent context carrying one. Added a regression test that aborts from a logger-less context. - ListExecutions builds its request URL through net/url instead of string concatenation, so a --gangway-url that already carries a query string or trailing '?' no longer corrupts the request.

…fs to abort Second-round review feedback: - deriveBulkURL now preserves any base-path prefix on --gangway-url, swapping the /v1/executions route for /v1/bulk-job-status-update instead of replacing the whole path. A prefixed URL (host/gangway/v1/executions) no longer drops the prefix and 404s the abort. - AbortJob refuses to abort when the job has no Spec.Refs. Refs are part of the bulk selector and the API cannot filter by job name, so a nil-refs abort could over-select. Skip instead. - Added tests for the prefixed-URL derivation and the no-refs no-op.

…rdering Third-round review feedback: - deriveBulkURL trims an optional trailing slash before stripping the /v1/executions suffix, so a --gangway-url like host/v1/executions/ no longer yields a doubled //v1/bulk-job-status-update path (404). Added a test case. - AbortJob checks job.Spec.Refs is non-nil before the isolation probe, avoiding the extra ListExecutions/GetJobStatus HTTP calls during shutdown when a nil-refs job would be skipped anyway.

Pin the bulk abort window to StartTime truncated to the second, matching the second-precision window the isolation check uses and the precision at which ProwJob StartTime is serialized. Prevents a sub-second timestamp from producing a [t,t] window that fails to match the intended execution. Added a test with a sub-second StartTime asserting the emitted window is second-precision.

Copilot

Pull request overview

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

Use strings.TrimRight to strip any number of trailing slashes before removing the /v1/executions suffix, so a misconfigured --gangway-url ending in multiple slashes still derives a valid bulk endpoint. Added a multi-slash test case.

Copilot

Pull request overview

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

…ry/Fragment) deriveBulkURL cleared RawQuery but not ForceQuery or Fragment. A --gangway-url ending in a bare '?' (ForceQuery) or carrying a fragment would leave a trailing '?' or '#...' on the derived bulk endpoint. Clear both so the endpoint is a clean, canonical URL. Added test cases for a bare trailing '?' and a fragment.

Copilot

Pull request overview

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

Copilot AI review requested due to automatic review settings June 26, 2026 16:25

Copilot started reviewing on behalf of raelga June 26, 2026 16:26 View session

Copilot AI reviewed Jun 26, 2026

View reviewed changes

Comment thread tools/prow-job-executor/prowjob/monitor.go

Comment thread tools/prow-job-executor/prowjob/client.go

raelga requested a review from Copilot June 26, 2026 17:21

Copilot started reviewing on behalf of raelga June 26, 2026 17:22 View session

Copilot AI reviewed Jun 26, 2026

View reviewed changes

Comment thread tools/prow-job-executor/prowjob/client.go

Comment thread tools/prow-job-executor/prowjob/client.go Outdated

raelga requested a review from Copilot June 30, 2026 06:10

Copilot started reviewing on behalf of raelga June 30, 2026 06:10 View session

Copilot AI reviewed Jun 30, 2026

View reviewed changes

Comment thread tools/prow-job-executor/prowjob/client.go Outdated

Comment thread tools/prow-job-executor/prowjob/client.go Outdated

raelga force-pushed the raelga/aroslsre-1339-abort-gating-e2e branch from 3470102 to b6275ea Compare July 6, 2026 08:56

raelga requested a review from Copilot July 8, 2026 14:42

Copilot started reviewing on behalf of raelga July 8, 2026 14:43 View session

Copilot AI reviewed Jul 8, 2026

View reviewed changes

Comment thread tools/prow-job-executor/prowjob/client.go Outdated

raelga added 5 commits July 9, 2026 09:11

Copilot AI review requested due to automatic review settings July 9, 2026 09:11

raelga force-pushed the raelga/aroslsre-1339-abort-gating-e2e branch from b6275ea to e9d874e Compare July 9, 2026 09:11

Copilot started reviewing on behalf of raelga July 9, 2026 09:12 View session

Copilot AI reviewed Jul 9, 2026

View reviewed changes

Comment thread tools/prow-job-executor/prowjob/client.go Outdated

Copilot AI review requested due to automatic review settings July 9, 2026 09:26

Copilot started reviewing on behalf of raelga July 9, 2026 09:27 View session

Copilot AI reviewed Jul 9, 2026

View reviewed changes

Comment thread tools/prow-job-executor/prowjob/client.go

Copilot AI review requested due to automatic review settings July 9, 2026 09:42

Copilot started reviewing on behalf of raelga July 9, 2026 09:43 View session

Copilot AI reviewed Jul 9, 2026

View reviewed changes

Comment thread tools/prow-job-executor/prowjob/monitor.go

Comment thread tools/prow-job-executor/prowjob/client.go

Uh oh!

Conversation

raelga commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Goal

What changes

Example

Validation

Follow-ups

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

raelga commented Jun 26, 2026 •

edited

Loading