NO-ISSUE: Reenable RHEL 9.8 and 10.2 leftover image builds and scenarios - #7045
NO-ISSUE: Reenable RHEL 9.8 and 10.2 leftover image builds and scenarios#7045ggiguash wants to merge 12 commits into
Conversation
|
@ggiguash: This pull request explicitly references no jira issue. 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
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (8)
WalkthroughAdds EL10 and EL9 bootc scenario coverage, updates release scenario VM settings, and adds disabled RHEL 9.8 tuned image and multi-configuration test assets. ChangesBootc scenario coverage
EL9 scenario coverage
Tuned release coverage
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant scenario.sh
participant host1
participant FIPS_Suite
scenario.sh->>host1: Create rhel102-bootc with FIPS kickstart
scenario.sh->>FIPS_Suite: Run suites/fips/ against host1
scenario.sh->>host1: Remove host1
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (13 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 |
|
[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
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
test/image-blueprints-bootc/el10/layer2-presubmit/group2/rhel102-bootc-source-fips.containerfile (1)
12-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUpdate the stale RHEL 9.x comment.
The comment references RHEL 9.x, but this is a RHEL 10.2 containerfile. Please update it to avoid confusion, and verify if
containernetworking-pluginsis still needed for RHEL 10 (or ifnetavarkis used instead).📝 Proposed update
-# Note: CNI plugins are required for podman to operate normally on RHEL 9.x. +# Note: CNI plugins are required for podman/cri-o to operate normally. # This package is no longer installed as cri-o dependency.🤖 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 `@test/image-blueprints-bootc/el10/layer2-presubmit/group2/rhel102-bootc-source-fips.containerfile` around lines 12 - 13, Update the CNI dependency comment in the RHEL 10.2 containerfile to reference RHEL 10 accurately, and verify the current networking implementation before stating that containernetworking-plugins is required; document netavark instead if it has replaced that dependency.
🤖 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 `@test/scenarios-bootc/el10/periodics/el102-src`@fips.sh:
- Around line 1-2: Update both
test/scenarios-bootc/el10/periodics/el102-src@fips.sh lines 1-2 and
test/scenarios-bootc/el10/presubmits/el102-src@upgrade-fails-then-recovers.sh
lines 1-2 to use the /usr/bin/bash shebang and add set -euo pipefail immediately
afterward.
In `@test/scenarios-bootc/el10/releases/el102-lrel`@optional-sigstore.sh:
- Line 26: Update the launch_vm invocation to pass rhel102-bootc as the first
positional argument instead of using the --boot_blueprint option, while
preserving the existing disk-size and vCPU arguments.
In `@test/scenarios-bootc/el9/presubmits/el98-src`@upgrade-fails-then-recovers.sh:
- Around line 1-2: Update the shebang and add strict error handling in all four
scripts:
test/scenarios-bootc/el9/presubmits/el98-src@upgrade-fails-then-recovers.sh
lines 1-2, test/scenarios/presubmits/el98-src@upgrade-fails-then-recovers.sh
lines 1-2, test/scenarios/presubmits/el98-src@low-latency.sh lines 1-5, and
test/scenarios/releases/el98-lrel@low-latency.sh lines 1-5. Use the
/usr/bin/bash shebang and insert set -euo pipefail immediately afterward.
In `@test/scenarios/releases/el98-zprel`@el98-lrel@rpm-upgrade.sh:
- Around line 1-2: Update the script shebang to #!/usr/bin/bash and add strict
shell options with set -euo pipefail immediately afterward.
---
Nitpick comments:
In
`@test/image-blueprints-bootc/el10/layer2-presubmit/group2/rhel102-bootc-source-fips.containerfile`:
- Around line 12-13: Update the CNI dependency comment in the RHEL 10.2
containerfile to reference RHEL 10 accurately, and verify the current networking
implementation before stating that containernetworking-plugins is required;
document netavark instead if it has replaced that dependency.
🪄 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: f8664e79-d4f0-4936-9e67-77abd6a551a7
📒 Files selected for processing (11)
test/image-blueprints-bootc/el10/layer2-presubmit/group2/rhel102-bootc-source-fips.containerfiletest/image-blueprints/layer2-presubmit/group1/rhel98-source-tuned.tomltest/scenarios-bootc/el10/periodics/el102-src@fips.shtest/scenarios-bootc/el10/presubmits/el102-src@upgrade-fails-then-recovers.shtest/scenarios-bootc/el10/releases/el102-lrel@optional-sigstore.shtest/scenarios-bootc/el9/presubmits/el98-src@upgrade-fails-then-recovers.shtest/scenarios-bootc/el9/releases/el98-lrel@optional-sigstore.shtest/scenarios/presubmits/el98-src@low-latency.shtest/scenarios/presubmits/el98-src@upgrade-fails-then-recovers.shtest/scenarios/releases/el98-lrel@low-latency.shtest/scenarios/releases/el98-zprel@el98-lrel@rpm-upgrade.sh
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 4
🧹 Nitpick comments (1)
test/image-blueprints-bootc/el10/layer2-presubmit/group2/rhel102-bootc-source-fips.containerfile (1)
12-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUpdate the stale RHEL 9.x comment.
The comment references RHEL 9.x, but this is a RHEL 10.2 containerfile. Please update it to avoid confusion, and verify if
containernetworking-pluginsis still needed for RHEL 10 (or ifnetavarkis used instead).📝 Proposed update
-# Note: CNI plugins are required for podman to operate normally on RHEL 9.x. +# Note: CNI plugins are required for podman/cri-o to operate normally. # This package is no longer installed as cri-o dependency.🤖 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 `@test/image-blueprints-bootc/el10/layer2-presubmit/group2/rhel102-bootc-source-fips.containerfile` around lines 12 - 13, Update the CNI dependency comment in the RHEL 10.2 containerfile to reference RHEL 10 accurately, and verify the current networking implementation before stating that containernetworking-plugins is required; document netavark instead if it has replaced that dependency.
🤖 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 `@test/scenarios-bootc/el10/periodics/el102-src`@fips.sh:
- Around line 1-2: Update both
test/scenarios-bootc/el10/periodics/el102-src@fips.sh lines 1-2 and
test/scenarios-bootc/el10/presubmits/el102-src@upgrade-fails-then-recovers.sh
lines 1-2 to use the /usr/bin/bash shebang and add set -euo pipefail immediately
afterward.
In `@test/scenarios-bootc/el10/releases/el102-lrel`@optional-sigstore.sh:
- Line 26: Update the launch_vm invocation to pass rhel102-bootc as the first
positional argument instead of using the --boot_blueprint option, while
preserving the existing disk-size and vCPU arguments.
In `@test/scenarios-bootc/el9/presubmits/el98-src`@upgrade-fails-then-recovers.sh:
- Around line 1-2: Update the shebang and add strict error handling in all four
scripts:
test/scenarios-bootc/el9/presubmits/el98-src@upgrade-fails-then-recovers.sh
lines 1-2, test/scenarios/presubmits/el98-src@upgrade-fails-then-recovers.sh
lines 1-2, test/scenarios/presubmits/el98-src@low-latency.sh lines 1-5, and
test/scenarios/releases/el98-lrel@low-latency.sh lines 1-5. Use the
/usr/bin/bash shebang and insert set -euo pipefail immediately afterward.
In `@test/scenarios/releases/el98-zprel`@el98-lrel@rpm-upgrade.sh:
- Around line 1-2: Update the script shebang to #!/usr/bin/bash and add strict
shell options with set -euo pipefail immediately afterward.
---
Nitpick comments:
In
`@test/image-blueprints-bootc/el10/layer2-presubmit/group2/rhel102-bootc-source-fips.containerfile`:
- Around line 12-13: Update the CNI dependency comment in the RHEL 10.2
containerfile to reference RHEL 10 accurately, and verify the current networking
implementation before stating that containernetworking-plugins is required;
document netavark instead if it has replaced that dependency.
🪄 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: f8664e79-d4f0-4936-9e67-77abd6a551a7
📒 Files selected for processing (11)
test/image-blueprints-bootc/el10/layer2-presubmit/group2/rhel102-bootc-source-fips.containerfiletest/image-blueprints/layer2-presubmit/group1/rhel98-source-tuned.tomltest/scenarios-bootc/el10/periodics/el102-src@fips.shtest/scenarios-bootc/el10/presubmits/el102-src@upgrade-fails-then-recovers.shtest/scenarios-bootc/el10/releases/el102-lrel@optional-sigstore.shtest/scenarios-bootc/el9/presubmits/el98-src@upgrade-fails-then-recovers.shtest/scenarios-bootc/el9/releases/el98-lrel@optional-sigstore.shtest/scenarios/presubmits/el98-src@low-latency.shtest/scenarios/presubmits/el98-src@upgrade-fails-then-recovers.shtest/scenarios/releases/el98-lrel@low-latency.shtest/scenarios/releases/el98-zprel@el98-lrel@rpm-upgrade.sh
🛑 Comments failed to post (4)
test/scenarios-bootc/el10/periodics/el102-src@fips.sh (1)
1-2: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the correct shebang and enable strict error handling.
As per coding guidelines, shell scripts must use
#!/usr/bin/bashandset -euo pipefail.
test/scenarios-bootc/el10/periodics/el102-src@fips.sh#L1-L2: Update the shebang and addset -euo pipefail.test/scenarios-bootc/el10/presubmits/el102-src@upgrade-fails-then-recovers.sh#L1-L2: Update the shebang and addset -euo pipefail.📍 Affects 2 files
test/scenarios-bootc/el10/periodics/el102-src@fips.sh#L1-L2(this comment)test/scenarios-bootc/el10/presubmits/el102-src@upgrade-fails-then-recovers.sh#L1-L2🤖 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 `@test/scenarios-bootc/el10/periodics/el102-src`@fips.sh around lines 1 - 2, Update both test/scenarios-bootc/el10/periodics/el102-src@fips.sh lines 1-2 and test/scenarios-bootc/el10/presubmits/el102-src@upgrade-fails-then-recovers.sh lines 1-2 to use the /usr/bin/bash shebang and add set -euo pipefail immediately afterward.Source: Coding guidelines
test/scenarios-bootc/el10/releases/el102-lrel@optional-sigstore.sh (1)
26-26: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Pass
boot_blueprintas a positional argument.
launch_vmexpectsboot_blueprintas its first positional argument. Passing--boot_blueprintwill cause the argument parser to fail and exit.🐛 Proposed fix
- launch_vm --boot_blueprint rhel102-bootc --vm_disksize 25 --vm_vcpus 4 + launch_vm rhel102-bootc --vm_disksize 25 --vm_vcpus 4📝 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.launch_vm rhel102-bootc --vm_disksize 25 --vm_vcpus 4🤖 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 `@test/scenarios-bootc/el10/releases/el102-lrel`@optional-sigstore.sh at line 26, Update the launch_vm invocation to pass rhel102-bootc as the first positional argument instead of using the --boot_blueprint option, while preserving the existing disk-size and vCPU arguments.test/scenarios-bootc/el9/presubmits/el98-src@upgrade-fails-then-recovers.sh (1)
1-2: 📐 Maintainability & Code Quality | 🔴 Critical | ⚡ Quick win
Fix shebang and add strict error handling.
As per coding guidelines, shell scripts must use the
#!/usr/bin/bashshebang and includeset -euo pipefail.
test/scenarios-bootc/el9/presubmits/el98-src@upgrade-fails-then-recovers.sh#L1-L2: Update the shebang and insertset -euo pipefail.test/scenarios/presubmits/el98-src@upgrade-fails-then-recovers.sh#L1-L2: Update the shebang and insertset -euo pipefail.test/scenarios/presubmits/el98-src@low-latency.sh#L1-L5: Update the shebang and insertset -euo pipefail.test/scenarios/releases/el98-lrel@low-latency.sh#L1-L5: Update the shebang and insertset -euo pipefail.📍 Affects 4 files
test/scenarios-bootc/el9/presubmits/el98-src@upgrade-fails-then-recovers.sh#L1-L2(this comment)test/scenarios/presubmits/el98-src@upgrade-fails-then-recovers.sh#L1-L2test/scenarios/presubmits/el98-src@low-latency.sh#L1-L5test/scenarios/releases/el98-lrel@low-latency.sh#L1-L5🤖 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 `@test/scenarios-bootc/el9/presubmits/el98-src`@upgrade-fails-then-recovers.sh around lines 1 - 2, Update the shebang and add strict error handling in all four scripts: test/scenarios-bootc/el9/presubmits/el98-src@upgrade-fails-then-recovers.sh lines 1-2, test/scenarios/presubmits/el98-src@upgrade-fails-then-recovers.sh lines 1-2, test/scenarios/presubmits/el98-src@low-latency.sh lines 1-5, and test/scenarios/releases/el98-lrel@low-latency.sh lines 1-5. Use the /usr/bin/bash shebang and insert set -euo pipefail immediately afterward.Source: Coding guidelines
test/scenarios/releases/el98-zprel@el98-lrel@rpm-upgrade.sh (1)
1-2: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use required shebang and strict shell options.
As per coding guidelines, shell scripts must use the
#!/usr/bin/bashshebang and includeset -euo pipefail.🛠️ Proposed fix
-#!/bin/bash - +#!/usr/bin/bash +set -euo pipefail +📝 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.#!/usr/bin/bash set -euo pipefail🤖 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 `@test/scenarios/releases/el98-zprel`@el98-lrel@rpm-upgrade.sh around lines 1 - 2, Update the script shebang to #!/usr/bin/bash and add strict shell options with set -euo pipefail immediately afterward.Source: Coding guidelines
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
/test e2e-aws-tests-bootc-release-el9 |
2 similar comments
|
/test e2e-aws-tests-bootc-release-el9 |
|
/test e2e-aws-tests-bootc-release-el9 |
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
|
/test ? |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
/test e2e-aws-tests-cache |
|
/test e2e-aws-tests-bootc-release-arm-el9 |
…kernel-rt RHEL 10 moved iptables compat kernel modules (nft_compat, xt_CT) from kernel-modules-core to kernel-modules-extra. Use conditional rich dependencies so that each variant is gated on its own kernel-core package — installing MicroShift on a kernel-rt system does not pull in the regular kernel (and vice-versa). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> pre-commit.check-secrets: ENABLED
|
/test e2e-aws-tests-cache |
|
@ggiguash: 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. |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
/test e2e-aws-tests-cache |
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 `@test/scenarios/releases/el98-lrel`@ginkgo-multi-config.sh.disabled:
- Around line 65-78: Update the MicroShift stop command in the TLS configuration
setup to fail the scenario when `systemctl stop microshift` fails; remove the
trailing `|| true` from the `run_command_on_vm` invocation, or explicitly verify
the service is inactive before continuing. Keep the subsequent TLS configuration
and service start steps unchanged.
🪄 Autofix
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: Pro Plus
Run ID: 5bf85985-aceb-4162-b9b7-31799652c555
📒 Files selected for processing (10)
test/image-blueprints-bootc/el10/layer2-presubmit/group2/rhel102-bootc-source-fips.containerfiletest/image-blueprints/layer4-release/group3/rhel98-brew-lrel-tuned.toml.disabledtest/scenarios-bootc/el10/periodics/el102-src@fips.shtest/scenarios-bootc/el10/presubmits/el102-src@upgrade-fails-then-recovers.shtest/scenarios-bootc/el10/releases/el102-lrel@optional-sigstore.shtest/scenarios-bootc/el9/presubmits/el98-src@upgrade-fails-then-recovers.shtest/scenarios-bootc/el9/releases/el98-lrel@optional-sigstore.shtest/scenarios/presubmits/el98-src@upgrade-fails-then-recovers.shtest/scenarios/releases/el98-lrel@ginkgo-multi-config.sh.disabledtest/scenarios/releases/el98-zprel@el98-lrel@rpm-upgrade.sh
🚧 Files skipped from review as they are similar to previous changes (8)
- test/image-blueprints-bootc/el10/layer2-presubmit/group2/rhel102-bootc-source-fips.containerfile
- test/scenarios/presubmits/el98-src@upgrade-fails-then-recovers.sh
- test/scenarios-bootc/el10/presubmits/el102-src@upgrade-fails-then-recovers.sh
- test/scenarios-bootc/el10/releases/el102-lrel@optional-sigstore.sh
- test/scenarios-bootc/el9/presubmits/el98-src@upgrade-fails-then-recovers.sh
- test/scenarios-bootc/el10/periodics/el102-src@fips.sh
- test/scenarios/releases/el98-zprel@el98-lrel@rpm-upgrade.sh
- test/scenarios-bootc/el9/releases/el98-lrel@optional-sigstore.sh
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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 `@test/scenarios/releases/el98-lrel`@ginkgo-multi-config.sh.disabled:
- Around line 65-78: Update the MicroShift stop command in the TLS configuration
setup to fail the scenario when `systemctl stop microshift` fails; remove the
trailing `|| true` from the `run_command_on_vm` invocation, or explicitly verify
the service is inactive before continuing. Keep the subsequent TLS configuration
and service start steps unchanged.
🪄 Autofix
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: Pro Plus
Run ID: 5bf85985-aceb-4162-b9b7-31799652c555
📒 Files selected for processing (10)
test/image-blueprints-bootc/el10/layer2-presubmit/group2/rhel102-bootc-source-fips.containerfiletest/image-blueprints/layer4-release/group3/rhel98-brew-lrel-tuned.toml.disabledtest/scenarios-bootc/el10/periodics/el102-src@fips.shtest/scenarios-bootc/el10/presubmits/el102-src@upgrade-fails-then-recovers.shtest/scenarios-bootc/el10/releases/el102-lrel@optional-sigstore.shtest/scenarios-bootc/el9/presubmits/el98-src@upgrade-fails-then-recovers.shtest/scenarios-bootc/el9/releases/el98-lrel@optional-sigstore.shtest/scenarios/presubmits/el98-src@upgrade-fails-then-recovers.shtest/scenarios/releases/el98-lrel@ginkgo-multi-config.sh.disabledtest/scenarios/releases/el98-zprel@el98-lrel@rpm-upgrade.sh
🚧 Files skipped from review as they are similar to previous changes (8)
- test/image-blueprints-bootc/el10/layer2-presubmit/group2/rhel102-bootc-source-fips.containerfile
- test/scenarios/presubmits/el98-src@upgrade-fails-then-recovers.sh
- test/scenarios-bootc/el10/presubmits/el102-src@upgrade-fails-then-recovers.sh
- test/scenarios-bootc/el10/releases/el102-lrel@optional-sigstore.sh
- test/scenarios-bootc/el9/presubmits/el98-src@upgrade-fails-then-recovers.sh
- test/scenarios-bootc/el10/periodics/el102-src@fips.sh
- test/scenarios/releases/el98-zprel@el98-lrel@rpm-upgrade.sh
- test/scenarios-bootc/el9/releases/el98-lrel@optional-sigstore.sh
🛑 Comments failed to post (1)
test/scenarios/releases/el98-lrel@ginkgo-multi-config.sh.disabled (1)
65-78: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "Locate target file:" fd -a 'el98-lrel@ginkgo-multi-config.sh.disabled' . || true target="$(fd 'el98-lrel@ginkgo-multi-config.sh.disabled' . | head -n 1 || true)" if [ -n "${target:-}" ]; then echo "Target: $target" wc -l "$target" sed -n '1,130p' "$target" | cat -n fi echo echo "Search run_command_on_vm:" rg -n "function run_command_on_vm|run_command_on_vm\\(" . || trueRepository: openshift/microshift
Length of output: 4991
🏁 Script executed:
#!/bin/bash set -euo pipefail target="$(fd 'el98-lrel@ginkgo-multi-config.sh.disabled' . | head -n 1 || true)" echo "Target: $target" sed -n '1,40p' "$target" | cat -n echo sed -n '80,115p' "./test/bin/scenario.sh" | cat -nRepository: openshift/microshift
Length of output: 3194
Fail when MicroShift cannot stop.
run_command_on_vmexits on failure, so|| trueat Line 66 suppresses a failedsystemctl stop microshift. If MicroShift stays active, Line 78 may start it without applying/etc/microshift/config.d/10-tls.yaml; delete|| trueor fail the scenario after verifying the service is inactive.Proposed fix
- run_command_on_vm host1 "sudo systemctl stop microshift" || true + run_command_on_vm host1 "sudo systemctl stop microshift"📝 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.# Stop MicroShift before applying TLS configuration run_command_on_vm host1 "sudo systemctl stop microshift" # Apply TLSv1.3 configuration via drop-in config echo "INFO: Configuring TLSv1.3..." run_command_on_vm host1 "sudo mkdir -p /etc/microshift/config.d" run_command_on_vm host1 "sudo tee /etc/microshift/config.d/10-tls.yaml > /dev/null << 'EOF' apiServer: tls: minVersion: VersionTLS13 EOF" # Start MicroShift to apply TLS configuration run_command_on_vm host1 "sudo systemctl start microshift"🤖 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 `@test/scenarios/releases/el98-lrel`@ginkgo-multi-config.sh.disabled around lines 65 - 78, Update the MicroShift stop command in the TLS configuration setup to fail the scenario when `systemctl stop microshift` fails; remove the trailing `|| true` from the `run_command_on_vm` invocation, or explicitly verify the service is inactive before continuing. Keep the subsequent TLS configuration and service start steps unchanged.
3 participants
The following issues are addressed in this PR:
Summary by CodeRabbit
New Features
Bug Fixes