added ocp node parameters in pipeline #1026
Conversation
rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
WalkthroughThe main pipeline YAML was refactored for clarity and extended with new parameters for secrets, credentials, machine type, and replica count. A new task was added to process a configuration file, and the nested pipeline logic was updated to use the new parameters. Default values for certain parameters were removed in another pipeline. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MainPipeline
participant TestMetadataTask
participant ProcessRHADSConfigTask
participant StartNestedPipelinesTask
participant SubPipeline
User->>MainPipeline: Start pipeline (with parameters)
MainPipeline->>TestMetadataTask: Run test-metadata
TestMetadataTask-->>MainPipeline: Output test-name
MainPipeline->>ProcessRHADSConfigTask: Download RHADS config (based on test-name)
ProcessRHADSConfigTask-->>MainPipeline: Output rhads-config
MainPipeline->>StartNestedPipelinesTask: Start nested pipelines (with new parameters)
loop For each OCP version in RHADS config
StartNestedPipelinesTask->>SubPipeline: Start sub-pipeline (with machine-type, replicas, etc.)
end
StartNestedPipelinesTask->>MainPipeline: Wait for all sub-pipelines, check status
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
integration-tests/pipelines/e2e-main-pipeline.yaml (2)
64-75: Missing defensive checks around JOB_SPEC extractionIf
JOB_SPECis empty or the expected keys are missing, the resulting variables will be empty strings and the curl above will still run, likely against a non-existent path.
Add basic validation and fail early.
123-146:waitForhelper can spin forever if any oc call failsA transient
ocAPI failure causes the command to return non-zero, but the loop treats it as “condition still true” and continues forever.
Consider adding a retry counter or failing fast on command errors to avoid 120-minute tight loops.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
integration-tests/pipelines/e2e-main-pipeline.yaml(1 hunks)integration-tests/pipelines/rhtap-cli-e2e.yaml(0 hunks)
💤 Files with no reviewable changes (1)
- integration-tests/pipelines/rhtap-cli-e2e.yaml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: rhopp
PR: redhat-appstudio/rhtap-cli#998
File: integration-tests/config/rhads-config:1-6
Timestamp: 2025-07-09T12:31:10.478Z
Learning: When using Tekton CLI (tkn) version 0.40.0, multi-line parameters are handled correctly and do not break into separate arguments like they did in earlier versions (≥ 0.29). Multi-line configuration files can be safely passed as Tekton parameters without needing to compress or base64-encode them.
integration-tests/pipelines/e2e-main-pipeline.yaml (1)
Learnt from: rhopp
PR: redhat-appstudio/rhtap-cli#998
File: integration-tests/config/rhads-config:1-6
Timestamp: 2025-07-09T12:31:10.478Z
Learning: When using Tekton CLI (tkn) version 0.40.0, multi-line parameters are handled correctly and do not break into separate arguments like they did in earlier versions (≥ 0.29). Multi-line configuration files can be safely passed as Tekton parameters without needing to compress or base64-encode them.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Red Hat Konflux / rhtap-cli-on-pull-request
| volumes: | ||
| - name: shared-data | ||
| emptyDir: {} | ||
| results: | ||
| - name: rhads-config | ||
| description: JSON formatted configuration data | ||
| steps: | ||
| - name: get-rhads-config | ||
| image: quay.io/konflux-ci/appstudio-utils:ab6b0b8e40e440158e7288c73aff1cf83a2cc8a9@sha256:24179f0efd06c65d16868c2d7eb82573cce8e43533de6cea14fec3b7446e0b14 | ||
| env: | ||
| - name: JOB_SPEC | ||
| value: $(tasks.test-metadata.results.job-spec) | ||
| volumeMounts: | ||
| - name: shared-data | ||
| mountPath: /shared | ||
| script: | | ||
| #!/usr/bin/env bash | ||
| echo "Downloading rhads-config file:" | ||
| GIT_REPO="$(jq -r '.git.repo // empty' <<< $JOB_SPEC)" | ||
| REPO_ORG=$(jq -r '.git.source_repo_org' <<< $JOB_SPEC) | ||
| BRANCH=$(jq -r '.git.source_repo_branch' <<< $JOB_SPEC) | ||
| rhads_config_file_location="integration-tests/config/rhads-config" | ||
| curl -o rhads-config https://raw.githubusercontent.com/$REPO_ORG/$GIT_REPO/refs/heads/$BRANCH/$rhads_config_file_location | ||
| cat rhads-config | tee $(results.rhads-config.path) | ||
| - name: start-nested-pipelines |
There was a problem hiding this comment.
Raw-GitHub path is very likely wrong – drop the refs/heads/ segment
raw.githubusercontent.com/<org>/<repo>/<ref>/<file> already treats <ref> as a branch name when it isn’t a SHA.
Appending refs/heads/ produces URLs that return 404 for most repositories (e.g.
https://raw.githubusercontent.com/kubernetes/kubernetes/refs/heads/master/README.md).
Both downloads (rhads-config here and the pipeline YAML in the next task) will silently fail and give you an empty file, breaking the pipeline only later.
- curl -o rhads-config https://raw.githubusercontent.com/$REPO_ORG/$GIT_REPO/refs/heads/$BRANCH/$rhads_config_file_location
+ curl -fLo rhads-config \
+ "https://raw.githubusercontent.com/$REPO_ORG/$GIT_REPO/$BRANCH/$rhads_config_file_location"
+
+ # Fail fast if download failed
+ test -s rhads-config || { echo "rhads-config download failed" >&2; exit 1; }📝 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.
| volumes: | |
| - name: shared-data | |
| emptyDir: {} | |
| results: | |
| - name: rhads-config | |
| description: JSON formatted configuration data | |
| steps: | |
| - name: get-rhads-config | |
| image: quay.io/konflux-ci/appstudio-utils:ab6b0b8e40e440158e7288c73aff1cf83a2cc8a9@sha256:24179f0efd06c65d16868c2d7eb82573cce8e43533de6cea14fec3b7446e0b14 | |
| env: | |
| - name: JOB_SPEC | |
| value: $(tasks.test-metadata.results.job-spec) | |
| volumeMounts: | |
| - name: shared-data | |
| mountPath: /shared | |
| script: | | |
| #!/usr/bin/env bash | |
| echo "Downloading rhads-config file:" | |
| GIT_REPO="$(jq -r '.git.repo // empty' <<< $JOB_SPEC)" | |
| REPO_ORG=$(jq -r '.git.source_repo_org' <<< $JOB_SPEC) | |
| BRANCH=$(jq -r '.git.source_repo_branch' <<< $JOB_SPEC) | |
| rhads_config_file_location="integration-tests/config/rhads-config" | |
| curl -o rhads-config https://raw.githubusercontent.com/$REPO_ORG/$GIT_REPO/refs/heads/$BRANCH/$rhads_config_file_location | |
| cat rhads-config | tee $(results.rhads-config.path) | |
| - name: start-nested-pipelines | |
| script: | | |
| #!/usr/bin/env bash | |
| echo "Downloading rhads-config file:" | |
| GIT_REPO="$(jq -r '.git.repo // empty' <<< $JOB_SPEC)" | |
| REPO_ORG=$(jq -r '.git.source_repo_org' <<< $JOB_SPEC) | |
| BRANCH=$(jq -r '.git.source_repo_branch' <<< $JOB_SPEC) | |
| rhads_config_file_location="integration-tests/config/rhads-config" | |
| - curl -o rhads-config https://raw.githubusercontent.com/$REPO_ORG/$GIT_REPO/refs/heads/$BRANCH/$rhads_config_file_location | |
| + curl -fLo rhads-config \ | |
| + "https://raw.githubusercontent.com/$REPO_ORG/$GIT_REPO/$BRANCH/$rhads_config_file_location" | |
| + | |
| + # Fail fast if download failed | |
| + test -s rhads-config || { echo "rhads-config download failed" >&2; exit 1; } | |
| cat rhads-config | tee $(results.rhads-config.path) |
🤖 Prompt for AI Agents
In integration-tests/pipelines/e2e-main-pipeline.yaml around lines 48 to 75, the
URL used to download the rhads-config file incorrectly includes the
'refs/heads/' segment in the raw.githubusercontent.com path, causing 404 errors
and empty downloads. Remove the 'refs/heads/' portion from the URL so that the
branch name is used directly after the repository name in the URL. This will
ensure the curl command fetches the correct file and prevents silent failures in
the pipeline.
| - name: ocp-instance-type | ||
| description: 'The type of machine to use for the cluster nodes.' | ||
| default: "m5.2xlarge" | ||
| type: string | ||
| - name: ocp-replicas | ||
| description: 'The number of replicas for the cluster nodes.' | ||
| default: "3" | ||
| type: string |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using integer-typed params for numeric values
ocp-instance-type is fine as string, but ocp-replicas represents a purely numeric count and is later used as such. Defining it as string invites accidental non-numeric values and prevents simple numeric validation by tooling.
- - name: ocp-replicas
- ...
- default: "3"
- type: string
+ - name: ocp-replicas
+ description: 'Number of worker nodes to create.'
+ default: "3"
+ type: int📝 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.
| - name: ocp-instance-type | |
| description: 'The type of machine to use for the cluster nodes.' | |
| default: "m5.2xlarge" | |
| type: string | |
| - name: ocp-replicas | |
| description: 'The number of replicas for the cluster nodes.' | |
| default: "3" | |
| type: string | |
| - name: ocp-instance-type | |
| description: 'The type of machine to use for the cluster nodes.' | |
| default: "m5.2xlarge" | |
| type: string | |
| - name: ocp-replicas | |
| description: 'Number of worker nodes to create.' | |
| default: "3" | |
| type: int |
🤖 Prompt for AI Agents
In integration-tests/pipelines/e2e-main-pipeline.yaml around lines 22 to 29, the
parameter ocp-replicas is defined as a string but represents a numeric count.
Change the type of ocp-replicas from string to integer to enable proper numeric
validation and prevent accidental non-numeric values.
| PIPELINE_RUN=$(tkn pipeline start -f https://raw.githubusercontent.com/$REPO_ORG/rhtap-cli/refs/heads/$BRANCH/integration-tests/pipelines/rhtap-cli-e2e.yaml \ | ||
| --param ocp-version="$OCP_VERSION"\ | ||
| --param job-spec="$JOB_SPEC"\ | ||
| --param konflux-test-infra-secret="$(params.konflux-test-infra-secret)" \ | ||
| --param rhads-config="$RHADS_CONFIG_CONTENT" \ | ||
| --param cloud-credential-key="$(params.cloud-credential-key)" \ | ||
| --param machine-type="$(params.ocp-instance-type)" \ | ||
| --param replicas="$(params.ocp-replicas)" \ | ||
| $(if [[ "${tssc_image}" != "" ]]; then echo "--param tssc-image=${tssc_image}"; fi) \ |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Same bad raw-GitHub path used for nested pipeline + minor param-name risk
- The
tkn pipeline start -f …URL also containsrefs/heads/; see previous comment – will 404. - Ensure the nested pipeline really defines parameters
machine-typeandreplicas; a typo will maketknprompt and block this step.
- -f https://raw.githubusercontent.com/$REPO_ORG/rhtap-cli/refs/heads/$BRANCH/integration-tests/pipelines/rhtap-cli-e2e.yaml \
+ -f https://raw.githubusercontent.com/$REPO_ORG/rhtap-cli/$BRANCH/integration-tests/pipelines/rhtap-cli-e2e.yaml \I’ll verify the URL patterns and parameter definitions in the nested pipeline before rewriting the comment.
🏁 Script executed:
#!/bin/bash
echo "Searching for raw.githubusercontent.com URLs with refs/heads in integration-tests directory"
rg 'raw\.githubusercontent\.com/.*/refs/heads' -n integration-tests || echo "No refs/heads URLs found"
echo "Locating the nested pipeline file"
PIPELINE_FILE=$(fd -t f 'rhtap-cli-e2e.yaml' integration-tests | head -n 1)
if [ -n "$PIPELINE_FILE" ]; then
echo "Found pipeline file at: $PIPELINE_FILE"
echo "Listing parameter declarations in $PIPELINE_FILE"
grep -n 'params:' -A 20 "$PIPELINE_FILE"
echo "Searching for 'name: machine-type'"
grep -n 'name: *machine-type' "$PIPELINE_FILE" || echo "name: machine-type not found"
echo "Searching for 'name: replicas'"
grep -n 'name: *replicas' "$PIPELINE_FILE" || echo "name: replicas not found"
else
echo "Pipeline file rhtap-cli-e2e.yaml not found in integration-tests"
fiLength of output: 9342
Update raw.githubusercontent.com URLs to remove "refs/heads"
The nested pipeline parameters machine-type and replicas are correctly declared in integration-tests/pipelines/rhtap-cli-e2e.yaml, so no changes there. However, both the curl and the tkn pipeline start commands in integration-tests/pipelines/e2e-main-pipeline.yaml still include refs/heads, which will 404. Please update as follows:
- In
integration-tests/pipelines/e2e-main-pipeline.yaml:- At line 72:
- curl -o rhads-config https://raw.githubusercontent.com/$REPO_ORG/$GIT_REPO/refs/heads/$BRANCH/$rhads_config_file_location + curl -o rhads-config https://raw.githubusercontent.com/$REPO_ORG/$GIT_REPO/$BRANCH/$rhads_config_file_location
- Around lines 167–168:
- -f https://raw.githubusercontent.com/$REPO_ORG/rhtap-cli/refs/heads/$BRANCH/integration-tests/pipelines/rhtap-cli-e2e.yaml \ + -f https://raw.githubusercontent.com/$REPO_ORG/rhtap-cli/$BRANCH/integration-tests/pipelines/rhtap-cli-e2e.yaml \
- At line 72:
📝 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.
| PIPELINE_RUN=$(tkn pipeline start -f https://raw.githubusercontent.com/$REPO_ORG/rhtap-cli/refs/heads/$BRANCH/integration-tests/pipelines/rhtap-cli-e2e.yaml \ | |
| --param ocp-version="$OCP_VERSION"\ | |
| --param job-spec="$JOB_SPEC"\ | |
| --param konflux-test-infra-secret="$(params.konflux-test-infra-secret)" \ | |
| --param rhads-config="$RHADS_CONFIG_CONTENT" \ | |
| --param cloud-credential-key="$(params.cloud-credential-key)" \ | |
| --param machine-type="$(params.ocp-instance-type)" \ | |
| --param replicas="$(params.ocp-replicas)" \ | |
| $(if [[ "${tssc_image}" != "" ]]; then echo "--param tssc-image=${tssc_image}"; fi) \ | |
| PIPELINE_RUN=$(tkn pipeline start -f https://raw.githubusercontent.com/$REPO_ORG/rhtap-cli/$BRANCH/integration-tests/pipelines/rhtap-cli-e2e.yaml \ | |
| --param ocp-version="$OCP_VERSION"\ | |
| --param job-spec="$JOB_SPEC"\ | |
| --param konflux-test-infra-secret="$(params.konflux-test-infra-secret)" \ | |
| --param rhads-config="$RHADS_CONFIG_CONTENT" \ | |
| --param cloud-credential-key="$(params.cloud-credential-key)" \ | |
| --param machine-type="$(params.ocp-instance-type)" \ | |
| --param replicas="$(params.ocp-replicas)" \ | |
| $(if [[ "${tssc_image}" != "" ]]; then echo "--param tssc-image=${tssc_image}"; fi) \ |
| PIPELINE_RUN=$(tkn pipeline start -f https://raw.githubusercontent.com/$REPO_ORG/rhtap-cli/refs/heads/$BRANCH/integration-tests/pipelines/rhtap-cli-e2e.yaml \ | |
| --param ocp-version="$OCP_VERSION"\ | |
| --param job-spec="$JOB_SPEC"\ | |
| --param konflux-test-infra-secret="$(params.konflux-test-infra-secret)" \ | |
| --param rhads-config="$RHADS_CONFIG_CONTENT" \ | |
| --param cloud-credential-key="$(params.cloud-credential-key)" \ | |
| --param machine-type="$(params.ocp-instance-type)" \ | |
| --param replicas="$(params.ocp-replicas)" \ | |
| $(if [[ "${tssc_image}" != "" ]]; then echo "--param tssc-image=${tssc_image}"; fi) \ | |
| curl -o rhads-config https://raw.githubusercontent.com/$REPO_ORG/$GIT_REPO/$BRANCH/$rhads_config_file_location |
🤖 Prompt for AI Agents
In integration-tests/pipelines/e2e-main-pipeline.yaml around lines 167 to 175,
the URL used in the tkn pipeline start command includes "refs/heads" which
causes a 404 error when accessing raw.githubusercontent.com. Remove "refs/heads"
from the URL path so it correctly points to the branch file location. Make the
same update at line 72 as well to ensure all raw.githubusercontent.com URLs are
correct.
|
@coderabbitai[bot]: changing LGTM is restricted to collaborators DetailsIn response to this:
Instructions 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. |
|
Skipped: Cannot respond to another bot. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jkopriva, xinredhat The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
bot
merged commit a582af2
into
redhat-appstudio:main
2 participants
rh-pre-commit.version: 2.3.2
rh-pre-commit.check-secrets: ENABLED
Summary by CodeRabbit
New Features
Improvements
Changes