Skip to content

USHIFT-6983: Optimize microshift service restart when applying TLS configuration#6753

Merged
openshift-merge-bot[bot] merged 4 commits into
openshift:mainfrom
ggiguash:fix-tlsv13-config-errors
May 28, 2026
Merged

USHIFT-6983: Optimize microshift service restart when applying TLS configuration#6753
openshift-merge-bot[bot] merged 4 commits into
openshift:mainfrom
ggiguash:fix-tlsv13-config-errors

Conversation

@ggiguash

@ggiguash ggiguash commented May 27, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Tests
    • Improved error handling in VM command execution by explicitly capturing and returning exit codes.
    • Updated test scenarios to properly sequence service stop/start operations during TLS configuration application.
    • Re-enabled VM boot verification to ensure proper system initialization before proceeding.
    • Updated VM image selection for enhanced test environment consistency.

@openshift-ci-robot

openshift-ci-robot commented May 27, 2026

Copy link
Copy Markdown

@ggiguash: This pull request references USHIFT-6983 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.

Details

In 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 openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label May 27, 2026
@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 6005d59d-825f-432b-a8ca-9c8126bb09c2

📥 Commits

Reviewing files that changed from the base of the PR and between c93cc19 and 9df75a0.

📒 Files selected for processing (7)
  • test/bin/scenario.sh
  • test/image-blueprints/layer4-release/group3/rhel98-brew-lrel-tuned.toml
  • test/scenarios-bootc/el10/releases/el102-lrel@multi-config-standard1.sh
  • test/scenarios-bootc/el10/releases/el102-lrel@multi-config-standard2.sh
  • test/scenarios-bootc/el9/releases/el98-lrel@multi-config-standard1.sh
  • test/scenarios-bootc/el9/releases/el98-lrel@multi-config-standard2.sh
  • test/scenarios/releases/el98-lrel@ginkgo-multi-config.sh

Walkthrough

This PR updates test infrastructure error handling and MicroShift service control across multiple scenario scripts. SSH/SCP helpers now capture exit codes for explicit caller handling. MicroShift TLS configuration tests shift from service restart to stop-then-start lifecycle across el10, el9, and el98 ginkgo scenarios. The ginkgo scenario also enables boot-count polling and updates the base VM image.

Changes

Test Infrastructure Error Handling and MicroShift Service Lifecycle

Layer / File(s) Summary
SSH/SCP helper error handling
test/bin/scenario.sh
run_command_on_vm, copy_file_to_vm, and copy_file_from_vm now capture exit codes and return them explicitly instead of relying on fail-fast set -e termination, allowing callers to handle failures inline.
MicroShift TLS configuration service lifecycle
test/scenarios-bootc/el10/releases/el102-lrel@*.sh, test/scenarios-bootc/el9/releases/el98-lrel@*.sh, test/scenarios/releases/el98-lrel@ginkgo-multi-config.sh
All scenario files shift MicroShift TLS configuration from restart-based to stop-then-start lifecycle. The ginkgo scenario also enables boot-count polling and updates the base image to rhel98-brew-lrel-tuned.

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested reviewers

  • pacevedom
  • vanhalenar
🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (14 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main objective of the changeset: optimizing MicroShift service restart behavior when applying TLS configuration across multiple test scenario files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed PR modifies only shell scripts (.sh files), not Go/Ginkgo tests. Check for Ginkgo test names is not applicable to bash scenario scripts.
Test Structure And Quality ✅ Passed PR contains no Ginkgo test code changes; all modifications are bash scenario scripts. Custom check does not apply to this PR.
Microshift Test Compatibility ✅ Passed No new Ginkgo e2e tests (It, Describe, Context, When) are added. PR modifies only shell scenario scripts that orchestrate MicroShift service operations; check is not applicable.
Single Node Openshift (Sno) Test Compatibility ✅ Passed PR contains only bash shell script modifications for scenario testing; no Ginkgo e2e test files (It, Describe, Context, When) were added or modified.
Topology-Aware Scheduling Compatibility ✅ Passed PR modifies only Bash test scripts (no deployment manifests, operator code, or controllers). Check is not applicable.
Ote Binary Stdout Contract ✅ Passed PR modifies only shell scripts in test/; no Go source code changes. OTE Binary Stdout Contract applies to Go binary code, not shell scripts.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed This PR contains no new Ginkgo e2e tests. All modified files are shell scripts (.sh) for VM/scenario management; no Go test files were added or changed.
No-Weak-Crypto ✅ Passed PR introduces no weak cryptography. Changes are test infrastructure refactoring and enabling TLSv1.3 (strong crypto standard).
Container-Privileges ✅ Passed No container privilege escalation flags found in modified files; PR only changes test scenarios and shell helper functions, not container/K8s configurations.
No-Sensitive-Data-In-Logs ✅ Passed PR changes only add status messages (INFO/SUCCESS/timeout messages) and modify service control sequences; no sensitive data like passwords, tokens, keys, or internal hostnames are exposed in logs.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ggiguash

Copy link
Copy Markdown
Contributor Author

/test all

@openshift-ci openshift-ci Bot requested review from eslutsky and pacevedom May 27, 2026 20:42
@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 27, 2026
@ggiguash

Copy link
Copy Markdown
Contributor Author

/retest-required

@agullon

agullon commented May 28, 2026

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label May 28, 2026
@openshift-ci

openshift-ci Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: agullon, ggiguash

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ggiguash

Copy link
Copy Markdown
Contributor Author

/verified by ci

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label May 28, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@ggiguash: This PR has been marked as verified by ci.

Details

In response to this:

/verified by ci

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-merge-bot

Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD c93cc19 and 2 for PR HEAD 9df75a0 in total

@openshift-ci

openshift-ci Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

@ggiguash: all tests passed!

Full PR test history. Your PR dashboard.

Details

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.

@openshift-merge-bot openshift-merge-bot Bot merged commit 23e5d7f into openshift:main May 28, 2026
25 checks passed
@ggiguash ggiguash deleted the fix-tlsv13-config-errors branch May 28, 2026 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants