feat(prow-job-executor): abort gating E2E job on rollout cancellation (AROSLSRE-1339)

[Rael Garcia (raelga)]

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), since 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) on execute, and the same flag (also default true) on the standalone monitor subcommand, so resuming monitoring of an already-submitted job also aborts it on cancellation instead of silently leaving it running.
• ListExecutions (used by the isolation check) now retries transient failures with a short, bounded backoff, matching the retry behaviour of the rest of the client. Previously a single transient error there made the isolation check fail outright, silently skipping the abort for that job.
• --gangway-url/--prow-url are now validated as real http(s) URLs in Validate() on both execute and monitor, so a malformed value fails fast with a clear error instead of relying on deriveBulkURL's silent fallback (kept as defense-in-depth for NewClient, which has no error return).

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, retried]
    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]

Loading

Validation

From tools/prow-job-executor:

go build ./...
go vet ./...
go test ./...        # all packages ok, incl. prowjob/abort_test.go and options_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, a transient ListExecutions failure that still lets the abort through, the monitor subcommand's --abort-on-cancel flag, the bulk-URL/terminal-state helpers, and the new --gangway-url/--prow-url validation.

WaitForCompletion now also has a doc comment spelling out where the SIGTERM wiring actually lives (ARO-HCP's test/cmd/prow-job-executor/main.go, via signal.NotifyContext), since that context isn't visible from this repo alone.

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.

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Rael Garcia (raelga)]

Rebased this branch onto the current main to clear the merge conflict in options.go. The conflict came from the newer AllowedSubscriptions fields, so I kept those fields and the PR's AbortOnCancel wiring.

The rebased head is 82d9133. go test -race ./... and go vet ./... pass in tools/prow-job-executor.

[Rael Garcia (raelga)]

This is rebased and conflict-free now. Jan-Hendrik Boll (@janboll) Gerd Oberlechner (@geoberle), could one of you review it?

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

tools/prow-job-executor/prowjob/abort_test.go:89

• In the test fixture, an unknown status query value silently falls back to JOB_EXECUTION_STATUS_UNSPECIFIED because JobExecutionStatus_value[name] returns 0 for missing keys. That can mask regressions where ListExecutions sends an unexpected status string, since the fixture would then stop filtering instead of failing the test.

		if jobName := q.Get("job_name"); jobName != "" {
			var want prowgangway.JobExecutionStatus
			if name := q.Get("status"); name != "" {
				want = prowgangway.JobExecutionStatus(prowgangway.JobExecutionStatus_value[name])
			}

[Rael Garcia (raelga)]

Pushed febb879 to fix the verify lint failure.

Run 30533085288 failed on code this PR adds:

tools/prow-job-executor/options.go:128:1: File is not properly formatted (gci)

The new AbortOnCancel field was misaligned in both RawExecuteOptions and completedExecuteOptions, so gofmt wanted one more space. That is the whole change, applied with make lint-fix. make lint is clean locally.

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

Suppressed comments (13)

tools/prow-job-executor/go.mod:34

• This PR introduces a number of indirect dependency version downgrades (AWS SDK v2, OpenTelemetry, cel-go, oauth2, grpc, x/crypto, etc.). Unless these were intentional/required by the new protobuf usage, it would be safer to minimize unrelated dependency churn (and avoid potential regression or security patch loss) by re-tidying with the repo’s standard Go toolchain/settings and/or explicitly constraining only the newly required modules.

	github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.30.0 // indirect

tools/prow-job-executor/go.mod:42

• This PR introduces a number of indirect dependency version downgrades (AWS SDK v2, OpenTelemetry, cel-go, oauth2, grpc, x/crypto, etc.). Unless these were intentional/required by the new protobuf usage, it would be safer to minimize unrelated dependency churn (and avoid potential regression or security patch loss) by re-tidying with the repo’s standard Go toolchain/settings and/or explicitly constraining only the newly required modules.

	github.com/aws/aws-sdk-go-v2 v1.36.3 // indirect
	github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.6.6 // indirect

tools/prow-job-executor/go.mod:48

• This PR introduces a number of indirect dependency version downgrades (AWS SDK v2, OpenTelemetry, cel-go, oauth2, grpc, x/crypto, etc.). Unless these were intentional/required by the new protobuf usage, it would be safer to minimize unrelated dependency churn (and avoid potential regression or security patch loss) by re-tidying with the repo’s standard Go toolchain/settings and/or explicitly constraining only the newly required modules.

	github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.34 // indirect
	github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.34 // indirect

tools/prow-job-executor/go.mod:59

• This PR introduces a number of indirect dependency version downgrades (AWS SDK v2, OpenTelemetry, cel-go, oauth2, grpc, x/crypto, etc.). Unless these were intentional/required by the new protobuf usage, it would be safer to minimize unrelated dependency churn (and avoid potential regression or security patch loss) by re-tidying with the repo’s standard Go toolchain/settings and/or explicitly constraining only the newly required modules.

	github.com/aws/smithy-go v1.22.3 // indirect

tools/prow-job-executor/go.mod:74

• This PR introduces a number of indirect dependency version downgrades (AWS SDK v2, OpenTelemetry, cel-go, oauth2, grpc, x/crypto, etc.). Unless these were intentional/required by the new protobuf usage, it would be safer to minimize unrelated dependency churn (and avoid potential regression or security patch loss) by re-tidying with the repo’s standard Go toolchain/settings and/or explicitly constraining only the newly required modules.

	github.com/envoyproxy/go-control-plane/envoy v1.35.0 // indirect
	github.com/envoyproxy/protoc-gen-validate v1.2.1 // indirect

tools/prow-job-executor/go.mod:83

• This PR introduces a number of indirect dependency version downgrades (AWS SDK v2, OpenTelemetry, cel-go, oauth2, grpc, x/crypto, etc.). Unless these were intentional/required by the new protobuf usage, it would be safer to minimize unrelated dependency churn (and avoid potential regression or security patch loss) by re-tidying with the repo’s standard Go toolchain/settings and/or explicitly constraining only the newly required modules.

	github.com/go-jose/go-jose/v4 v4.1.3 // indirect

tools/prow-job-executor/go.mod:96

• This PR introduces a number of indirect dependency version downgrades (AWS SDK v2, OpenTelemetry, cel-go, oauth2, grpc, x/crypto, etc.). Unless these were intentional/required by the new protobuf usage, it would be safer to minimize unrelated dependency churn (and avoid potential regression or security patch loss) by re-tidying with the repo’s standard Go toolchain/settings and/or explicitly constraining only the newly required modules.

	github.com/google/cel-go v0.28.0 // indirect

tools/prow-job-executor/go.mod:138

• This PR introduces a number of indirect dependency version downgrades (AWS SDK v2, OpenTelemetry, cel-go, oauth2, grpc, x/crypto, etc.). Unless these were intentional/required by the new protobuf usage, it would be safer to minimize unrelated dependency churn (and avoid potential regression or security patch loss) by re-tidying with the repo’s standard Go toolchain/settings and/or explicitly constraining only the newly required modules.

	github.com/tektoncd/pipeline v1.6.0 // indirect

tools/prow-job-executor/go.mod:151

• This PR introduces a number of indirect dependency version downgrades (AWS SDK v2, OpenTelemetry, cel-go, oauth2, grpc, x/crypto, etc.). Unless these were intentional/required by the new protobuf usage, it would be safer to minimize unrelated dependency churn (and avoid potential regression or security patch loss) by re-tidying with the repo’s standard Go toolchain/settings and/or explicitly constraining only the newly required modules.

	go.opentelemetry.io/contrib/detectors/gcp v1.38.0 // indirect
	go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.60.0 // indirect
	go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.65.0 // indirect
	go.opentelemetry.io/otel v1.40.0 // indirect
	go.opentelemetry.io/otel/exporters/stdout/stdoutmetric v1.40.0 // indirect
	go.opentelemetry.io/otel/metric v1.40.0 // indirect
	go.opentelemetry.io/otel/sdk v1.40.0 // indirect
	go.opentelemetry.io/otel/sdk/metric v1.40.0 // indirect
	go.opentelemetry.io/otel/trace v1.40.0 // indirect

tools/prow-job-executor/go.mod:161

• This PR introduces a number of indirect dependency version downgrades (AWS SDK v2, OpenTelemetry, cel-go, oauth2, grpc, x/crypto, etc.). Unless these were intentional/required by the new protobuf usage, it would be safer to minimize unrelated dependency churn (and avoid potential regression or security patch loss) by re-tidying with the repo’s standard Go toolchain/settings and/or explicitly constraining only the newly required modules.

	golang.org/x/crypto v0.51.0 // indirect
	golang.org/x/exp v0.0.0-20250210185358-939b2ce775ac // indirect
	golang.org/x/net v0.55.0 // indirect
	golang.org/x/oauth2 v0.34.0 // indirect

tools/prow-job-executor/go.mod:173

• This PR introduces a number of indirect dependency version downgrades (AWS SDK v2, OpenTelemetry, cel-go, oauth2, grpc, x/crypto, etc.). Unless these were intentional/required by the new protobuf usage, it would be safer to minimize unrelated dependency churn (and avoid potential regression or security patch loss) by re-tidying with the repo’s standard Go toolchain/settings and/or explicitly constraining only the newly required modules.

	google.golang.org/grpc v1.78.0 // indirect

tools/prow-job-executor/prowjob/abort_test.go:438

• Using a fixed sleep to coordinate goroutines can make this test flaky under load. Prefer synchronizing deterministically (e.g., have the fake server signal when it has served the first status request, or use a channel/hook in the client) and then cancel immediately after that signal.

	// Let the monitor observe the job at least once, then cancel.
	time.Sleep(30 * time.Millisecond)
	cancel()

tools/prow-job-executor/prowjob/abort_test.go:474

• Same flakiness concern as the earlier cancellation test: relying on time.Sleep for scheduling can cause intermittent failures. Consider switching to an explicit synchronization point that confirms WaitForCompletion has started polling before calling cancel().

	time.Sleep(30 * time.Millisecond)
	cancel()

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Rael Garcia (raelga)]

The verify job in run 31122220346 was canceled after 21 minutes and produced no job log, so it never validated the current head 001ddbd. I am rerunning the canceled job against the same commit.

[Rael Garcia (raelga)]

Run 31122220346, attempt 2 failed because make tidy changed tools/prow-job-executor/go.mod and go.sum, and the PR also gained the needs-rebase label after current main changed the same monitor and options code.

I rebased the cancellation work onto current main, kept both the new EV2 retry behavior and this PR's abort-on-cancel behavior, and regenerated the module files. go test -race ./..., go vet ./..., make lint, and a clean second make tidy pass locally. I am force-pushing commit cfb254c with lease so CI can validate the conflict resolution.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: raelga

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [raelga]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Copilot]

Pull request overview

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

Suppressed comments (2)

tools/prow-job-executor/validation.go:84

• The validateHTTPURL doc comment says callers can treat URL parse failures as unreachable “rather than falling back silently”, but deriveBulkURL still intentionally falls back by returning the raw input unchanged when parsing fails. This makes the comment misleading and contradicts client.go’s own deriveBulkURL comment.

// validateHTTPURL validates that rawURL is an absolute http(s) URL. Client code
// (e.g. deriveBulkURL) relies on this having already been checked, so it can
// treat a parse failure as unreachable rather than falling back silently.

tools/prow-job-executor/go.mod:44

• go.mod/go.sum include several indirect dependency downgrades (e.g. aws-sdk-go-v2 and tektoncd/pipeline) that appear unrelated to the functional change (adding protobuf/protojson usage). Even if tests pass, this increases review surface area and may reintroduce older transitive versions. Consider minimizing dependency churn (or documenting why the downgrades are required) so the PR’s intent stays focused.

	github.com/aws/aws-sdk-go v1.55.5 // indirect
	github.com/aws/aws-sdk-go-v2 v1.36.3 // indirect
	github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.6.6 // indirect
	github.com/aws/aws-sdk-go-v2/config v1.29.14 // indirect
	github.com/aws/aws-sdk-go-v2/credentials v1.17.67 // indirect