Skip to content

Add STP for CNV-68270: OVN-K MTU defaults#43

Open
guyoron1 wants to merge 1 commit intoRedHatQE:mainfrom
guyoron1:CNV-68270-ovn-mtu-defaults-stp
Open

Add STP for CNV-68270: OVN-K MTU defaults#43
guyoron1 wants to merge 1 commit intoRedHatQE:mainfrom
guyoron1:CNV-68270-ovn-mtu-defaults-stp

Conversation

@guyoron1
Copy link
Collaborator

@guyoron1 guyoron1 commented Mar 2, 2026

Summary

  • Add Software Test Plan (STP) for CNV-68270: Document Better MTU Defaults for OVN Kubernetes (Closed Loop)
  • Validates corrected MTU documentation values for OVN-K secondary networks: layer 2 overlay (MTU 1400) and localnet (MTU 1500)
  • 16 test scenarios (7 Tier 1 functional, 9 Tier 2 end-to-end) covering connectivity validation, CNO default MTU, backward compatibility, MTU enforcement, PMTUD, and documentation correctness

Test plan

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • Tests

    • Added comprehensive test suite for OVN MTU default verification across Layer 2 overlay and Localnet configurations, including backward compatibility and multi-VM scenarios.
  • Documentation

    • Added detailed test plan documenting MTU default validation strategy, test scenarios, and acceptance criteria.

Add Software Test Plan (STP) and test stubs (STD) for CNV-68270:
Document Better MTU Defaults for OVN Kubernetes (Closed Loop).

Covers 16 test scenarios (7 Tier 1 functional, 9 Tier 2 end-to-end)
validating corrected MTU documentation values for layer 2 overlay
(MTU 1400) and localnet (MTU 1500) OVN-Kubernetes secondary networks.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@openshift-virtualization-qe-bot

Report bugs in Issues

Welcome! 🎉

This pull request will be automatically processed with the following features:

🔄 Automatic Actions

  • Reviewer Assignment: Reviewers are automatically assigned based on the OWNERS file in the repository root
  • Size Labeling: PR size labels (XS, S, M, L, XL, XXL) are automatically applied based on changes
  • Issue Creation: A tracking issue is created for this PR and will be closed when the PR is merged or closed
  • Branch Labeling: Branch-specific labels are applied to track the target branch
  • Auto-verification: Auto-verified users have their PRs automatically marked as verified
  • Labels: Enabled categories: branch, can-be-merged, cherry-pick, has-conflicts, hold, needs-rebase, size, verified, wip

📋 Available Commands

PR Status Management

  • /wip - Mark PR as work in progress (adds WIP: prefix to title)
  • /wip cancel - Remove work in progress status
  • /hold - Block PR merging (approvers only)
  • /hold cancel - Unblock PR merging
  • /verified - Mark PR as verified
  • /verified cancel - Remove verification status
  • /reprocess - Trigger complete PR workflow reprocessing (useful if webhook failed or configuration changed)
  • /regenerate-welcome - Regenerate this welcome message

Review & Approval

  • /lgtm - Approve changes (looks good to me)
  • /approve - Approve PR (approvers only)
  • /assign-reviewers - Assign reviewers based on OWNERS file
  • /assign-reviewer @username - Assign specific reviewer
  • /check-can-merge - Check if PR meets merge requirements

Testing & Validation

  • /retest tox - Run Python test suite with tox
  • /retest all - Run all available tests

Cherry-pick Operations

  • /cherry-pick <branch> - Schedule cherry-pick to target branch when PR is merged
    • Multiple branches: /cherry-pick branch1 branch2 branch3

Label Management

  • /<label-name> - Add a label to the PR
  • /<label-name> cancel - Remove a label from the PR

✅ Merge Requirements

This PR will be automatically approved when the following conditions are met:

  1. Approval: /approve from at least one approver
  2. LGTM Count: Minimum 2 /lgtm from reviewers
  3. Status Checks: All required status checks must pass
  4. No Blockers: No WIP, hold, conflict labels
  5. Verified: PR must be marked as verified (if verification is enabled)

📊 Review Process

Approvers and Reviewers

Approvers:

  • EdDev
  • rnetser

Reviewers:

  • Anatw
  • EdDev
  • azhivovk
  • rnetser
  • servolkov
  • yossisegev
Available Labels
  • hold
  • verified
  • wip
  • lgtm
  • approve

💡 Tips

  • WIP Status: Use /wip when your PR is not ready for review
  • Verification: The verified label is automatically removed on each new commit
  • Cherry-picking: Cherry-pick labels are processed when the PR is merged
  • Permission Levels: Some commands require approver permissions
  • Auto-verified Users: Certain users have automatic verification and merge privileges

For more information, please refer to the project documentation or contact the maintainers.

@coderabbitai
Copy link

coderabbitai bot commented Mar 2, 2026

Walkthrough

This pull request introduces a new test module with stub test classes for OVN MTU defaults validation and an accompanying test plan document. The module defines test cases across eight categories (Layer 2 overlay, Localnet, CNO defaults, backward compatibility, PMTUD, documentation, multi-VM, and cross-version scenarios) with detailed docstrings but no implementation logic.

Changes

Cohort / File(s) Summary
Test Implementation Stubs
stds/sig-network/test_ovn_mtu_defaults.py
New test module with 8 test classes and multiple test methods covering OVN MTU defaults validation scenarios. All methods contain placeholder bodies (pass). Module marked non-collectible via __test__ = False.
Test Planning Documentation
stps/sig-network/ovn-mtu-defaults-stp.md
New test plan document outlining QE test strategy for MTU default corrections in OVN-Kubernetes Layer 2 overlay and Localnet topologies, including motivation, scope, test scenarios, traceability, and acceptance criteria.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add STP for CNV-68270: OVN-K MTU defaults' accurately captures the main change—adding a Software Test Plan document for MTU default verification in OVN Kubernetes, as confirmed by the summary showing both the STP document and test stubs.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
stds/sig-network/test_ovn_mtu_defaults.py (1)

123-123: Class name includes "MTU1300" which is inconsistent with other class names.

Most test classes follow the pattern Test<Feature>MTU<Value> (e.g., TestLayer2OverlayMTU1400, TestLocalnetMTU1500), but this class is named TestBackwardCompatibilityMTU1300 while the STP document and AI summary reference it as TestBackwardCompatibilityMTU. Consider renaming to TestBackwardCompatibilityMTU for consistency with the STP document, since the "1300" value is already clear from the test method name and docstring.

♻️ Optional: Rename class for consistency with STP
-class TestBackwardCompatibilityMTU1300:
+class TestBackwardCompatibilityMTU:
     """End-to-end backward compatibility tests with old MTU 1300 value.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@stds/sig-network/test_ovn_mtu_defaults.py` at line 123, Rename the test class
TestBackwardCompatibilityMTU1300 to TestBackwardCompatibilityMTU to match the
project's test-class naming convention; update the class declaration (class
TestBackwardCompatibilityMTU1300 -> class TestBackwardCompatibilityMTU) and any
references to that class (e.g., in test discovery or imports) while leaving the
test method names and docstring (which already specify the 1300 value)
unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@stds/sig-network/test_ovn_mtu_defaults.py`:
- Line 123: Rename the test class TestBackwardCompatibilityMTU1300 to
TestBackwardCompatibilityMTU to match the project's test-class naming
convention; update the class declaration (class TestBackwardCompatibilityMTU1300
-> class TestBackwardCompatibilityMTU) and any references to that class (e.g.,
in test discovery or imports) while leaving the test method names and docstring
(which already specify the 1300 value) unchanged.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ae42e3c and df51b27.

📒 Files selected for processing (2)
  • stds/sig-network/test_ovn_mtu_defaults.py
  • stps/sig-network/ovn-mtu-defaults-stp.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants