feat(must-gather): add must-gather generate#223
feat(must-gather): add must-gather generate#223Meina-rh wants to merge 1 commit intoopenshift-eng:mainfrom
Conversation
WalkthroughAdds generation documentation to the must-gather plugin: README updated to include /must-gather:generate and generate-related workflow notes; a new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches✅ Passed checks (7 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
|
Hi @Meina-rh. Thanks for your PR. I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
|
/ok-to-test |
|
@Meina-rh: Cannot trigger testing until a trusted user reviews the PR and leaves an 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 kubernetes-sigs/prow repository. |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
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
📒 Files selected for processing (2)
plugins/must-gather/README.md(2 hunks)plugins/must-gather/commands/generate.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
plugins/must-gather/commands/generate.md
10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
56-56: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
62-62: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
67-67: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
72-72: 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)
93-93: Bare URL used
(MD034, no-bare-urls)
plugins/must-gather/README.md
259-259: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (2)
plugins/must-gather/commands/generate.md (1)
14-46: Clarify whether Implementation section is prescriptive or descriptive.The Implementation section (lines 21–46) reads as detailed pseudocode or a specification for how the command should be implemented. If this documentation is meant to guide the actual implementation of the
/must-gather:generatecommand, ensure that the implementation in the actual plugin code strictly follows these steps, particularly around error handling for prerequisites and image determination logic. If this is already implemented, confirm that the code matches this documented behavior.plugins/must-gather/README.md (1)
256-282: Documentation integration is well-structured and consistent.The new
/must-gather:generatecommand is well-integrated into the README with clear examples, requirements, and workflow guidance. The documentation aligns with the detailed specification ingenerate.md, creating a good hierarchy: brief overview in README, complete reference in command documentation. The end-to-end workflow (generate → analyze) is clearly described and reinforced across both documents.
|
|
||
| - `/must-gather:analyze` - Analyze collected must-gather data | ||
| - `oc adm must-gather --help` - For all available options | ||
| - OpenShift documentation: https://docs.openshift.com/container-platform/latest/support/gathering-cluster-data.html |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Wrap bare URL in link syntax.
Line 93 contains a bare URL flagged by markdownlint (MD034). Convert it to proper markdown link syntax for consistency and to resolve the linting issue.
- OpenShift documentation: https://docs.openshift.com/container-platform/latest/support/gathering-cluster-data.html
+ OpenShift documentation: [Gathering cluster data](https://docs.openshift.com/container-platform/latest/support/gathering-cluster-data.html)📝 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.
| - OpenShift documentation: https://docs.openshift.com/container-platform/latest/support/gathering-cluster-data.html | |
| - OpenShift documentation: [Gathering cluster data](https://docs.openshift.com/container-platform/latest/support/gathering-cluster-data.html) |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
93-93: Bare URL used
(MD034, no-bare-urls)
🤖 Prompt for AI Agents
In plugins/must-gather/commands/generate.md around line 93, there's a bare URL
("https://docs.openshift.com/container-platform/latest/support/gathering-cluster-data.html")
causing markdownlint MD034; replace it with proper Markdown link syntax by
wrapping the URL in square brackets with a descriptive label followed by the URL
in parentheses (e.g., [OpenShift
documentation](https://docs.openshift.com/container-platform/latest/support/gathering-cluster-data.html))
so the link renders and lint passes.
|
@bentito PTAL, thanks |
| ## Overview | ||
|
|
||
| This plugin provides tools to analyze must-gather data collected from OpenShift clusters, displaying resource status in familiar `oc`-like format and identifying cluster issues. | ||
| This plugin provides tools to generate and analyze must-gather data from OpenShift clusters, displaying resource status in familiar `oc`-like format and identifying cluster issues. |
There was a problem hiding this comment.
It would be good expand the README a bit to disambiguate with the other must-gather related helpers:
• cluster collection (/must-gather:generate)
• analysis of existing data (/must-gather:analyze)
• CI artifact extraction (/prow-job:extract-must-gather) 
There was a problem hiding this comment.
@bentito thanks for reviewing this. I've added some contexts in README.md
|
/ok-to-test |
Signed-off-by: Meina-rh <meinli@redhat.com>
1c08a25 to
4cef15a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
plugins/must-gather/commands/generate.md (1)
89-93: Wrap bare URL in markdown link syntax (MD034 violation).Line 93 contains a bare URL, which violates markdownlint's MD034 rule. Wrap it in markdown link syntax with a descriptive label.
🔎 Proposed fix
## See Also - `/must-gather:analyze` - Analyze collected must-gather data - `oc adm must-gather --help` - For all available options - - OpenShift documentation: https://docs.openshift.com/container-platform/latest/support/gathering-cluster-data.html + - [OpenShift documentation: Gathering cluster data](https://docs.openshift.com/container-platform/latest/support/gathering-cluster-data.html)
📜 Review details
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
📒 Files selected for processing (2)
plugins/must-gather/README.md(2 hunks)plugins/must-gather/commands/generate.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
plugins/must-gather/commands/generate.md
10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
56-56: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
62-62: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
67-67: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
72-72: 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)
93-93: Bare URL used
(MD034, no-bare-urls)
plugins/must-gather/README.md
262-262: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (4)
plugins/must-gather/README.md (2)
1-11: Good! Overview improvements directly address disambiguation feedback.The expanded overview clearly separates the three distinct must-gather-related workflows (generation, analysis, and CI extraction), which addresses the prior feedback to disambiguate between capabilities. This helps users understand exactly what this plugin does.
287-303: LGTM!The /must-gather:analyze documentation is clear and includes helpful examples of both basic and component-specific analysis.
plugins/must-gather/commands/generate.md (2)
21-46: LGTM!The implementation section provides a clear, step-by-step procedural guide that covers all necessary prerequisites, image selection logic, and post-collection workflows. The inclusion of the optional analysis step aligns well with the end-to-end generate + analyze workflow described in the PR.
48-80: LGTM!The return values are clearly documented, and the five examples comprehensively cover common usage patterns. Example 5 particularly stands out for demonstrating the generate-then-analyze workflow, which aligns with the PR's goal of providing end-to-end must-gather functionality.
| ``` | ||
| /must-gather:generate [image] [--dest-dir path] | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "generate.md" -path "*/must-gather/*" 2>/dev/null | head -20Repository: openshift-eng/ai-helpers
Length of output: 110
🏁 Script executed:
cat -n ./plugins/must-gather/commands/generate.mdRepository: openshift-eng/ai-helpers
Length of output: 4157
Add language identifier to fenced code blocks (MD040 violations).
The file contains six code blocks lacking language identifiers. All code blocks display shell command examples and should be marked with ```bash. Affected lines: 10, 56, 62, 67, 72, and 77.
## Synopsis
- ```
+ ```bash
/must-gather:generate [image] [--dest-dir path]- Generate must-gather with default image:
-
/must-gather:generate
- Generate must-gather with specific image:
-
/must-gather:generate registry.redhat.io/openshift4/ose-must-gather:v4.14
- Generate must-gather to specific directory:
-
/must-gather:generate --dest-dir /tmp/my-must-gather
- Generate must-gather with custom image (e.g., network diagnostics):
-
/must-gather:generate quay.io/openshift/origin-must-gather:latest
- Generate and then analyze:
-
/must-gather:generate
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.18.1)</summary>
10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In plugins/must-gather/commands/generate.md around lines 10 to 12 (and also
affecting lines 56, 62, 67, 72, 77), several fenced code blocks lack a language
identifier; update each triple-backtick fence that shows shell command examples
to use bash so all six code blocks are marked as bash (e.g., change to
highlighting across the file.| #### `/must-gather:generate [image] [--dest-dir path]` | ||
| Generates must-gather diagnostic data from an OpenShift cluster. | ||
|
|
||
| ``` | ||
| # Generate with default image | ||
| /must-gather:generate | ||
|
|
||
| # Use specific must-gather image | ||
| /must-gather:generate registry.redhat.io/openshift4/ose-must-gather:v4.14 | ||
|
|
||
| # Specify output directory | ||
| /must-gather:generate --dest-dir /tmp/my-must-gather | ||
| ``` | ||
|
|
||
| Collects diagnostic information including: | ||
| - Cluster resource definitions | ||
| - Operator logs and status | ||
| - Pod logs and events | ||
| - Network configuration | ||
| - Storage information | ||
|
|
||
| Requires: | ||
| - `oc` CLI installed and configured | ||
| - Active connection to an OpenShift cluster | ||
| - Cluster-admin or appropriate permissions | ||
|
|
||
| Output is saved to `/tmp/must-gather-<timestamp>/` by default. After collection, you can use `/must-gather:analyze` to analyze the data. | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the file exists and examine lines 259-286
if [ -f "plugins/must-gather/README.md" ]; then
wc -l "plugins/must-gather/README.md"
echo "---"
sed -n '259,286p' "plugins/must-gather/README.md"
else
echo "File not found"
fiRepository: openshift-eng/ai-helpers
Length of output: 920
Add language identifier to code block (MD040 violation).
The fenced code block at line 262 lacks a language identifier, which violates markdownlint's MD040 rule. Change the opening fence from ``` to ```bash to specify bash language for proper syntax highlighting.
#### `/must-gather:generate [image] [--dest-dir path]`
Generates must-gather diagnostic data from an OpenShift cluster.
- ```
+ ```bash
# Generate with default image
/must-gather:generate
# Use specific must-gather image
/must-gather:generate registry.redhat.io/openshift4/ose-must-gather:v4.14
# Specify output directory
/must-gather:generate --dest-dir /tmp/my-must-gather
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.18.1)</summary>
262-262: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In plugins/must-gather/README.md around lines 259 to 286, the fenced code block
starting at line 262 has no language identifier causing an MD040 markdownlint
violation; update the opening fence to include a language (e.g., bash) so the
block becomes ```bash and keep the existing content and closing fence unchanged
to enable proper syntax highlighting.
</details>
<!-- fingerprinting:phantom:triton:mongoose -->
<!-- This is an auto-generated comment by CodeRabbit -->
|
/test lint |
|
@Meina-rh: The specified target(s) for Use 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 kubernetes-sigs/prow repository. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bentito, Meina-rh 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 |
What this PR does / why we need it:
This PR adds
/must-gather:generatecommand and interactively prompts to analyze data with/must-gather:analyzeafter collectionWhich issue(s) this PR fixes:
Special notes for your reviewer:
Checklist:
Summary by CodeRabbit
New Features
/must-gather:generatecommand to create diagnostic bundles from OpenShift clusters, enabling a complete generate-and-analyze workflow.Documentation
✏️ Tip: You can customize this high-level summary in your review settings.