MG-272: Added Hypershift weekly job for support-log-gather operator#79339
MG-272: Added Hypershift weekly job for support-log-gather operator#79339shivprakashmuley wants to merge 3 commits into
Conversation
|
@shivprakashmuley: This pull request references MG-272 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 story to target either version "5.0." or "openshift-5.0.", but it targets "openshift-4.22" instead. 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. |
WalkthroughA new weekly e2e job, ChangesE2E Hypershift Operator Weekly Job
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Suggested labels
🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
|
/pj-rehearse periodic-ci-openshift-must-gather-operator-release-4.22-e2e-hypershift-operator-weekly |
|
@shivprakashmuley: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/must-gather-operator/openshift-must-gather-operator-release-4.22.yaml`:
- Around line 234-238: The oc rollout status invocation in the commands block
(the line invoking "oc -n must-gather-operator rollout status deployment
must-gather-operator") can hang indefinitely; update that command to include an
explicit timeout (e.g., add "--timeout=10m" or another appropriate duration) so
the verify-install step fails fast instead of blocking periodic runs.
🪄 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: 59f5f162-5417-4328-badc-720cc18cd4eb
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift/must-gather-operator/openshift-must-gather-operator-release-4.22-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (1)
ci-operator/config/openshift/must-gather-operator/openshift-must-gather-operator-release-4.22.yaml
| - as: verify-install | ||
| cli: latest | ||
| commands: | | ||
| oc -n must-gather-operator rollout status deployment must-gather-operator | ||
| from: src |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the default timeout behavior of oc rollout statuswhen--timeout is not specified?
💡 Result:
If you run oc rollout status without --timeout, it will keep watching until the rollout finishes (i.e., it does not end watch early due to a client-side timeout). This is because the command’s --timeout option (“length of time to wait before ending watch”) defaults to 0, and 0 means “never” [1]. Note: this timeout controls ending the watch, not per-request connection/watch timeouts; those are governed by a separate --request-timeout flag [1].
Citations:
🏁 Script executed:
# Search for other oc rollout status commands in CI configuration to check typical timeout patterns
rg "rollout status" --type yaml -B 2 -A 2Repository: openshift/release
Length of output: 50373
Add an explicit rollout timeout to prevent stuck weekly runs.
oc rollout status without --timeout waits indefinitely and can tie up periodic capacity.
Proposed fix
- oc -n must-gather-operator rollout status deployment must-gather-operator
+ oc -n must-gather-operator rollout status deployment must-gather-operator --timeout=15m📝 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.
| - as: verify-install | |
| cli: latest | |
| commands: | | |
| oc -n must-gather-operator rollout status deployment must-gather-operator | |
| from: src | |
| - as: verify-install | |
| cli: latest | |
| commands: | | |
| oc -n must-gather-operator rollout status deployment must-gather-operator --timeout=15m | |
| from: src |
🤖 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/must-gather-operator/openshift-must-gather-operator-release-4.22.yaml`
around lines 234 - 238, The oc rollout status invocation in the commands block
(the line invoking "oc -n must-gather-operator rollout status deployment
must-gather-operator") can hang indefinitely; update that command to include an
explicit timeout (e.g., add "--timeout=10m" or another appropriate duration) so
the verify-install step fails fast instead of blocking periodic runs.
|
/pj-rehearse periodic-ci-openshift-must-gather-operator-release-4.22-e2e-hypershift-operator-weekly |
|
@shivprakashmuley: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-must-gather-operator-release-4.22-e2e-hypershift-operator-weekly |
|
@shivprakashmuley: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: shivprakashmuley 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 |
|
[REHEARSALNOTIFIER]
A total of 78 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/hypershift/hostedcluster/create/hostedcluster/hypershift-hostedcluster-create-hostedcluster-commands.sh`:
- Around line 147-149: The script currently tests and appends EXTRA_ARGS
unsafely (uses [[ -n "${EXTRA_ARGS}" ]] which can fail under set -u and performs
unquoted word splitting/globbing when appending to ARGS). Change the check to
guard against unset (use a default or parameter expansion like "${EXTRA_ARGS-}"
or test for set) and split EXTRA_ARGS into a temporary array (e.g.,
tmp=($EXTRA_ARGS) but using proper safe splitting like read -r -a tmp <<<
"${EXTRA_ARGS-}") before appending elements to ARGS in a loop so each element is
added quoted (refer to EXTRA_ARGS and ARGS in the script), avoiding
word-splitting and globbing.
🪄 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: 1e6df69b-1ccd-44fd-8be1-21f3ffd83c6d
📒 Files selected for processing (1)
ci-operator/step-registry/hypershift/hostedcluster/create/hostedcluster/hypershift-hostedcluster-create-hostedcluster-commands.sh
| if [[ -n "${EXTRA_ARGS}" ]]; then | ||
| ARGS+=( ${EXTRA_ARGS} ) | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if file exists and examine the relevant section
if [[ -f "ci-operator/step-registry/hypershift/hostedcluster/create/hostedcluster/hypershift-hostedcluster-create-hostedcluster-commands.sh" ]]; then
echo "=== File head (first 10 lines) ==="
head -10 "ci-operator/step-registry/hypershift/hostedcluster/create/hostedcluster/hypershift-hostedcluster-create-hostedcluster-commands.sh"
echo ""
echo "=== Lines 140-160 (around the target) ==="
sed -n '140,160p' "ci-operator/step-registry/hypershift/hostedcluster/create/hostedcluster/hypershift-hostedcluster-create-hostedcluster-commands.sh" | cat -n
echo ""
echo "=== Search for EXTRA_ARGS usage ==="
grep -n "EXTRA_ARGS" "ci-operator/step-registry/hypershift/hostedcluster/create/hostedcluster/hypershift-hostedcluster-create-hostedcluster-commands.sh"
else
echo "File not found"
fiRepository: openshift/release
Length of output: 1507
Guard EXTRA_ARGS for nounset and append it safely.
Line 147 can abort the script under set -u when EXTRA_ARGS is unset, and Line 148 performs unsafe word splitting/globbing. Default the check and split into a temporary array before appending.
Suggested fix
- if [[ -n "${EXTRA_ARGS}" ]]; then
- ARGS+=( ${EXTRA_ARGS} )
+ if [[ -n "${EXTRA_ARGS:-}" ]]; then
+ read -r -a extra_args <<< "${EXTRA_ARGS}"
+ ARGS+=("${extra_args[@]}")
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 [[ -n "${EXTRA_ARGS}" ]]; then | |
| ARGS+=( ${EXTRA_ARGS} ) | |
| fi | |
| if [[ -n "${EXTRA_ARGS:-}" ]]; then | |
| read -r -a extra_args <<< "${EXTRA_ARGS}" | |
| ARGS+=("${extra_args[@]}") | |
| fi |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 148-148: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.
(SC2206)
🤖 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/hypershift/hostedcluster/create/hostedcluster/hypershift-hostedcluster-create-hostedcluster-commands.sh`
around lines 147 - 149, The script currently tests and appends EXTRA_ARGS
unsafely (uses [[ -n "${EXTRA_ARGS}" ]] which can fail under set -u and performs
unquoted word splitting/globbing when appending to ARGS). Change the check to
guard against unset (use a default or parameter expansion like "${EXTRA_ARGS-}"
or test for set) and split EXTRA_ARGS into a temporary array (e.g.,
tmp=($EXTRA_ARGS) but using proper safe splitting like read -r -a tmp <<<
"${EXTRA_ARGS-}") before appending elements to ARGS in a loop so each element is
added quoted (refer to EXTRA_ARGS and ARGS in the script), avoiding
word-splitting and globbing.
|
@shivprakashmuley: The following tests 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. |
Summary
This PR adds CI configuration to the openshift/must-gather-operator 4.22 CI to introduce a new Hypershift weekly end-to-end job and a small hypershift create-cluster improvement. It wires Hypershift hosted-cluster validation into the operator's CI pipeline so the operator is exercised regularly in a hosted (Hypershift) environment.
Key changes
New weekly Hypershift e2e job: e2e-hypershift-operator-weekly (ci-operator/config/openshift/must-gather-operator/openshift-must-gather-operator-release-4.22.yaml)
Hypershift CLI pass-through: small change to the Hypershift hosted-cluster creation script (ci-operator/step-registry/hypershift/hostedcluster/create/hostedcluster/hypershift-hostedcluster-create-hostedcluster-commands.sh)
hypershift create cluster aws.Practical impact
Notes / Review items