Skip to content

OCPERT-349: Add job command for running Stage testing prow job #969

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tomasdavidorg wants to merge 2 commits into
base: main
Choose a base branch
from
+97 −9
Open

OCPERT-349: Add job command for running Stage testing prow job #969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
17b5577
OCPERT-349: Add job command for running Stage testing prow job
tomasdavidorg Mar 26, 2026
9feda89
Remove trailing whitespace
tomasdavidorg Mar 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 28 additions & 5 deletions prow/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@ Options:
--help Show this message and exit.

Commands:
get_payloads Check the latest payload of each version.
get_results Return the Prow job executed info.
list List the jobs which support the API call.
run Run a job and save results to /tmp/prow-jobs.csv
run_required Run required jobs from a file
get_payloads Check the latest payload of each version.
get_results Return the Prow job executed info.
list List the jobs which support the API call.
run Run a job and save results to /tmp/prow-jobs.csv
run_image_consistency_check Run image consistency check Prow job.
run_required Run required jobs from a file
run_stage_testing Run stage testing Prow job for a given payload.
```

### Run job
Expand Down Expand Up @@ -75,6 +77,27 @@ Returned job id: 3ebe0a6e-ea5c-4c96-9ca4-295074f9eaa3
periodic-ci-openshift-openshift-tests-private-release-4.11-amd64-nightly-4.11-upgrade-from-stable-4.10-gcp-ipi-disconnected-private-p2-f14 None 3ebe0a6e-ea5c-4c96-9ca4-295074f9eaa3 2023-06-07T06:15:10Z https://qe-private-deck-ci.apps.ci.l2s4.p1.openshiftapps.com/view/gs/qe-private-deck/logs/periodic-ci-openshift-openshift-tests-private-release-4.11-amd64-nightly-4.11-upgrade-from-stable-4.10-gcp-ipi-disconnected-private-p2-f14/1666327924111839232
```

### Run stage testing

Run the stage testing Prow job for a given release payload. The job name is version-specific and constructed automatically from the payload URL.

```console
$ job run_stage_testing -p quay.io/openshift-release-dev/ocp-release:4.19.1-x86_64
Stage testing job name: periodic-ci-openshift-openshift-tests-private-release-4.19-stage-testing-e2e-aws-ipi
Stage testing job id: a1b2c3d4-e5f6-7890-abcd-ef1234567890
Stage testing job url: https://qe-private-deck-ci.apps.ci.l2s4.p1.openshiftapps.com/view/gs/qe-private-deck/logs/...
```

### Run image consistency check

Run the image consistency check Prow job for a given release payload and shipment MR.

```console
$ job run_image_consistency_check -p quay.io/openshift-release-dev/ocp-release:4.19.1-x86_64 -m 189
Image consistency check job id: a1b2c3d4-e5f6-7890-abcd-ef1234567890
Comment on lines +93 to +97
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 26, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Align MR argument wording with the implemented CLI option.

Line 93 says “shipment MR”, but the command interface uses merge request ID via -m/--mr-id (prow/job/job.py, Line 816). Please align wording/flag naming in README to prevent confusion.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@prow/README.md` around lines 93 - 97, The README wording "shipment MR" is
inconsistent with the CLI option; update the README example and description for
the run_image_consistency_check command to refer to the merge request ID and the
actual flag name used by the CLI (--mr-id / -m), e.g., change "shipment MR" to
"merge request ID (MR) via -m/--mr-id" and ensure the sample invocation and
explanatory text match the option implemented in the job
run_image_consistency_check command and the CLI option --mr-id in
prow/job/job.py.

Image consistency check job url: https://qe-private-deck-ci.apps.ci.l2s4.p1.openshiftapps.com/view/gs/qe-private-deck/logs/...
```

### Debug failure job
- `Error code: 500, reason: Internal Server Error`
This error indicates the input job name doesn't exist, you need to input an exist one.
Expand Down
73 changes: 69 additions & 4 deletions prow/job/job.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class Jobs:
POLL_INTERVAL_SECONDS = 5

IMAGE_CONSISTENCY_CHECK_JOB_NAME = "periodic-ci-openshift-release-tests-main-image-consistency-check"
STAGE_TESTING_JOB_NAME_TEMPLATE = "periodic-ci-openshift-openshift-tests-private-release-{minor_release}-stage-testing-e2e-aws-ipi"

def __init__(self):
self.run = False
Expand Down Expand Up @@ -346,11 +347,14 @@ def _is_valid_payload_url(self, payload_url: str) -> bool:
Returns:
True if the payload URL is valid, False otherwise.
"""

pattern = r"^quay\.io/openshift-release-dev/ocp-release:\d+\.\d+\.\d+.*-x86_64$"
return re.match(pattern, payload_url) is not None

Copy link
Copy Markdown
Contributor

@rioliu-rh rioliu-rh Mar 30, 2026

Choose a reason for hiding this comment

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

Remove trailing whitespace on this line for consistency with project style.

Copy link
Copy Markdown
Contributor Author

@tomasdavidorg tomasdavidorg Mar 30, 2026

Choose a reason for hiding this comment

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

Fixed.


def _get_minor_release_from_payload_url(self, payload_url):
"""Extract minor release (e.g. '4.19') from a validated payload URL."""
z_version = payload_url.split(":")[1].split("-")[0]
return ".".join(z_version.split(".")[:2])
Copy link
Copy Markdown
Contributor

@rioliu-rh rioliu-rh Mar 30, 2026

Choose a reason for hiding this comment

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

Since you've extracted the minor release parsing logic into _get_minor_release_from_payload_url(), consider refactoring run_image_consistency_check() to use this same helper method instead of duplicating the logic.

This would eliminate duplication and make the code more maintainable. For example, you could replace the inline parsing in run_image_consistency_check() with a call to this helper.

Copy link
Copy Markdown
Contributor Author

@tomasdavidorg tomasdavidorg Mar 30, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

There is no duplicated parsing in run_image_consistency_check(). It uses a single constant job name (IMAGE_CONSISTENCY_CHECK_JOB_NAME) and passes the payload URL as MULTISTAGE_PARAM_OVERRIDE_PAYLOAD_URL without needing to extract the minor release from it.

Or do you mean something else?

Copy link
Copy Markdown
Contributor

@rioliu-rh rioliu-rh Mar 30, 2026

Choose a reason for hiding this comment

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

all right


def run_image_consistency_check(self, payload_url: str, mr_id: int) -> dict:
"""
Run image consistency check Prow job.
Expand All @@ -362,7 +366,6 @@ def run_image_consistency_check(self, payload_url: str, mr_id: int) -> dict:
Returns:
A dictionary containing the job info.
"""

if not self._is_valid_payload_url(payload_url):
raise Exception(f"Invalid payload URL: {payload_url}")

Expand Down Expand Up @@ -399,9 +402,59 @@ def run_image_consistency_check(self, payload_url: str, mr_id: int) -> dict:

return job_status

def run_stage_testing(self, payload_url: str) -> dict:
"""
Run stage testing Prow job.

The Prow job name is version-specific:
periodic-ci-openshift-openshift-tests-private-release-{minor_release}-stage-testing-e2e-aws-ipi

Args:
payload_url: The URL of the release payload to test
(e.g. quay.io/openshift-release-dev/ocp-release:4.19.1-x86_64)

Returns:
A dictionary containing the job info.
"""
if not self._is_valid_payload_url(payload_url):
raise Exception(f"Invalid payload URL: {payload_url}")

minor_release = self._get_minor_release_from_payload_url(payload_url)

job_name = Jobs.STAGE_TESTING_JOB_NAME_TEMPLATE.format(minor_release=minor_release)
Copy link
Copy Markdown
Contributor

@rioliu-rh rioliu-rh Mar 30, 2026

Choose a reason for hiding this comment

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

The job name is already printed on line 427, which is good for consistency with run_image_consistency_check(). Nice work maintaining the same output pattern!

print(f"Stage testing job name: {job_name}")

url = self.gangway_url + job_name
data = {
"job_execution_type": "1",
"pod_spec_options": {
"envs": {
"RELEASE_IMAGE_LATEST": payload_url,
}
}
}

job_run_res = self._get_session().post(
url=url,
json=data,
headers=self.get_prow_headers()
)
Comment on lines +437 to +441
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 26, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
python - <<'PY'
import re
from pathlib import Path

p = Path("prow/job/job.py")
text = p.read_text(encoding="utf-8")
m = re.search(r"def run_stage_testing\(.*?return job_status", text, re.S)
if not m:
    raise SystemExit("run_stage_testing block not found")
block = m.group(0)
print("timeout_present_in_run_stage_testing_post =", "timeout=" in block)
for i, line in enumerate(block.splitlines(), 1):
    if ".post(" in line or "timeout=" in line:
        print(f"{i:03d}: {line}")
PY

Repository: openshift/release-tests

Length of output: 169

🏁 Script executed:

#!/bin/bash
# Verify lines 440-444 match the review comment
head -444 prow/job/job.py | tail -20

Repository: openshift/release-tests

Length of output: 695

🏁 Script executed:

#!/bin/bash
# Search for all .post() calls in the file to check for other timeout issues
rg "\.post\(" prow/job/job.py -A 3 -B 2

Repository: openshift/release-tests

Length of output: 650

🏁 Script executed:

#!/bin/bash
# Check for any session configuration or timeout defaults in the file
rg "session|timeout" prow/job/job.py -i | head -20

Repository: openshift/release-tests

Length of output: 1183

Add a timeout to the Gangway POST request.

The POST call to the Gangway API (lines 440-444) currently has no timeout, so the CLI can block indefinitely on network/API stalls.

        job_run_res = self._get_session().post(
            url=url,
            json=data,
-           headers=self.get_prow_headers()
+           headers=self.get_prow_headers(),
+           timeout=30
        )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
job_run_res = self._get_session().post(
url=url,
json=data,
headers=self.get_prow_headers()
)
job_run_res = self._get_session().post(
url=url,
json=data,
headers=self.get_prow_headers(),
timeout=30
)
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@prow/job/job.py` around lines 440 - 444, The Gangway POST call at job_run_res
= self._get_session().post(...) has no timeout and can hang; add a timeout kwarg
to the post call (for example timeout=self.request_timeout or timeout=10) so the
request will fail fast on network stalls, and if you choose a configurable value
add a request_timeout attribute (or a constant) to the class and use that in the
call; update the post invocation in job_run_res to include
timeout=<configured_value>.

if job_run_res.status_code != 200:
raise Exception(
f"Failed to run stage testing job '{job_name}'. "
f"Error code: {job_run_res.status_code}, reason: {job_run_res.reason}"
)

job_id = json.loads(job_run_res.text)["id"]
print(f"Stage testing job id: {job_id}")

job_status = self.get_job_results(job_id, poll=True)
print(f"Stage testing job url: {job_status['jobURL']}")

return job_status

def run_job(self, job_name, payload, upgrade_from, upgrade_to):
"""Function run Prow job by calling the API"""

job_id = None

if job_name is None:
Expand Down Expand Up @@ -767,6 +820,18 @@ def run_image_consistency_check(payload_url: str, mr_id: int):
"""
JOB.run_image_consistency_check(payload_url, mr_id)


@cli.command("run_stage_testing")
@click.option("-p", "--payload-url", type=str, required=True, help="Payload URL (e.g. quay.io/openshift-release-dev/ocp-release:4.19.1-x86_64)")
def run_stage_testing(payload_url: str):
"""
Run stage testing Prow job for a given payload.

Args:
payload_url (str): The URL of the payload
"""
JOB.run_stage_testing(payload_url)

# no need this program entry since this file won't be imported as a module.
# if __name__ == "__main__":
# start = time.time()
Expand Down