USHIFT-6967: MicroShift CI Doctor - Test workflow-based analysis in Prow environment#80503
USHIFT-6967: MicroShift CI Doctor - Test workflow-based analysis in Prow environment#80503ggiguash wants to merge 2 commits into
Conversation
|
Skipping CI for Draft Pull Request. |
|
@ggiguash: This pull request references USHIFT-6967 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 bug to target the "5.0.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
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe doctor CI script transitions from relying on the pre-installed ChangesDoctor Script Directory and Tool Updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 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)
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 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ggiguash 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
|
/pj-rehearse periodic-ci-openshift-eng-edge-tooling-main-microshift-ci-doctor |
|
@ggiguash: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
![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
🧹 Nitpick comments (1)
ci-operator/step-registry/openshift/edge-tooling/microshift-ci/doctor/openshift-edge-tooling-microshift-ci-doctor-commands.sh (1)
246-248: 💤 Low valueRemove commented code before merging to production.
The commented-out lines provide useful context during development but should be removed before this PR is finalized and merged to avoid clutter.
🤖 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/openshift/edge-tooling/microshift-ci/doctor/openshift-edge-tooling-microshift-ci-doctor-commands.sh` around lines 246 - 248, Remove the three commented-out lines that set SRC_DIR to EDGE_TOOLING_DIR, PLUGIN_DIR to the microshift-ci plugin path, and change directory to SRC_DIR. These lines provide no functional value and should be deleted entirely rather than kept as comments to reduce code clutter.
🤖 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/openshift/edge-tooling/microshift-ci/doctor/openshift-edge-tooling-microshift-ci-doctor-commands.sh`:
- Around line 250-252: The git clone command will fail if the SRC_DIR directory
already exists from a prior run. Add a cleanup command to remove the SRC_DIR
directory before attempting the git clone. Place a `rm -rf "${SRC_DIR}"`
statement immediately before the `git clone` line to ensure the destination path
does not exist when the clone operation begins, preventing the "destination path
already exists" error.
---
Nitpick comments:
In
`@ci-operator/step-registry/openshift/edge-tooling/microshift-ci/doctor/openshift-edge-tooling-microshift-ci-doctor-commands.sh`:
- Around line 246-248: Remove the three commented-out lines that set SRC_DIR to
EDGE_TOOLING_DIR, PLUGIN_DIR to the microshift-ci plugin path, and change
directory to SRC_DIR. These lines provide no functional value and should be
deleted entirely rather than kept as comments to reduce code clutter.
🪄 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: ce22fe09-ea78-43d3-bba9-6cd167ba2a0b
📒 Files selected for processing (1)
ci-operator/step-registry/openshift/edge-tooling/microshift-ci/doctor/openshift-edge-tooling-microshift-ci-doctor-commands.sh
| SRC_DIR=/tmp/edge-tooling | ||
| PLUGIN_DIR="${SRC_DIR}/plugins/microshift-ci" | ||
| git clone https://github.com/ggiguash/edge-tooling.git -b ci-doctor-analyse-workflow "${SRC_DIR}" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if openshift/edge-tooling repository exists and check typical branch names
# Check if official repo exists
echo "Checking if openshift/edge-tooling exists:"
gh api repos/openshift/edge-tooling --jq '.full_name' 2>/dev/null || echo "Repository not found or not accessible"
# Check if the personal fork exists and list branches
echo -e "\nChecking ggiguash/edge-tooling branches:"
gh api repos/ggiguash/edge-tooling/branches --jq '.[].name' 2>/dev/null | head -10 || echo "Repository not found or not accessible"
# Search for other step-registry scripts that reference edge-tooling
echo -e "\nOther scripts referencing EDGE_TOOLING_DIR or edge-tooling repos:"
rg -n --type=sh 'EDGE_TOOLING_DIR|edge-tooling\.git' ci-operator/step-registry/ | head -20Repository: openshift/release
Length of output: 1330
🏁 Script executed:
# Read the full context of the script around lines 246-252
sed -n '240,260p' ci-operator/step-registry/openshift/edge-tooling/microshift-ci/doctor/openshift-edge-tooling-microshift-ci-doctor-commands.shRepository: openshift/release
Length of output: 786
🏁 Script executed:
# Check how EDGE_TOOLING_DIR is defined in related scripts
grep -n "EDGE_TOOLING_DIR" ci-operator/step-registry/openshift/edge-tooling/*/openshift-edge-tooling-*-commands.shRepository: openshift/release
Length of output: 400
🏁 Script executed:
# Check the script header to understand the error handling setup
head -20 ci-operator/step-registry/openshift/edge-tooling/microshift-ci/doctor/openshift-edge-tooling-microshift-ci-doctor-commands.shRepository: openshift/release
Length of output: 711
🏁 Script executed:
# Verify if EDGE_TOOLING_DIR is passed as an environment variable from step metadata
cat ci-operator/step-registry/openshift/edge-tooling/microshift-ci/doctor/openshift-edge-tooling-microshift-ci-doctor-ref.yamlRepository: openshift/release
Length of output: 1743
Add cleanup before git clone to prevent failure if /tmp/edge-tooling exists from a prior run.
The script clones into /tmp/edge-tooling without checking if the directory already exists. If it does (from a previous execution or concurrent step runs), git clone will fail with "destination path already exists and is not an empty directory," causing the script to exit due to set -euo pipefail.
🔧 Proposed fix
SRC_DIR=/tmp/edge-tooling
PLUGIN_DIR="${SRC_DIR}/plugins/microshift-ci"
+rm -rf "${SRC_DIR}"
git clone https://github.com/ggiguash/edge-tooling.git -b ci-doctor-analyse-workflow "${SRC_DIR}"🤖 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/openshift/edge-tooling/microshift-ci/doctor/openshift-edge-tooling-microshift-ci-doctor-commands.sh`
around lines 250 - 252, The git clone command will fail if the SRC_DIR directory
already exists from a prior run. Add a cleanup command to remove the SRC_DIR
directory before attempting the git clone. Place a `rm -rf "${SRC_DIR}"`
statement immediately before the `git clone` line to ensure the destination path
does not exist when the clone operation begins, preventing the "destination path
already exists" error.
|
/pj-rehearse periodic-ci-openshift-eng-edge-tooling-main-microshift-ci-doctor |
|
@ggiguash: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@ggiguash: 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. |
|
/pj-rehearse periodic-ci-openshift-eng-edge-tooling-main-microshift-ci-doctor |
|
@ggiguash: your |
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
2 participants
Summary by CodeRabbit
This PR adjusts the MicroShift CI “Doctor” workflow tooling used by OpenShift Prow jobs so it can be exercised against in-development changes to the edge-tooling repository.
Key changes:
openshift-edge-tooling-microshift-ci-doctor-commands.shto create a fresh/tmp/edge-toolingcheckout during the job run.git cloneshttps://github.com/ggiguash/edge-tooling.gitand checks out theci-doctor-analyse-workflowbranch, instead of relying on a pre-installedEDGE_TOOLING_DIR.claudetoclaude-latest.Practical impact: