SREP-4236: Update pr_check.sh to run without container engine#484
Conversation
|
@reedcort: This pull request references SREP-4236 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
added
the
jira/valid-reference
WalkthroughIntroduces a new Bash script Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ 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 |
reedcort
force-pushed
the
SREP-4236-prow-pr-check
branch
from
c02f38d to
2244b0f
Compare
|
@reedcort: This pull request references SREP-4236 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@build/prow_pr_check.sh`:
- Around line 24-26: The current check uses git diff --exit-code which misses
untracked generated files; replace the check around git diff --exit-code with a
two-part verification: run git diff --exit-code and then run git status
--porcelain --untracked-files=all (or git ls-files --others --exclude-standard)
and fail if either reports changes, and update the failure message printed by
the echo that currently starts "FAILURE: unexpected changes after building..."
to instruct the user to regenerate artifacts and commit them (e.g., "Run 'make
syncset package' or 'make generate' to regenerate artifacts, add and commit the
changes.") — locate and modify the git diff --exit-code check and the associated
echo/exit 1 lines in build/prow_pr_check.sh.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 49b811ae-d8ba-49af-aaad-ad5e32b20351
📒 Files selected for processing (1)
build/prow_pr_check.sh
| if ! git diff --exit-code; then | ||
| echo "FAILURE: unexpected changes after building. Run 'make syncset package' and commit changes." | ||
| exit 1 |
There was a problem hiding this comment.
git diff --exit-code misses untracked generated files
Line 24 only checks tracked-file diffs, so newly generated untracked files can slip through. That weakens the “generated artifacts are committed” gate. Also, Line 25’s remediation message still points to container-based targets.
Suggested fix
-if ! git diff --exit-code; then
- echo "FAILURE: unexpected changes after building. Run 'make syncset package' and commit changes."
+if [[ -n "$(git status --porcelain --untracked-files=all)" ]]; then
+ git status --short
+ echo "FAILURE: unexpected changes after building. Regenerate resources and commit all tracked/untracked changes."
exit 1
fi📝 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.
| if ! git diff --exit-code; then | |
| echo "FAILURE: unexpected changes after building. Run 'make syncset package' and commit changes." | |
| exit 1 | |
| if [[ -n "$(git status --porcelain --untracked-files=all)" ]]; then | |
| git status --short | |
| echo "FAILURE: unexpected changes after building. Regenerate resources and commit all tracked/untracked changes." | |
| exit 1 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@build/prow_pr_check.sh` around lines 24 - 26, The current check uses git diff
--exit-code which misses untracked generated files; replace the check around git
diff --exit-code with a two-part verification: run git diff --exit-code and then
run git status --porcelain --untracked-files=all (or git ls-files --others
--exclude-standard) and fail if either reports changes, and update the failure
message printed by the echo that currently starts "FAILURE: unexpected changes
after building..." to instruct the user to regenerate artifacts and commit them
(e.g., "Run 'make syncset package' or 'make generate' to regenerate artifacts,
add and commit the changes.") — locate and modify the git diff --exit-code check
and the associated echo/exit 1 lines in build/prow_pr_check.sh.
2244b0f to
dcf1392
Compare
Add a new script that runs tests and resource generation directly without a container engine, enabling the pr-check to run in Prow. The existing pr_check.sh is left intact for Jenkins until it is decommissioned. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
reedcort
force-pushed
the
SREP-4236-prow-pr-check
branch
from
dcf1392 to
9cadbe5
Compare
|
/lgtm |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
hack/prow_pr_check.sh (1)
24-26:⚠️ Potential issue | 🟠 MajorHarden the dirty-tree check to include untracked generated files.
Line 24 only checks tracked diffs, so newly generated untracked artifacts can pass this gate.
Suggested fix
-if ! git diff --exit-code; then - echo "FAILURE: unexpected changes after building. Run 'make syncset package' and commit changes." +if [[ -n "$(git status --porcelain --untracked-files=all)" ]]; then + git status --short + echo "FAILURE: generated artifacts are out of date. Regenerate resources and commit all tracked/untracked changes." exit 1 fi#!/bin/bash # Compare tracked-only vs full working-tree checks git diff --exit-code >/dev/null; echo "git diff exit code: $?" git status --porcelain --untracked-files=allAs per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hack/prow_pr_check.sh` around lines 24 - 26, The current check uses "git diff --exit-code" which only detects changes to tracked files; update the dirty-tree gate to detect untracked generated files as well by running a full working-tree check such as "git status --porcelain --untracked-files=all" (or "git diff --exit-code" combined with "git ls-files --others --exclude-standard") and fail when its output is non-empty; modify the script location where "git diff --exit-code" is invoked to replace or augment that command and ensure the script echoes the same failure message and exits with code 1 when untracked or modified files are present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@hack/prow_pr_check.sh`:
- Around line 24-26: The current check uses "git diff --exit-code" which only
detects changes to tracked files; update the dirty-tree gate to detect untracked
generated files as well by running a full working-tree check such as "git status
--porcelain --untracked-files=all" (or "git diff --exit-code" combined with "git
ls-files --others --exclude-standard") and fail when its output is non-empty;
modify the script location where "git diff --exit-code" is invoked to replace or
augment that command and ensure the script echoes the same failure message and
exits with code 1 when untracked or modified files are present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 55dad67e-ddf1-4bb0-a74f-6523d08c5736
📒 Files selected for processing (1)
hack/prow_pr_check.sh
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: joshbranham, reedcort 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-ci
bot
added
the
approved
|
@reedcort: all tests passed! 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. |
3 participants
Summary
build/prow_pr_check.shthat runs tests and resource generation directly without a container enginebuild/pr_check.shintact for Jenkins until it is decommissionedContext
This is part of migrating the pr-check from app-interface Jenkins to Prow (SREP-4236). A corresponding PR to openshift/release will add the Prow presubmit job configuration.
Test plan
build/prow_pr_check.shruns successfully in a Go environment without a container engine🤖 Generated with Claude Code