must-gather: Add multus CNI analyzer #199
Open
+3,129
−389
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: weliang1 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Contributor
WalkthroughRestructures the Must-Gather plugin docs from script-focused to command-centric usage, adding a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (6 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
plugins/must-gather/commands/multus.md (2)
10-10: Specify language identifiers for all code blocks.Multiple fenced code blocks are missing language specifiers, which impacts syntax highlighting and readability. All the following blocks should specify their language:
- Line 10:
bash(Synopsis example)- Line 14:
bash(Usage example)- Line 42:
bash(Script path)- Line 56:
bash(Script discovery)- Line 122:
text(Output structure)- Line 131, 137, 143, 149:
bash(Examples)- Line 157, 164, 170, 177:
text(Error messages)Apply this diff to fix the code blocks:
## Synopsis
- bash
/must-gather:multus [must-gather-path] [output-html-path]For all other code blocks, add the appropriate language identifier (`bash`, `text`, `yaml`, etc.) on the first line after the opening triple backticks. Also applies to: 14-14, 42-42, 56-56, 122-122, 131-131, 137-137, 143-143, 149-149, 157-157, 164-164, 170-170, 177-177 --- `50-106`: **Reframe Implementation section to clarify whether steps are user-facing or internal.** The "Implementation" section mixes two distinct concerns: (1) how the command itself locates and invokes the script (lines 55–62), and (2) what the Python analyzer script does internally (lines 69–106). This blurs the distinction between command orchestration and script internals. For user-facing documentation, consider restructuring to emphasize what the user observes, or split into "Command Flow" (external behavior) and "Analysis Steps" (internal script logic) subsections. Alternatively, if these are instructions for command authors, move them to a separate implementation guide or inline comments. </blockquote></details> <details> <summary>plugins/must-gather/README.md (1)</summary><blockquote> `14-14`: **Specify language identifiers for all code blocks.** Multiple fenced code blocks are missing language specifiers. Add language identifiers to lines 14, 22, 36, 77, 82, 87, and 101: - Line 14, 22, 36: `bash` (command examples) - Line 77, 82, 87: `bash` (usage examples) - Line 101: `text` (failure analysis output) Apply this diff to fix the code blocks: ```diff **Usage:**
- bash
/must-gather:ovn-dbs [must-gather-path] [--node ] [--query ]Apply the same pattern to all other code blocks throughout the file. Also applies to: 22-22, 36-36, 77-77, 82-82, 87-87, 101-101 </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Pro **Cache: Disabled due to data retention organization setting** **Knowledge base: Disabled due to data retention organization setting** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between edabeae251c1c255112f7a60c5554c32d7dd3cb3 and 1fc94f17a4c085b04f0bc407346ec1b9050ccaf6. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `plugins/must-gather/README.md` (1 hunks) * `plugins/must-gather/commands/multus.md` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>plugins/must-gather/commands/multus.md</summary> 10-10: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 42-42: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 122-122: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 131-131: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 137-137: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 143-143: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 149-149: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 157-157: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 164-164: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 170-170: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 177-177: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> <details> <summary>plugins/must-gather/README.md</summary> 14-14: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 22-22: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 36-36: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 77-77: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 82-82: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 87-87: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 101-101: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>🔇 Additional comments (1)</summary><blockquote> <details> <summary>plugins/must-gather/README.md (1)</summary><blockquote> `18-38`: **Verify consistency between README and multus.md command documentation.** The command documentation in multus.md (the dedicated command file) provides extensive detail on behavior, implementation steps, error handling, and use cases. Ensure that any cross-references or examples in the README align with the detailed command documentation, especially around: - Output file naming and structure (HTML report + `.failure-analysis.txt` suffix) - Expected prerequisites and error messages - Usage patterns and examples </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Author
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
OpenShift networking teams frequently need to troubleshoot Multus CNI issues across different cluster configurations, but analyzing must-gather data manually is time-consuming and error-prone. This plugin addresses this by providing automated analysis of Multus infrastructure, NetworkAttachmentDefinitions (NADs), and multi-networked pod configurations.
This PR introduces the must-gather:multus command, a comprehensive tool for analyzing Multus CNI configuration and multi-networked pods from must-gather data without manual inspection.
Use Cases
This plugin can be used for troubleshooting and post-mortem analysis of Multus networking:
Key Features
Commands Added
/must-gather:multus <must-gather-file-path> <MULTUS_ANALYSIS_REPORT-path>Plugin Structure
What's Included
Analysis Capabilities
The analyzer examines:
Files Changed
Generated Reports Examples
https://file.corp.redhat.com/~weliang/MULTUS_ANALYSIS_REPORT1.html
https://file.corp.redhat.com/~weliang/MULTUS_ANALYSIS_REPORT2.html
https://file.corp.redhat.com/~weliang/MULTUS_ANALYSIS_REPORT3.html
https://file.corp.redhat.com/~weliang/MULTUS_ANALYSIS_REPORT4.html
https://file.corp.redhat.com/~weliang/MULTUS_ANALYSIS_REPORT5.html
Summary by CodeRabbit
Release Notes
New Features
/must-gather:multuscommand to analyze Multus CNI configuration and multi-networked pods from must-gather data.Documentation
✏️ Tip: You can customize this high-level summary in your review settings.