Skip to content

NO-ISSUE: Test restart optimizations#7025

Open
pacevedom wants to merge 8 commits into
openshift:mainfrom
pacevedom:restart-optimizations
Open

NO-ISSUE: Test restart optimizations#7025
pacevedom wants to merge 8 commits into
openshift:mainfrom
pacevedom:restart-optimizations

Conversation

@pacevedom

@pacevedom pacevedom commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Due to the latest rebase PRs, MicroShift may take a bit more time to fully restart because there are more containers that need to signal readiness.
Because of this, there is a need to have efficient test cases that minimize the number of restarts while keeping the functional checks and correct MicroShift behavior, which is what this PR is addressing.

Summary by CodeRabbit

  • Bug Fixes
    • Improved test cleanup consistency across multiple configuration suites.
    • Restarts now happen at suite teardown in several cases, reducing leftover state between tests.
    • Updated configuration cleanup so removed settings are fully cleared before the next test runs.
    • Adjusted telemetry and feature-gate test teardown flow to better restore a clean system state.

@openshift-ci

openshift-ci Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 9, 2026
@pacevedom

Copy link
Copy Markdown
Contributor Author

/test e2e-aws-tests

@openshift-ci

openshift-ci Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pacevedom

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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 9, 2026
@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Multiple Robot Framework test suites are refactored to shift when MicroShift is restarted during test cleanup. Per-test teardowns/setups now generally only remove drop-in configuration files, while suite-level Teardown keywords are expanded to perform config removal and a single restart at suite end.

Changes

Test suite teardown restructuring

Layer / File(s) Summary
Storage tests teardown
test/suites/configuration1/configuration.robot
Per-test teardowns only remove drop-in configs; suite Teardown removes 10-storage and restarts once, with reordered kubeconfig/logout cleanup.
DNS resource config restart flow
test/suites/configuration1/dns-resource-configuration.robot
Setup/teardown and helper keywords (Apply DNS Resource Config, Apply Invalid DNS Resource Config, Remove DNS Resource Config) reorder removal/restart steps; suite Teardown now removes config and restarts.
Audit log teardown consolidation
test/suites/configuration2/audit-log.robot
Per-test teardowns drop per-test restarts; suite Teardown restarts once, removes drop-in, deletes namespace, removes kubeconfig, and logs out.
Kustomize cleanup updates
test/suites/configuration2/kustomize-sources.robot
Per-test teardowns and Cleanup Kustomize Test drop the restart step; suite Teardown removes dropped config and restarts once, then removes kubeconfig and logs out.
Etcd memory limit teardown
test/suites/standard1/etcd.robot
Memory limit tests replace Restore Default Config with direct removal of 10-etcd drop-in config.
Feature gates teardown reorder
test/suites/standard2/feature-gates.robot
Suite Teardown adds drop-in/lock-file removal plus restart before logout; Teardown Custom Feature Gates Test stops MicroShift and removes config/lock file without restarting.
Telemetry teardown and config removal
test/suites/telemetry/telemetry.robot
Suite Teardown removes telemetry drop-in, restores pull secrets, restarts, and waits for healthcheck before logout; Remove Telemetry Configuration no longer restarts.

Estimated code review effort: 2 (Simple) | ~12 minutes

Suggested reviewers: jerpeter1, kasturinarra

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 only changes .robot suites; no Ginkgo It/Describe/Context/When titles were added or modified.
Test Structure And Quality ✅ Passed PR only changes a Robot Framework suite; no Ginkgo It/BeforeEach/Eventually code was touched, so this check is not applicable.
Microshift Test Compatibility ✅ Passed No Ginkgo e2e tests were added; all changed files are Robot suites, so the MicroShift compatibility check is not applicable.
Single Node Openshift (Sno) Test Compatibility ✅ Passed Only Robot Framework suites changed; no new Ginkgo e2e tests or multi-node assumptions were added.
Topology-Aware Scheduling Compatibility ✅ Passed Only Robot test suites changed; no manifests, operator code, controllers, or scheduling constraints were added, so the topology-aware scheduling check is not applicable.
Ote Binary Stdout Contract ✅ Passed PR only changes a Robot test suite teardown; no process-level binary code or stdout logging paths were modified.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed PR only changes Robot Framework suites; no Go/Ginkgo e2e tests were added, so this IPv4/external-connectivity check does not apply.
No-Weak-Crypto ✅ Passed Touched files are Robot test orchestration only; scan found no MD5/SHA1/DES/RC4/3DES/Blowfish/ECB, custom crypto, or secret comparisons.
Container-Privileges ✅ Passed All touched files are Robot test suites; searches found no privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, or allowPrivilegeEscalation settings.
No-Sensitive-Data-In-Logs ✅ Passed The PR only changes restart/teardown flow; I found no new Log/Console calls or secret-bearing messages in the touched Robot files.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the main theme of the PR: optimizing test restart and cleanup behavior across suites.
✨ 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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
test/suites/configuration2/kustomize-sources.robot (1)

162-164: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Fix inaccurate documentation in Cleanup Kustomize Test.

The doc says "The next test's setup will restart MicroShift," but there is no [Setup] keyword on the tests — the restart happens in the test body via Drop In MicroShift Config + Restart MicroShift.

📝 Suggested doc fix
 Cleanup Kustomize Test
-    [Documentation]    Standard cleanup for a single-path kustomize test without restarting.
-    ...    The next test's setup will restart MicroShift.
+    [Documentation]    Standard cleanup for a single-path kustomize test without restarting.
+    ...    The next test will restart MicroShift with its own config.
🤖 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/suites/configuration2/kustomize-sources.robot` around lines 162 - 164,
The documentation for Cleanup Kustomize Test is inaccurate: it says the next
test’s setup restarts MicroShift, but the restart actually happens in the test
body. Update the [Documentation] text on Cleanup Kustomize Test to describe the
cleanup accurately, and make it consistent with the related Drop In MicroShift
Config and Restart MicroShift keywords instead of referring to a nonexistent
setup restart.
🤖 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.

Nitpick comments:
In `@test/suites/configuration2/kustomize-sources.robot`:
- Around line 162-164: The documentation for Cleanup Kustomize Test is
inaccurate: it says the next test’s setup restarts MicroShift, but the restart
actually happens in the test body. Update the [Documentation] text on Cleanup
Kustomize Test to describe the cleanup accurately, and make it consistent with
the related Drop In MicroShift Config and Restart MicroShift keywords instead of
referring to a nonexistent setup restart.

ℹ️ Review info
⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: 419ddfa5-37b0-45c3-a7e9-1d395b057740

📥 Commits

Reviewing files that changed from the base of the PR and between 0e09fc5 and c6fd94b.

📒 Files selected for processing (7)
  • test/suites/configuration1/configuration.robot
  • test/suites/configuration1/dns-resource-configuration.robot
  • test/suites/configuration2/audit-log.robot
  • test/suites/configuration2/kustomize-sources.robot
  • test/suites/standard1/etcd.robot
  • test/suites/standard2/feature-gates.robot
  • test/suites/telemetry/telemetry.robot

@pacevedom pacevedom changed the title WIP: Test restart optimizations NO-ISSUE: Test restart optimizations Jul 9, 2026
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 9, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@pacevedom: This pull request explicitly references no jira issue.

Details

In response to this:

Summary by CodeRabbit

  • Bug Fixes
  • Improved test cleanup consistency across multiple configuration suites.
  • Restarts now happen at suite teardown in several cases, reducing leftover state between tests.
  • Updated configuration cleanup so removed settings are fully cleared before the next test runs.
  • Adjusted telemetry and feature-gate test teardown flow to better restore a clean system state.

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.

@pacevedom pacevedom marked this pull request as ready for review July 9, 2026 13:35
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 9, 2026
@openshift-ci openshift-ci Bot requested review from eslutsky and pmtk July 9, 2026 13:40
@pacevedom

Copy link
Copy Markdown
Contributor Author

/retest

@openshift-ci

openshift-ci Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

@pacevedom: 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.

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants