Add non-payload OTE extension support for run-once-durationoverride-operator#79858
Open
ropatil010 wants to merge 3 commits into
Open
Add non-payload OTE extension support for run-once-durationoverride-operator#79858ropatil010 wants to merge 3 commits into
ropatil010 wants to merge 3 commits into
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new ci-operator step-registry entry ChangesOpenShift Tests Extension Step Registry Setup
🎯 3 (Moderate) | ⏱️ ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Script as rodoo-extension-commands.sh
participant oc as oc
participant TestAdmission as TestExtensionAdmission
participant ImageStream as ImageStream
participant openshiftTests as openshift-tests
Script->>oc: install CRD / apply TestExtensionAdmission
Script->>oc: apply namespace + ImageStream manifest (imports ${EXTENSION_IMAGE})
oc->>TestAdmission: created resource YAML
oc->>ImageStream: ImageStreamTag annotations set/imported
Script->>openshiftTests: run ${TEST_SUITE} (JUnit -> ${ARTIFACT_DIR}/junit)
Suggested labels: Suggested reviewers
🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ci-operator/step-registry/cli-manager-extension/cli-manager-extension-commands.sh (1)
99-105: ⚡ Quick winQuote variables to prevent word splitting.
The
${IST}variable should be quoted to prevent potential word splitting and globbing issues when passed tooc get.♻️ Proposed fix
IST=$(oc get imagestreamtag -n test-extensions -o name | grep cli-manager | head -1) if [ -n "$IST" ]; then echo "Found in test-extensions namespace:" - oc get ${IST} -n test-extensions -o jsonpath='{.metadata.annotations}' | python3 -m json.tool || true + oc get "${IST}" -n test-extensions -o jsonpath='{.metadata.annotations}' | python3 -m json.tool || true fi fi if oc get namespace ${OPERATOR_NAMESPACE} &>/dev/null; then IST=$(oc get imagestreamtag -n ${OPERATOR_NAMESPACE} -o name 2>/dev/null | grep cli-manager | head -1 || true) if [ -n "$IST" ]; then echo "Found in ${OPERATOR_NAMESPACE} namespace:" - oc get ${IST} -n ${OPERATOR_NAMESPACE} -o jsonpath='{.metadata.annotations}' | python3 -m json.tool || true + oc get "${IST}" -n "${OPERATOR_NAMESPACE}" -o jsonpath='{.metadata.annotations}' | python3 -m json.tool || true fi fiAlso applies to: 107-113
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci-operator/step-registry/cli-manager-extension/cli-manager-extension-commands.sh` around lines 99 - 105, The oc invocations are passing the IST shell variable unquoted which can cause word-splitting/globbing; update usages of IST (the IST variable used in the oc get commands inside the cli-manager-extension-commands.sh snippet—e.g., the oc get ${IST} -n test-extensions -o jsonpath=... and the similar occurrences around lines 107-113) to use quoted variable expansion ("${IST}") so the entire imagestreamtag name is treated as a single argument; ensure all other IST occurrences in that block are similarly quoted.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@ci-operator/step-registry/cli-manager-extension/cli-manager-extension-commands.sh`:
- Around line 35-41: The script assumes .spec.tags[0].name exists and uses
unquoted vars when annotating; update the LATEST_TAG retrieval (symbol
LATEST_TAG and the oc get imagestream call using OPERATOR_IMAGE_NAME and
OPERATOR_NAMESPACE) to validate that tags were returned and fail fast with a
clear error if empty, and then quote variable expansions in the oc annotate
imagestreamtag/${OPERATOR_IMAGE_NAME}:${LATEST_TAG} -n ${OPERATOR_NAMESPACE}
invocation (i.e., quote OPERATOR_IMAGE_NAME, LATEST_TAG and OPERATOR_NAMESPACE)
to prevent word splitting/globbing and ensure the annotate command is only
executed when LATEST_TAG is non-empty.
---
Nitpick comments:
In
`@ci-operator/step-registry/cli-manager-extension/cli-manager-extension-commands.sh`:
- Around line 99-105: The oc invocations are passing the IST shell variable
unquoted which can cause word-splitting/globbing; update usages of IST (the IST
variable used in the oc get commands inside the
cli-manager-extension-commands.sh snippet—e.g., the oc get ${IST} -n
test-extensions -o jsonpath=... and the similar occurrences around lines
107-113) to use quoted variable expansion ("${IST}") so the entire
imagestreamtag name is treated as a single argument; ensure all other IST
occurrences in that block are similarly quoted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: de66b206-de1c-43d3-a3a8-fca10f6c4f29
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift/origin/openshift-origin-release-4.22-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (5)
ci-operator/config/openshift/origin/openshift-origin-release-4.22.yamlci-operator/step-registry/cli-manager-extension/OWNERSci-operator/step-registry/cli-manager-extension/cli-manager-extension-commands.shci-operator/step-registry/cli-manager-extension/cli-manager-extension-ref.metadata.jsonci-operator/step-registry/cli-manager-extension/cli-manager-extension-ref.yaml
ropatil010
force-pushed
the
cli-manager-extension
branch
from
c746e9f to
31ce64b
Compare
ropatil010
changed the title
openshift-ci
Bot
added
the
do-not-merge/invalid-owners-file
ropatil010
force-pushed
the
cli-manager-extension
branch
from
31ce64b to
271752d
Compare
Contributor
|
@ropatil010: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ci-operator/config/openshift/run-once-duration-override-operator/openshift-run-once-duration-override-operator-main.yaml (1)
40-43: ⚡ Quick winConsider anchoring the
bundle.Dockerfilepattern in the regex.The
skip_if_only_changedregex includesbundle.Dockerfilewithout anchors, which would match partial paths likefoo/bundle.Dockerfileor unintended substring matches. Consider using^bundle\.Dockerfile$or(^|/)bundle\.Dockerfile$to ensure it matches only the intended file.📝 Suggested regex improvement
- skip_if_only_changed: ^\.tekton/|bundle.Dockerfile|\.md$|^(LICENSE|OWNERS)$ + skip_if_only_changed: ^\.tekton/|^bundle\.Dockerfile$|\.md$|^(LICENSE|OWNERS)$🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci-operator/config/openshift/run-once-duration-override-operator/openshift-run-once-duration-override-operator-main.yaml` around lines 40 - 43, The current skip_if_only_changed regex contains an unanchored bundle.Dockerfile token which can match unintended paths; update the skip_if_only_changed value so the bundle.Dockerfile pattern is anchored (e.g. use ^bundle\.Dockerfile$ or (^|/)bundle\.Dockerfile$) to ensure it only matches the intended filename; locate the skip_if_only_changed entry in the YAML and replace the unanchored bundle.Dockerfile token with the chosen anchored pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ci-operator/step-registry/rodoo-extension/rodoo-extension-ref.yaml`:
- Around line 12-17: The resources block in rodoo-extension-ref.yaml currently
requests cpu: "3" and limits.memory: 10Gi which is much higher than similar
extension refs; change resources.requests.cpu and resources.requests.memory to
conservative values (e.g., match other extensions like cpu: 100m and memory:
200Mi or a justified intermediate value), set reasonable limits.memory (e.g.,
512Mi or 1Gi) to avoid CI waste, and if high resources are truly required, add a
brief justification comment in the manifest or PR description explaining why
these specific values are necessary (reference keys: resources.requests.cpu,
resources.requests.memory, resources.limits.memory).
---
Nitpick comments:
In
`@ci-operator/config/openshift/run-once-duration-override-operator/openshift-run-once-duration-override-operator-main.yaml`:
- Around line 40-43: The current skip_if_only_changed regex contains an
unanchored bundle.Dockerfile token which can match unintended paths; update the
skip_if_only_changed value so the bundle.Dockerfile pattern is anchored (e.g.
use ^bundle\.Dockerfile$ or (^|/)bundle\.Dockerfile$) to ensure it only matches
the intended filename; locate the skip_if_only_changed entry in the YAML and
replace the unanchored bundle.Dockerfile token with the chosen anchored pattern.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 9c682dfe-3c9a-4eef-8852-8b326d656f5f
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift/run-once-duration-override-operator/openshift-run-once-duration-override-operator-main-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (5)
ci-operator/config/openshift/run-once-duration-override-operator/openshift-run-once-duration-override-operator-main.yamlci-operator/step-registry/rodoo-extension/OWNERSci-operator/step-registry/rodoo-extension/rodoo-extension-commands.shci-operator/step-registry/rodoo-extension/rodoo-extension-ref.metadata.jsonci-operator/step-registry/rodoo-extension/rodoo-extension-ref.yaml
✅ Files skipped from review due to trivial changes (2)
- ci-operator/step-registry/rodoo-extension/rodoo-extension-ref.metadata.json
- ci-operator/step-registry/rodoo-extension/OWNERS
🚧 Files skipped from review as they are similar to previous changes (1)
- ci-operator/step-registry/rodoo-extension/rodoo-extension-commands.sh
Contributor
|
@ropatil010: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
openshift-ci
Bot
removed
the
do-not-merge/invalid-owners-file
ropatil010
force-pushed
the
cli-manager-extension
branch
2 times, most recently
from
c0c89b2 to
9bd0a7e
Compare
Contributor
|
@ropatil010: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
ropatil010
force-pushed
the
cli-manager-extension
branch
from
9bd0a7e to
ff04a75
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ci-operator/step-registry/rodoo-extension/rodoo-extension-commands.sh`:
- Line 15: The current unconditional "|| true" after running openshift-tests
extension-admission install-crd masks failures; replace it by running
openshift-tests extension-admission install-crd and checking its exit status and
output: if it succeeds return normally, if it fails but output/stderr contains
the expected "already exists" message treat as success, otherwise propagate the
failure (exit non-zero) so the step fails. Implement this control around the
openshift-tests extension-admission install-crd invocation (capture its
stdout/stderr and exit code) and remove the unconditional "|| true".
- Line 60: Replace the brittle jsonpath + python tool pipeline with
deterministic JSON + jq: change the oc call that uses "imagestreamtag
run-once-duration-override-operator-tests:latest -o jsonpath='...'" and "python3
-m json.tool || true" to output full JSON and pipe to jq, e.g. use "oc get
imagestreamtag run-once-duration-override-operator-tests:latest -n
test-extensions -o json | jq '.metadata.annotations // {}'" so the annotations
parsing is valid JSON and deterministic (remove the "python3 -m json.tool ||
true" construct).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 15296719-1bca-4bf7-913a-3c27fa1bbcb3
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift/run-once-duration-override-operator/openshift-run-once-duration-override-operator-main-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (5)
ci-operator/config/openshift/run-once-duration-override-operator/openshift-run-once-duration-override-operator-main.yamlci-operator/step-registry/rodoo-extension/OWNERSci-operator/step-registry/rodoo-extension/rodoo-extension-commands.shci-operator/step-registry/rodoo-extension/rodoo-extension-ref.metadata.jsonci-operator/step-registry/rodoo-extension/rodoo-extension-ref.yaml
✅ Files skipped from review due to trivial changes (1)
- ci-operator/step-registry/rodoo-extension/rodoo-extension-ref.metadata.json
🚧 Files skipped from review as they are similar to previous changes (2)
- ci-operator/step-registry/rodoo-extension/rodoo-extension-ref.yaml
- ci-operator/config/openshift/run-once-duration-override-operator/openshift-run-once-duration-override-operator-main.yaml
Contributor
Author
|
/pj-rehearse pull-ci-openshift-run-once-duration-override-operator-main-e2e-aws-operator-serial-ote |
Contributor
|
@ropatil010: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Contributor
Author
|
/assign @stbenjam @ingvagabund @gangwgr |
ropatil010
force-pushed
the
cli-manager-extension
branch
from
ff04a75 to
9a3bf80
Compare
…operator This PR adds OpenShift Tests Extension (OTE) support for run-once-duration-override-operator using the non-payload extension discovery mechanism. The run-once-duration-override-operator is an optional operator installed via OLM, not part of the core release payload, so it uses TestExtensionAdmission to dynamically discover and run tests. Changes: - Added e2e-aws-operator-serial-ote test job to main config - Created rodoo-extension step registry with: - Extension setup and test execution script - TestExtensionAdmission CRD installation and CR creation - ImageStream setup with required OTE annotations - Test suite execution: openshift/run-once-duration-override-operator/operator/serial - Configuration matches working pattern from cli-manager-operator extension: - Uses 'from: tests' base image (contains openshift-tests binary) - Uses 'timeout: 3600s' for proper timeout handling - Individual approvers/reviewers in OWNERS Related PRs: - openshift/run-once-duration-override-operator#595 (OTE infrastructure) - openshift/run-once-duration-override-operator#596 (Ginkgo tests)
ropatil010
force-pushed
the
cli-manager-extension
branch
from
9a3bf80 to
bf98458
Compare
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ropatil010 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
1 similar comment
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ropatil010 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Reorganize the step-registry structure to follow the standard pattern used by other components (e.g., cli-manager): - Move from rodoo-extension/ to run-once-duration-override-operator/test-extension/ - Rename step references from rodoo-extension to run-once-duration-override-operator-test-extension - Add OWNERS files at both component and step levels - Update CI config to reference the new step name This change aligns with the established naming convention where the component name is the parent directory and the specific step/test is the subdirectory. Co-Authored-By: Rohit Patil <ropatil@redhat.com>
Contributor
Author
|
/pj-rehearse pull-ci-openshift-run-once-duration-override-operator-main-e2e-aws-operator-serial-ote |
Contributor
|
@ropatil010: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
ropatil010
changed the title
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
…perator OTE test Add wait loop to ensure ImageStream import completes before verification. Root cause: The test script was creating an ImageStream and immediately trying to verify the ImageStreamTag exists. However, image import is asynchronous and takes time to complete, causing intermittent failures with "imagestreamtags.image.openshift.io not found" errors. Changes: - Added polling loop (5-minute timeout) that checks every 5 seconds - Waits for ImageStreamTag to exist before proceeding - Provides progress messages during the wait - Includes error handling with ImageStream status dump on timeout This prevents the race condition where the script attempts to access run-once-duration-override-operator-tests:latest before the import has finished. Co-Authored-By: Rohit Patil <ropatil@redhat.com>
Contributor
|
@ropatil010, Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
Contributor
Author
|
/pj-rehearse pull-ci-openshift-run-once-duration-override-operator-main-e2e-aws-operator-serial-ote |
Contributor
|
@ropatil010: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Contributor
|
@ropatil010: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Contributor
Author
|
Execution logs: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds OpenShift Tests Extension (OTE) support for the run-once-duration-override-operator using the non-payload extension discovery mechanism.
The run-once-duration-override-operator is an optional operator (installed via OLM), not part of the core release payload. Therefore, it uses TestExtensionAdmission to dynamically discover and run tests.
Changes:
Related:
Summary by CodeRabbit
This PR adds OpenShift Tests Extension (OTE) support for the run-once-duration-override-operator in the OpenShift CI infrastructure. Since run-once-duration-override-operator is an optional operator installed via OLM and not part of the core release payload, the PR implements non-payload extension discovery to enable test execution during the main branch builds.
Changes to Release Configuration
The PR modifies
openshift-origin-main.yamlto add a new e2e test job (e2e-aws-operator-serial-ote-rodoo) that:ipi-awsworkflow with FIPS enabledEXTENSION_IMAGEpointing to the run-once-duration-override-operator imagerodoo-extensiontest referenceipi-aws-preandfips-checkNew Test Step Registry
A new step registry is created under
ci-operator/step-registry/rodoo-extension/with:openshift/run-once-duration-override-operator/operator/serialtest suite with JUnit outputTechnical Details
Test Binary Location
ImageStream Annotations
TestExtensionAdmission CR
openshift-tests extension-admission create rodoo-extensions \ --permit=test-extensions/*Test Suite
Prerequisites
The operator repository already has the required OTE infrastructure (merged PRs #595 and #596):
cmd/run-once-duration-override-operator-tests-ext/main.go)test/e2e/operator.go,test/e2e/helpers.go)[Operator]and[Serial]tagsThe implementation enables the OpenShift CI system to dynamically discover and execute run-once-duration-override-operator tests as part of the main branch pipeline, following the non-payload extension pattern already established in the release repository.
Summary by CodeRabbit
This PR updates OpenShift CI (openshift/release) to discover and run OpenShift Tests Extension (OTE) non-payload tests for the run-once-duration-override-operator (an optional OLM-installed operator not included in the release payload).
What it changes in practical terms
Adds a step-registry entry rodoo-extension that prepares the cluster and runs the operator's OTE serial tests:
Adds an optional CI job to the operator's ci-operator config:
Metadata and OWNERS:
Why this matters
Files of note (high-level)