MG-334: add user docs - #377
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@neha037: This pull request references MG-334 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 story to target the "5.0.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
WalkthroughThis documentation change request defines GA updates for Support Log Gather modules, including permission guidance, installation examples, CLI configuration procedures, corrected MustGather examples, parameter reference updates, validation notes, and writer checklists. ChangesSupport Log Gather documentation
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: neha037 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/MG-334-support-log-gather-docs-update.md (1)
180-190: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winLink the bundled RBAC manifests. The controller only checks that the service account exists; point users to the
must-gather-adminClusterRole/ClusterRoleBinding so the prerequisite is actionable.🤖 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 `@docs/MG-334-support-log-gather-docs-update.md` around lines 180 - 190, Update the service-account prerequisite near the MustGather CR requirements to link to the bundled must-gather-admin ClusterRole and ClusterRoleBinding manifests, instructing users to apply or configure them for the service account. Keep the existing namespace and permissions guidance intact.
🤖 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.
Inline comments:
In `@docs/MG-334-support-log-gather-docs-update.md`:
- Around line 304-305: Update the oc verification commands in this
documentation, including the must-gather lookup and the referenced command
ranges, to explicitly target the must-gather-operator namespace. Preserve the
existing resource names and output formats while ensuring every check queries
the namespace where the examples create resources.
- Around line 193-195: Remove the invalid proxyConfig field from the example
MustGather YAML and all later duplicated examples in this document. Keep proxy
configuration represented only through the operator environment variables
HTTP_PROXY, HTTPS_PROXY, and NO_PROXY.
---
Nitpick comments:
In `@docs/MG-334-support-log-gather-docs-update.md`:
- Around line 180-190: Update the service-account prerequisite near the
MustGather CR requirements to link to the bundled must-gather-admin ClusterRole
and ClusterRoleBinding manifests, instructing users to apply or configure them
for the service account. Keep the existing namespace and permissions guidance
intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: be5d1b8e-f824-425e-9fe2-67a86bd8bf9e
📒 Files selected for processing (1)
docs/MG-334-support-log-gather-docs-update.md
| ### Change: REPLACE — Example MustGather YAML (remove invalid `proxyConfig`) | ||
|
|
||
| **Current problem:** The example includes `proxyConfig`, which is **not** a field on `MustGather` / `MustGatherSpec`. Proxy settings come from the Operator pod environment (cluster proxy). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Relevant files ==\n'
git ls-files | rg '(^|/)(docs/MG-334-support-log-gather-docs-update\.md|.*mustgather.*|.*MustGather.*|.*proxy.*|.*crd.*\.ya?ml)$' || true
printf '\n== Search for proxyConfig and MustGather ==\n'
rg -n --hidden -S 'proxyConfig|MustGatherSpec|kind: MustGather|mustgather' . || trueRepository: openshift/must-gather-operator
Length of output: 50386
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== File size and outline for the docs file ==\n'
wc -l docs/MG-334-support-log-gather-docs-update.md
ast-grep outline docs/MG-334-support-log-gather-docs-update.md --view expanded || true
printf '\n== Show surrounding lines around the cited section ==\n'
sed -n '180,205p' docs/MG-334-support-log-gather-docs-update.mdRepository: openshift/must-gather-operator
Length of output: 1890
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Focused MustGather API files ==\n'
fd -a '.*' . | rg 'MustGather|mustgather|proxy' || true
printf '\n== Focused source search in likely API directories ==\n'
for d in api config pkg internal controllers docs; do
[ -d "$d" ] || continue
rg -n --hidden -S 'proxyConfig|type MustGather|MustGatherSpec|kind: MustGather' "$d" || true
doneRepository: openshift/must-gather-operator
Length of output: 10667
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== MustGatherSpec definition ==\n'
sed -n '1,120p' api/v1alpha1/mustgather_types.go
printf '\n== Proxy-related code paths ==\n'
rg -n --hidden -S 'HTTP_PROXY|HTTPS_PROXY|NO_PROXY|proxy|Proxy' controllers/mustgather api/v1alpha1 README.md docs/MG-334-support-log-gather-docs-update.md | sed -n '1,220p'Repository: openshift/must-gather-operator
Length of output: 26513
Remove proxyConfig from the examples. MustGatherSpec has no such field, and the operator already forwards HTTP_PROXY/HTTPS_PROXY/NO_PROXY from its environment to the upload container. Same issue appears in the later copies.
🤖 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 `@docs/MG-334-support-log-gather-docs-update.md` around lines 193 - 195, Remove
the invalid proxyConfig field from the example MustGather YAML and all later
duplicated examples in this document. Keep proxy configuration represented only
through the operator environment variables HTTP_PROXY, HTTPS_PROXY, and
NO_PROXY.
Source: MCP tools
| $ oc get mustgather example-mg -o yaml | ||
| ---- |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Scope verification commands to the CR namespace.
The examples create resources in must-gather-operator, but these commands use the current namespace and can report false “missing” results.
Proposed fix
-$ oc get mustgather
+$ oc get mustgather -n must-gather-operator
-$ oc get pods
+$ oc get pods -n must-gather-operator
-$ oc logs -f pod/<mustgather-pod-name> -c upload
+$ oc logs -n must-gather-operator -f pod/<mustgather-pod-name> -c uploadAlso applies to: 370-377, 384-384
🤖 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 `@docs/MG-334-support-log-gather-docs-update.md` around lines 304 - 305, Update
the oc verification commands in this documentation, including the must-gather
lookup and the referenced command ranges, to explicitly target the
must-gather-operator namespace. Preserve the existing resource names and output
formats while ensuring every check queries the namespace where the examples
create resources.
2 participants
Summary by CodeRabbit