Adding Claude Subagent for creating release jobs#81406
Open
shyadav10 wants to merge 1 commit into
Open
Conversation
Contributor
WalkthroughAdds documentation (AGENTS.md and three agent spec files) and three corresponding Bash scripts for the ocp-qe-perfscale-ci config directory, automating creation of versioned control-plane, loaded-upgrade, and node-density-heavy Prow CI job configurations for new OpenShift releases. ChangesAgents and Job Generation Scripts
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Agent as Job Creation Agent
participant Script as Job Creation Script
participant Repo as Release Repo
User->>Agent: Request jobs for target OpenShift version
Agent->>Script: Invoke corresponding create_*_jobs.sh TARGET_VERSION
Script->>Script: Derive prior version(s) and filenames
Script->>Repo: Verify source YAML exists
Script->>Repo: Prompt overwrite if target YAML exists
Script->>Repo: Generate target YAML via sed substitutions
Script->>Repo: Run make jobs
Script-->>Agent: Print created file path and verification checklist
Agent-->>User: Report completion and next steps
Related PRs: None found. Suggested labels: documentation, ci-operator Suggested reviewers: None found. Poem: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: shyadav10 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 |
openshift-merge-bot
Bot
added
the
rehearsals-ack
Contributor
|
[REHEARSALNOTIFIER] Note: If this PR includes changes to step registry files ( |
Contributor
|
@shyadav10: 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. |
![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: 4
🧹 Nitpick comments (3)
ci-operator/config/openshift-eng/ocp-qe-perfscale-ci/scripts/create_node_density_heavy_jobs.sh (1)
60-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog message doesn't match the command run.
Line 60 says
"Running 'make update' to generate Prow jobs..."but line 65 invokesmake jobs, notmake update. The sibling scripts correctly log "make jobs".✏️ Proposed fix
-echo "Running 'make update' to generate Prow jobs..." +echo "Running 'make jobs' to generate Prow jobs..."🤖 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-eng/ocp-qe-perfscale-ci/scripts/create_node_density_heavy_jobs.sh` around lines 60 - 65, The log message in create_node_density_heavy_jobs.sh is inconsistent with the command executed: the script prints that it is running make update, but the create_node_density_heavy_jobs.sh flow actually calls make jobs. Update the echo text near the make jobs invocation to match the real command, and keep it aligned with the sibling job-generation scripts for consistency.ci-operator/config/openshift-eng/ocp-qe-perfscale-ci/scripts/create_control_plane_jobs.sh (1)
36-45: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider a non-interactive override for automated agent invocation.
AGENTS.md documents Claude invoking this script automatically. If
$TARGET_FILEalready exists, the script blocks on an interactivey/nprompt, which won't behave predictably without a TTY (may abort silently rather than overwrite). A--force/-yflag or env var would make this safer for automation.🤖 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-eng/ocp-qe-perfscale-ci/scripts/create_control_plane_jobs.sh` around lines 36 - 45, Add a non-interactive override to the TARGET_FILE existence check in create_control_plane_jobs.sh so automated callers don’t hit the y/n prompt. Update the script’s argument handling and overwrite branch to support a flag or environment variable (for example, a force mode) that skips the read prompt, while keeping the current interactive confirmation as the default behavior when the override is not set.ci-operator/config/openshift-eng/ocp-qe-perfscale-ci/AGENTS.md (1)
13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd language identifiers to fenced code blocks.
Static analysis flags these as plain code fences without a language (MD040). Low-cost markdown polish.
Also applies to: 36-36, 59-59
🤖 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-eng/ocp-qe-perfscale-ci/AGENTS.md` at line 13, The markdown fences in this document are missing language identifiers, triggering MD040. Update each fenced code block to include the appropriate language tag (for example, the existing shell or text context) in the relevant markdown sections. Check all affected fences in the document, including the ones referenced by the review, and keep the content unchanged aside from adding the language labels.Source: Linters/SAST tools
🤖 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/config/openshift-eng/ocp-qe-perfscale-ci/agents/create_node_density_heavy_jobs.md`:
- Around line 44-49: The Step 2 command in create_node_density_heavy_jobs.md
incorrectly uses cd with the script path, so fix the “Run the Creation Script”
instructions to match the sibling agent docs’ two-step pattern: first change
into the ci-operator/config/openshift-eng/ocp-qe-perfscale-ci directory, then
run scripts/create_node_density_heavy_jobs.sh with <TARGET_VERSION>. Use the
existing Step 2 section and the create_node_density_heavy_jobs.sh reference to
update the command text consistently.
In
`@ci-operator/config/openshift-eng/ocp-qe-perfscale-ci/scripts/create_control_plane_jobs.sh`:
- Around line 47-67: The create_control_plane_jobs.sh helper generates the new
YAML file and then calls make jobs without validating the output first. Add a
YAML syntax validation step after the sed-generated TARGET_FILE is written and
before the make jobs invocation, using the script’s existing flow around
TARGET_FILE, SCRIPT_DIR, and RELEASE_ROOT. If validation fails, stop the script
with a clear error instead of proceeding to make jobs.
In
`@ci-operator/config/openshift-eng/ocp-qe-perfscale-ci/scripts/create_loaded_upgrade_jobs.sh`:
- Around line 53-80: The create_loaded_upgrade_jobs.sh helper is generating
$TARGET_FILE without validating that the YAML is syntactically correct before
calling make jobs. Add a YAML syntax check for the generated file after the
sed-based creation and before cd into the release repo / running make jobs,
using the same validation pattern as the sibling scripts so the script fails
fast on invalid output.
In
`@ci-operator/config/openshift-eng/ocp-qe-perfscale-ci/scripts/create_node_density_heavy_jobs.sh`:
- Around line 47-65: The generated YAML in create_node_density_heavy_jobs.sh is
never syntax-checked before make jobs runs. After creating $TARGET_FILE with sed
in the script’s main flow, add a YAML validation step for that file and fail
fast if it is invalid, then only proceed to cd into RELEASE_ROOT and run make
jobs. Use the existing create-node-density-heavy workflow and the $TARGET_FILE
variable as the insertion point.
---
Nitpick comments:
In `@ci-operator/config/openshift-eng/ocp-qe-perfscale-ci/AGENTS.md`:
- Line 13: The markdown fences in this document are missing language
identifiers, triggering MD040. Update each fenced code block to include the
appropriate language tag (for example, the existing shell or text context) in
the relevant markdown sections. Check all affected fences in the document,
including the ones referenced by the review, and keep the content unchanged
aside from adding the language labels.
In
`@ci-operator/config/openshift-eng/ocp-qe-perfscale-ci/scripts/create_control_plane_jobs.sh`:
- Around line 36-45: Add a non-interactive override to the TARGET_FILE existence
check in create_control_plane_jobs.sh so automated callers don’t hit the y/n
prompt. Update the script’s argument handling and overwrite branch to support a
flag or environment variable (for example, a force mode) that skips the read
prompt, while keeping the current interactive confirmation as the default
behavior when the override is not set.
In
`@ci-operator/config/openshift-eng/ocp-qe-perfscale-ci/scripts/create_node_density_heavy_jobs.sh`:
- Around line 60-65: The log message in create_node_density_heavy_jobs.sh is
inconsistent with the command executed: the script prints that it is running
make update, but the create_node_density_heavy_jobs.sh flow actually calls make
jobs. Update the echo text near the make jobs invocation to match the real
command, and keep it aligned with the sibling job-generation scripts for
consistency.
🪄 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: d868df04-6272-471c-98e8-de0e1fdf018d
📒 Files selected for processing (7)
ci-operator/config/openshift-eng/ocp-qe-perfscale-ci/AGENTS.mdci-operator/config/openshift-eng/ocp-qe-perfscale-ci/agents/create_control_plane_jobs.mdci-operator/config/openshift-eng/ocp-qe-perfscale-ci/agents/create_loaded_upgrade_jobs.mdci-operator/config/openshift-eng/ocp-qe-perfscale-ci/agents/create_node_density_heavy_jobs.mdci-operator/config/openshift-eng/ocp-qe-perfscale-ci/scripts/create_control_plane_jobs.shci-operator/config/openshift-eng/ocp-qe-perfscale-ci/scripts/create_loaded_upgrade_jobs.shci-operator/config/openshift-eng/ocp-qe-perfscale-ci/scripts/create_node_density_heavy_jobs.sh
Comment on lines
+44
to
+49
| ### Step 2: Run the Creation Script | ||
| Use the automated script to create the configuration file: | ||
|
|
||
| ```bash | ||
| cd ci-operator/config/openshift-eng/ocp-qe-perfscale-ci/scripts/create_node_density_heavy_jobs.sh <TARGET_VERSION> | ||
| ``` |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Broken command in Step 2.
Unlike the control-plane and loaded-upgrade agent docs, this step incorrectly merges directory navigation and script execution into a single cd invocation, which will fail (cd cannot target a script file). Compare with the sibling docs' two-line pattern.
🛠️ Proposed fix
```bash
-cd ci-operator/config/openshift-eng/ocp-qe-perfscale-ci/scripts/create_node_density_heavy_jobs.sh <TARGET_VERSION>
+cd ci-operator/config/openshift-eng/ocp-qe-perfscale-ci
+./scripts/create_node_density_heavy_jobs.sh <TARGET_VERSION></details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🤖 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-eng/ocp-qe-perfscale-ci/agents/create_node_density_heavy_jobs.md`
around lines 44 - 49, The Step 2 command in create_node_density_heavy_jobs.md
incorrectly uses cd with the script path, so fix the “Run the Creation Script”
instructions to match the sibling agent docs’ two-step pattern: first change
into the ci-operator/config/openshift-eng/ocp-qe-perfscale-ci directory, then
run scripts/create_node_density_heavy_jobs.sh with <TARGET_VERSION>. Use the
existing Step 2 section and the create_node_density_heavy_jobs.sh reference to
update the command text consistently.
Comment on lines
+47
to
+67
| # Create the new file with version replacements | ||
| sed -e "s/\"${PRIOR_VERSION}\"/\"${TARGET_VERSION}\"/g" \ | ||
| -e "s/variant: ${PRIOR_VERSION}-nightly-control-plane/variant: ${TARGET_VERSION}-nightly-control-plane/g" \ | ||
| "$SOURCE_FILE" > "$TARGET_FILE" | ||
|
|
||
| echo "Created: $TARGET_FILE" | ||
| echo "" | ||
| echo "Please verify the following in the new file:" | ||
| echo " - base_images.upi-installer.name is \"${TARGET_VERSION}\"" | ||
| echo " - All releases.*.version are \"${TARGET_VERSION}\"" | ||
| echo " - zz_generated_metadata.variant is ${TARGET_VERSION}-nightly-control-plane" | ||
| echo " - Review cron schedules (they may need manual adjustment to avoid conflicts)" | ||
| echo "" | ||
|
|
||
| # Navigate to release repo root and run make update | ||
| echo "Running 'make jobs' to generate Prow jobs..." | ||
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
| RELEASE_ROOT="$(cd "${SCRIPT_DIR}/../../../../.." && pwd)" | ||
|
|
||
| cd "${RELEASE_ROOT}" | ||
| make jobs |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add YAML syntax validation before running make jobs.
Per repository guidelines, helper scripts here must validate YAML syntax before invoking make jobs. This script generates $TARGET_FILE via sed and immediately runs make jobs without any syntax check.
As per coding guidelines, ci-operator/config/openshift-eng/ocp-qe-perfscale-ci/scripts/*.sh: "All helper scripts must validate YAML syntax before running make jobs."
🛠️ Proposed fix
sed -e "s/\"${PRIOR_VERSION}\"/\"${TARGET_VERSION}\"/g" \
-e "s/variant: ${PRIOR_VERSION}-nightly-control-plane/variant: ${TARGET_VERSION}-nightly-control-plane/g" \
"$SOURCE_FILE" > "$TARGET_FILE"
+# Validate YAML syntax before proceeding
+if ! python3 -c "import yaml, sys; yaml.safe_load(open(sys.argv[1]))" "$TARGET_FILE"; then
+ echo "Error: Generated file is not valid YAML: $TARGET_FILE"
+ exit 1
+fi
+
echo "Created: $TARGET_FILE"📝 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
| # Create the new file with version replacements | |
| sed -e "s/\"${PRIOR_VERSION}\"/\"${TARGET_VERSION}\"/g" \ | |
| -e "s/variant: ${PRIOR_VERSION}-nightly-control-plane/variant: ${TARGET_VERSION}-nightly-control-plane/g" \ | |
| "$SOURCE_FILE" > "$TARGET_FILE" | |
| echo "Created: $TARGET_FILE" | |
| echo "" | |
| echo "Please verify the following in the new file:" | |
| echo " - base_images.upi-installer.name is \"${TARGET_VERSION}\"" | |
| echo " - All releases.*.version are \"${TARGET_VERSION}\"" | |
| echo " - zz_generated_metadata.variant is ${TARGET_VERSION}-nightly-control-plane" | |
| echo " - Review cron schedules (they may need manual adjustment to avoid conflicts)" | |
| echo "" | |
| # Navigate to release repo root and run make update | |
| echo "Running 'make jobs' to generate Prow jobs..." | |
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | |
| RELEASE_ROOT="$(cd "${SCRIPT_DIR}/../../../../.." && pwd)" | |
| cd "${RELEASE_ROOT}" | |
| make jobs | |
| # Create the new file with version replacements | |
| sed -e "s/\"${PRIOR_VERSION}\"/\"${TARGET_VERSION}\"/g" \ | |
| -e "s/variant: ${PRIOR_VERSION}-nightly-control-plane/variant: ${TARGET_VERSION}-nightly-control-plane/g" \ | |
| "$SOURCE_FILE" > "$TARGET_FILE" | |
| # Validate YAML syntax before proceeding | |
| if ! python3 -c "import yaml, sys; yaml.safe_load(open(sys.argv[1]))" "$TARGET_FILE"; then | |
| echo "Error: Generated file is not valid YAML: $TARGET_FILE" | |
| exit 1 | |
| fi | |
| echo "Created: $TARGET_FILE" | |
| echo "" | |
| echo "Please verify the following in the new file:" | |
| echo " - base_images.upi-installer.name is \"${TARGET_VERSION}\"" | |
| echo " - All releases.*.version are \"${TARGET_VERSION}\"" | |
| echo " - zz_generated_metadata.variant is ${TARGET_VERSION}-nightly-control-plane" | |
| echo " - Review cron schedules (they may need manual adjustment to avoid conflicts)" | |
| echo "" | |
| # Navigate to release repo root and run make update | |
| echo "Running 'make jobs' to generate Prow jobs..." | |
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | |
| RELEASE_ROOT="$(cd "${SCRIPT_DIR}/../../../../.." && pwd)" | |
| cd "${RELEASE_ROOT}" | |
| make jobs |
🤖 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-eng/ocp-qe-perfscale-ci/scripts/create_control_plane_jobs.sh`
around lines 47 - 67, The create_control_plane_jobs.sh helper generates the new
YAML file and then calls make jobs without validating the output first. Add a
YAML syntax validation step after the sed-generated TARGET_FILE is written and
before the make jobs invocation, using the script’s existing flow around
TARGET_FILE, SCRIPT_DIR, and RELEASE_ROOT. If validation fails, stop the script
with a clear error instead of proceeding to make jobs.
Source: Coding guidelines
Comment on lines
+53
to
+80
| # Create the new file with version replacements | ||
| # Need to update: | ||
| # 1. Initial version bounds (lower: 4.22.0-0 -> 4.23.0-0, upper: 4.23.0-0 -> 4.24.0-0) | ||
| # 2. Latest version (4.23 -> 4.24) | ||
| # 3. Variant in metadata | ||
| sed -e "s/lower: ${PRIOR_PRIOR_VERSION}.0-0/lower: ${PRIOR_VERSION}.0-0/g" \ | ||
| -e "s/upper: ${PRIOR_VERSION}.0-0/upper: ${TARGET_VERSION}.0-0/g" \ | ||
| -e "s/version: \"${PRIOR_VERSION}\"/version: \"${TARGET_VERSION}\"/g" \ | ||
| -e "s/variant: aws-${PRIOR_VERSION}-nightly-x86-loaded-upgrade-from-${PRIOR_PRIOR_VERSION}/variant: aws-${TARGET_VERSION}-nightly-x86-loaded-upgrade-from-${PRIOR_VERSION}/g" \ | ||
| "$SOURCE_FILE" > "$TARGET_FILE" | ||
|
|
||
| echo "Created: $TARGET_FILE" | ||
| echo "" | ||
| echo "Please verify the following in the new file:" | ||
| echo " - releases.initial.version_bounds.lower is ${PRIOR_VERSION}.0-0" | ||
| echo " - releases.initial.version_bounds.upper is ${TARGET_VERSION}.0-0" | ||
| echo " - releases.latest.version is \"${TARGET_VERSION}\"" | ||
| echo " - zz_generated_metadata.variant is aws-${TARGET_VERSION}-nightly-x86-loaded-upgrade-from-${PRIOR_VERSION}" | ||
| echo " - Review cron schedules (they may need manual adjustment to avoid conflicts)" | ||
| echo "" | ||
|
|
||
| # Navigate to release repo root and run make update | ||
| echo "Running 'make jobs' to generate Prow jobs..." | ||
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
| RELEASE_ROOT="$(cd "${SCRIPT_DIR}/../../../../.." && pwd)" | ||
|
|
||
| cd "${RELEASE_ROOT}" | ||
| make jobs |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add YAML syntax validation before running make jobs.
Same gap as the sibling scripts — the generated $TARGET_FILE (line 62) is never syntax-checked before make jobs runs.
As per coding guidelines, ci-operator/config/openshift-eng/ocp-qe-perfscale-ci/scripts/*.sh: "All helper scripts must validate YAML syntax before running make jobs."
🛠️ Proposed fix
"$SOURCE_FILE" > "$TARGET_FILE"
+# Validate YAML syntax before proceeding
+if ! python3 -c "import yaml, sys; yaml.safe_load(open(sys.argv[1]))" "$TARGET_FILE"; then
+ echo "Error: Generated file is not valid YAML: $TARGET_FILE"
+ exit 1
+fi
+
echo "Created: $TARGET_FILE"📝 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
| # Create the new file with version replacements | |
| # Need to update: | |
| # 1. Initial version bounds (lower: 4.22.0-0 -> 4.23.0-0, upper: 4.23.0-0 -> 4.24.0-0) | |
| # 2. Latest version (4.23 -> 4.24) | |
| # 3. Variant in metadata | |
| sed -e "s/lower: ${PRIOR_PRIOR_VERSION}.0-0/lower: ${PRIOR_VERSION}.0-0/g" \ | |
| -e "s/upper: ${PRIOR_VERSION}.0-0/upper: ${TARGET_VERSION}.0-0/g" \ | |
| -e "s/version: \"${PRIOR_VERSION}\"/version: \"${TARGET_VERSION}\"/g" \ | |
| -e "s/variant: aws-${PRIOR_VERSION}-nightly-x86-loaded-upgrade-from-${PRIOR_PRIOR_VERSION}/variant: aws-${TARGET_VERSION}-nightly-x86-loaded-upgrade-from-${PRIOR_VERSION}/g" \ | |
| "$SOURCE_FILE" > "$TARGET_FILE" | |
| echo "Created: $TARGET_FILE" | |
| echo "" | |
| echo "Please verify the following in the new file:" | |
| echo " - releases.initial.version_bounds.lower is ${PRIOR_VERSION}.0-0" | |
| echo " - releases.initial.version_bounds.upper is ${TARGET_VERSION}.0-0" | |
| echo " - releases.latest.version is \"${TARGET_VERSION}\"" | |
| echo " - zz_generated_metadata.variant is aws-${TARGET_VERSION}-nightly-x86-loaded-upgrade-from-${PRIOR_VERSION}" | |
| echo " - Review cron schedules (they may need manual adjustment to avoid conflicts)" | |
| echo "" | |
| # Navigate to release repo root and run make update | |
| echo "Running 'make jobs' to generate Prow jobs..." | |
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | |
| RELEASE_ROOT="$(cd "${SCRIPT_DIR}/../../../../.." && pwd)" | |
| cd "${RELEASE_ROOT}" | |
| make jobs | |
| # Create the new file with version replacements | |
| # Need to update: | |
| # 1. Initial version bounds (lower: 4.22.0-0 -> 4.23.0-0, upper: 4.23.0-0 -> 4.24.0-0) | |
| # 2. Latest version (4.23 -> 4.24) | |
| # 3. Variant in metadata | |
| sed -e "s/lower: ${PRIOR_PRIOR_VERSION}.0-0/lower: ${PRIOR_VERSION}.0-0/g" \ | |
| -e "s/upper: ${PRIOR_VERSION}.0-0/upper: ${TARGET_VERSION}.0-0/g" \ | |
| -e "s/version: \"${PRIOR_VERSION}\"/version: \"${TARGET_VERSION}\"/g" \ | |
| -e "s/variant: aws-${PRIOR_VERSION}-nightly-x86-loaded-upgrade-from-${PRIOR_PRIOR_VERSION}/variant: aws-${TARGET_VERSION}-nightly-x86-loaded-upgrade-from-${PRIOR_VERSION}/g" \ | |
| "$SOURCE_FILE" > "$TARGET_FILE" | |
| # Validate YAML syntax before proceeding | |
| if ! python3 -c "import yaml, sys; yaml.safe_load(open(sys.argv[1]))" "$TARGET_FILE"; then | |
| echo "Error: Generated file is not valid YAML: $TARGET_FILE" | |
| exit 1 | |
| fi | |
| echo "Created: $TARGET_FILE" | |
| echo "" | |
| echo "Please verify the following in the new file:" | |
| echo " - releases.initial.version_bounds.lower is ${PRIOR_VERSION}.0-0" | |
| echo " - releases.initial.version_bounds.upper is ${TARGET_VERSION}.0-0" | |
| echo " - releases.latest.version is \"${TARGET_VERSION}\"" | |
| echo " - zz_generated_metadata.variant is aws-${TARGET_VERSION}-nightly-x86-loaded-upgrade-from-${PRIOR_VERSION}" | |
| echo " - Review cron schedules (they may need manual adjustment to avoid conflicts)" | |
| echo "" | |
| # Navigate to release repo root and run make update | |
| echo "Running 'make jobs' to generate Prow jobs..." | |
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | |
| RELEASE_ROOT="$(cd "${SCRIPT_DIR}/../../../../.." && pwd)" | |
| cd "${RELEASE_ROOT}" | |
| make jobs |
🤖 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-eng/ocp-qe-perfscale-ci/scripts/create_loaded_upgrade_jobs.sh`
around lines 53 - 80, The create_loaded_upgrade_jobs.sh helper is generating
$TARGET_FILE without validating that the YAML is syntactically correct before
calling make jobs. Add a YAML syntax check for the generated file after the
sed-based creation and before cd into the release repo / running make jobs,
using the same validation pattern as the sibling scripts so the script fails
fast on invalid output.
Source: Coding guidelines
Comment on lines
+47
to
+65
| # Create the new file with version replacements | ||
| sed -e "s/\"${PRIOR_VERSION}\"/\"${TARGET_VERSION}\"/g" -e "s/variant: ${PRIOR_VERSION}-nightly-node-density-heavy/variant: ${TARGET_VERSION}-nightly-node-density-heavy/g" "$SOURCE_FILE" > "$TARGET_FILE" | ||
|
|
||
| echo "Created: $TARGET_FILE" | ||
| echo "" | ||
| echo "Please verify the following in the new file:" | ||
| echo " - base_images.upi-installer.name is \"${TARGET_VERSION}\"" | ||
| echo " - All releases.*.version are \"${TARGET_VERSION}\"" | ||
| echo " - zz_generated_metadata.variant is ${TARGET_VERSION}-nightly-node-density-heavy" | ||
| echo " - Review cron schedules (they may need manual adjustment to avoid conflicts)" | ||
| echo "" | ||
|
|
||
| # Navigate to release repo root and run make update | ||
| echo "Running 'make update' to generate Prow jobs..." | ||
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
| RELEASE_ROOT="$(cd "${SCRIPT_DIR}/../../../../.." && pwd)" | ||
|
|
||
| cd "${RELEASE_ROOT}" | ||
| make jobs |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add YAML syntax validation before running make jobs.
Same gap as the other two scripts — $TARGET_FILE (line 48) is generated via sed and never validated before make jobs runs.
As per coding guidelines, ci-operator/config/openshift-eng/ocp-qe-perfscale-ci/scripts/*.sh: "All helper scripts must validate YAML syntax before running make jobs."
🛠️ Proposed fix
sed -e "s/\"${PRIOR_VERSION}\"/\"${TARGET_VERSION}\"/g" -e "s/variant: ${PRIOR_VERSION}-nightly-node-density-heavy/variant: ${TARGET_VERSION}-nightly-node-density-heavy/g" "$SOURCE_FILE" > "$TARGET_FILE"
+# Validate YAML syntax before proceeding
+if ! python3 -c "import yaml, sys; yaml.safe_load(open(sys.argv[1]))" "$TARGET_FILE"; then
+ echo "Error: Generated file is not valid YAML: $TARGET_FILE"
+ exit 1
+fi
+
echo "Created: $TARGET_FILE"📝 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
| # Create the new file with version replacements | |
| sed -e "s/\"${PRIOR_VERSION}\"/\"${TARGET_VERSION}\"/g" -e "s/variant: ${PRIOR_VERSION}-nightly-node-density-heavy/variant: ${TARGET_VERSION}-nightly-node-density-heavy/g" "$SOURCE_FILE" > "$TARGET_FILE" | |
| echo "Created: $TARGET_FILE" | |
| echo "" | |
| echo "Please verify the following in the new file:" | |
| echo " - base_images.upi-installer.name is \"${TARGET_VERSION}\"" | |
| echo " - All releases.*.version are \"${TARGET_VERSION}\"" | |
| echo " - zz_generated_metadata.variant is ${TARGET_VERSION}-nightly-node-density-heavy" | |
| echo " - Review cron schedules (they may need manual adjustment to avoid conflicts)" | |
| echo "" | |
| # Navigate to release repo root and run make update | |
| echo "Running 'make update' to generate Prow jobs..." | |
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | |
| RELEASE_ROOT="$(cd "${SCRIPT_DIR}/../../../../.." && pwd)" | |
| cd "${RELEASE_ROOT}" | |
| make jobs | |
| # Create the new file with version replacements | |
| sed -e "s/\"${PRIOR_VERSION}\"/\"${TARGET_VERSION}\"/g" -e "s/variant: ${PRIOR_VERSION}-nightly-node-density-heavy/variant: ${TARGET_VERSION}-nightly-node-density-heavy/g" "$SOURCE_FILE" > "$TARGET_FILE" | |
| # Validate YAML syntax before proceeding | |
| if ! python3 -c "import yaml, sys; yaml.safe_load(open(sys.argv[1]))" "$TARGET_FILE"; then | |
| echo "Error: Generated file is not valid YAML: $TARGET_FILE" | |
| exit 1 | |
| fi | |
| echo "Created: $TARGET_FILE" | |
| echo "" | |
| echo "Please verify the following in the new file:" | |
| echo " - base_images.upi-installer.name is \"${TARGET_VERSION}\"" | |
| echo " - All releases.*.version are \"${TARGET_VERSION}\"" | |
| echo " - zz_generated_metadata.variant is ${TARGET_VERSION}-nightly-node-density-heavy" | |
| echo " - Review cron schedules (they may need manual adjustment to avoid conflicts)" | |
| echo "" | |
| # Navigate to release repo root and run make update | |
| echo "Running 'make update' to generate Prow jobs..." | |
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | |
| RELEASE_ROOT="$(cd "${SCRIPT_DIR}/../../../../.." && pwd)" | |
| cd "${RELEASE_ROOT}" | |
| make jobs |
🤖 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-eng/ocp-qe-perfscale-ci/scripts/create_node_density_heavy_jobs.sh`
around lines 47 - 65, The generated YAML in create_node_density_heavy_jobs.sh is
never syntax-checked before make jobs runs. After creating $TARGET_FILE with sed
in the script’s main flow, add a YAML validation step for that file and fail
fast if it is invalid, then only proceed to cd into RELEASE_ROOT and run make
jobs. Use the existing create-node-density-heavy workflow and the $TARGET_FILE
variable as the insertion point.
Source: Coding guidelines
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
1 participant
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.
Summary by CodeRabbit
Added Claude Code subagent support for the
ocp-qe-perfscale-ciCI config so release job generation can be handled through dedicated agent workflows instead of manual scripting.In practical terms, this updates the OpenShift release CI configuration for
openshift-eng/ocp-qe-perfscale-ciby:make jobsThis should make it easier and more consistent to produce new CI job definitions for upcoming OpenShift releases in this repository.