Add OpenShift router inspection commands and shared skills#269
Add OpenShift router inspection commands and shared skills#269alebedev87 wants to merge 1 commit intoopenshift-eng:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: alebedev87 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 |
WalkthroughAdds three OpenShift router inspection commands (router-show-config, router-show-info, router-show-sessions), their command docs and plugin metadata updates, plus two reusable skill docs for oc prerequisites and router pod discovery/validation. Commands query router pods (HAProxy) and save outputs to .work. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Plugin as "Openshift Plugin"
participant OC as "`oc` CLI"
participant API as "OpenShift API"
participant Pod as "Router Pod"
participant HA as "HAProxy (admin socket)"
Note over User,Plugin: User invokes one of the new commands
User->>Plugin: /openshift:router-show-*
Plugin->>OC: run oc-prereqs (check oc, cluster)
alt oc present & cluster reachable
OC->>API: query/list router pods (router-pod-helper)
API-->>OC: pod list
OC->>Pod: exec socat / oc exec to HAProxy admin socket
Pod->>HA: admin socket request (e.g., "show sess all" / "show stat" / cat config)
HA-->>Pod: admin socket response
Pod-->>OC: command output
OC-->>Plugin: output + saved file path (.work/openshift/...)
Plugin-->>User: display output and file location
else failure
OC-->>Plugin: error
Plugin-->>User: error message
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Pre-merge checks✅ Passed checks (7 passed)
📜 Recent review detailsConfiguration 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 (7)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🪛 markdownlint-cli2 (0.18.1)plugins/openshift/commands/router-show-config.md10-10: Fenced code blocks should have a language specified (MD040, fenced-code-language) plugins/openshift/commands/router-show-sessions.md10-10: Fenced code blocks should have a language specified (MD040, fenced-code-language) 🔇 Additional comments (4)
Comment |
openshift-ci
bot
added
the
needs-ok-to-test
|
Hi @alebedev87. Thanks for your PR. I'm waiting for a openshift-eng 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI Agents
In @plugins/openshift/commands/router-show-info.md:
- Line 10: Several fenced code blocks in the router-show-info docs are missing
language specifiers; update the command synopsis fence that contains
"/openshift:router-show-info [router-pod-name]" to use ```bash and update the
example output fences that contain "/openshift:router-show-info" to use ```text
so they render with proper syntax highlighting and readability; locate these
fences around the command synopsis (the block with the router-show-info
invocation) and the example output blocks (the example output occurrences) and
add the appropriate language identifiers.
- Line 30: Replace the bare OpenShift mirror URL in the "OpenShift CLI (oc):
Required to access router pods" list item with a proper Markdown link by
wrapping the URL in [link text](https://...) syntax; use descriptive link text
like "OpenShift mirror" or "OpenShift CLI download" and ensure the URL is the
actual mirror address so the item becomes something like: OpenShift CLI (oc):
Required to access router pods — OpenShift mirror (https://...).
- Line 81: The example command using socat to query HAProxy ("oc exec -n
$NAMESPACE $ROUTER_POD -- socat stdio /var/lib/haproxy/run/haproxy.sock <<<
\"show info\"") assumes socat exists in the router pod; update the documentation
to state that socat is not present by default and provide alternatives: instruct
to install socat into a UBI-based debug pod (e.g., use dnf install -y socat)
before running the command, or recommend running the command from a
network-debug image that includes socat, or suggest using oc port-forward to
access /var/lib/haproxy/run/haproxy.sock externally; mention the exact HAProxy
socket path (/var/lib/haproxy/run/haproxy.sock) and the oc exec example so
readers know where to apply the alternative.
🧹 Nitpick comments (5)
plugins/openshift/commands/router-show-config.md (1)
26-40: Consider referencing the oc-prereqs skill instead of duplicating prerequisites.The Prerequisites section duplicates content from the
oc-prereqsskill. Since this PR introduces that skill specifically for reusability, consider replacing this section with a brief reference like:"This command requires OpenShift CLI prerequisites. See the oc-prereqs skill for details on oc CLI installation and cluster connectivity requirements."
This maintains DRY principles and reduces maintenance burden if prerequisites change.
plugins/openshift/commands/router-show-info.md (1)
27-41: Consider referencing the oc-prereqs skill instead of duplicating prerequisites.Similar to
router-show-config.md, this Prerequisites section duplicates content from theoc-prereqsskill. Consider replacing with a brief reference to maintain consistency and reduce duplication.plugins/openshift/skills/oc-prereqs/SKILL.md (1)
1-136: LGTM! Well-structured reusable skill.The oc-prereqs skill is well-documented with clear examples and proper error handling. The progressive check pattern (CLI availability → connectivity → optional context) is logical and reusable.
Minor suggestion: Consider adding a "Commands Using This Skill" section at the end that lists the router commands (router-show-config, router-show-info, router-show-sessions) to help users understand where this skill is applied.
plugins/openshift/commands/router-show-sessions.md (1)
47-99: Document socat as a required dependency and add error handling.The approach using socat with the HAProxy Unix socket is standard Red Hat practice, but the documentation doesn't explicitly mention that
socatmust be available in the router pod. If it's absent, the command fails with an unclear error message.Consider:
- Adding socat to the Prerequisites section
- Adding error handling for the case when socat is unavailable
- Suggesting fallback approaches (e.g., if socat is missing, check
/var/lib/haproxy/conf/haproxy.configinstead)plugins/openshift/skills/router-pod-helper/SKILL.md (1)
33-53: Document the input parameter$router_pod_nameexplicitly.The conditional logic references
$router_pod_nameto optionally accept a specific pod name, but the documentation never states that this is an input parameter or how it should be provided. The "Environment Variables" section only documents outputs, not inputs.Add a new "Input Parameters" or "Configuration" section clarifying that
$router_pod_nameis an optional input variable. Specify that if provided, it will be used directly; otherwise, the skill auto-discovers the first available router pod. This is crucial for downstream commands that want to leverage this skill.🔎 Suggested addition to the Environment Variables section
Add this before or after the existing "Environment Variables" section (around line 156):
## Input Parameters This skill accepts one optional parameter: - `$router_pod_name` - (Optional) Name of a specific router pod to use. If not provided, the skill will auto-discover and use the first available router pod in the `openshift-ingress` namespace. ## Environment VariablesOr integrate into existing variable documentation:
## Input and Output Variables **Input Parameters:** - `$router_pod_name` - (Optional) Name of a specific router pod; if empty, auto-discover the first available pod **Output Variables (set by this skill):** - `$NAMESPACE` - Always `openshift-ingress` - `$ROUTER_POD` - Name of the selected router pod - `$POD_STATUS` - Status of the pod (should be "Running")
📜 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 (7)
PLUGINS.mddocs/data.jsonplugins/openshift/commands/router-show-config.mdplugins/openshift/commands/router-show-info.mdplugins/openshift/commands/router-show-sessions.mdplugins/openshift/skills/oc-prereqs/SKILL.mdplugins/openshift/skills/router-pod-helper/SKILL.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
plugins/openshift/commands/router-show-info.md
10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
30-30: Bare URL used
(MD034, no-bare-urls)
138-138: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
142-142: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
160-160: 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)
🔇 Additional comments (7)
docs/data.json (2)
708-725: LGTM! Router command entries are well-structured.The three new router inspection commands follow the established patterns and include consistent metadata. The optional
router-pod-nameargument design provides good flexibility.
742-752: Skill entries are properly structured and documented.The two new skill entries (
oc-prereqsandrouter-pod-helper) follow existing conventions with clear descriptions and correct IDs. The router-pod-helper skill documentation exists and is properly placed.PLUGINS.md (1)
247-249: LGTM! Command entries properly documented.The three new router commands are correctly added to the OpenShift plugin documentation with consistent formatting and proper placement.
plugins/openshift/commands/router-show-config.md (1)
72-80: No changes needed. The HAProxy configuration file path/var/lib/haproxy/conf/haproxy.configis the correct standard location specified in official OpenShift/OKD documentation for accessing the HAProxy configuration inside OpenShift router pods.plugins/openshift/skills/router-pod-helper/SKILL.md (3)
107-154: Complete example properly demonstrates the integrated workflow.The example integrates prerequisites (oc-prereqs checks) with pod discovery and validation in a logical sequence. The code is syntactically correct and shows the full flow that consuming commands should follow.
29-79: Pod discovery and validation logic is sound.The bash code for discovering router pods using label selectors, extracting the first pod via awk, and validating Running status is correct. Error handling (lines 41–45, 73–76) provides clear feedback for failure scenarios.
173-182: References to consuming commands are accurate per PR objectives.The three router inspection commands (
router-show-config,router-show-sessions,router-show-info) are correctly listed as commands that depend on this skill, aligning with the PR's goal of reusable infrastructure.
|
https://issues.redhat.com/browse/DPP-18905 is closed but it may take time to take effect. So, I'll have to recreate the PR to get ok-to-test. |
alebedev87
force-pushed
the
add-router-show-commands
branch
from
646bb17 to
123ef94
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI Agents
In @plugins/openshift/commands/router-show-config.md:
- Around line 9-12: The Markdown file
plugins/openshift/commands/router-show-config.md has multiple fenced code blocks
without language specifiers and a capitalization fix for the "Return Value"
heading; update the fenced blocks so command snippets (e.g., the Synopsis block
and usage examples like "/openshift:router-show-config") use ```bash and the
displayed output blocks in Examples and content-display sections use ```text,
ensuring each opening fence matches its closing fence, and also correct the
"Return Value" heading capitalization under the Return Value section.
- Around line 29-31: Replace the bare URL on the "Install from:" line under
"OpenShift CLI (oc)" with a markdown link; edit the text that currently reads
"Install from: https://mirror.openshift.com/pub/openshift-v4/clients/ocp/" to
use link syntax, e.g. "Install from: [OpenShift CLI
download](https://mirror.openshift.com/pub/openshift-v4/clients/ocp/)" so the
URL is not bare and matches the document's markdown link style.
In @plugins/openshift/commands/router-show-sessions.md:
- Around line 29-31: Replace the bare URL
"https://mirror.openshift.com/pub/openshift-v4/clients/ocp/" on the Install line
with a proper Markdown link; for example change the line that currently reads
"Install from: https://mirror.openshift.com/pub/openshift-v4/clients/ocp/" to
use link syntax such as "Install from: [OpenShift CLI (oc)
downloads](https://mirror.openshift.com/pub/openshift-v4/clients/ocp/)" so the
URL is not bare and is consistent with the rest of the document.
- Around line 138-147: In the example blocks in router-show-sessions.md, the
fenced code blocks following the "Output:" lines are missing language
specifiers; update each of those triple-backtick fences that wrap the example
output (the blocks showing "Finding router pods..." and "Using specified router
pod...") from ``` to ```text so the output is explicitly marked as plain text.
📜 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 (7)
PLUGINS.mddocs/data.jsonplugins/openshift/commands/router-show-config.mdplugins/openshift/commands/router-show-info.mdplugins/openshift/commands/router-show-sessions.mdplugins/openshift/skills/oc-prereqs/SKILL.mdplugins/openshift/skills/router-pod-helper/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (5)
- plugins/openshift/skills/router-pod-helper/SKILL.md
- plugins/openshift/skills/oc-prereqs/SKILL.md
- PLUGINS.md
- plugins/openshift/commands/router-show-info.md
- docs/data.json
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
plugins/openshift/commands/router-show-sessions.md
10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
30-30: Bare URL used
(MD034, no-bare-urls)
138-138: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
142-142: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
plugins/openshift/commands/router-show-config.md
31-31: Bare URL used
(MD034, no-bare-urls)
122-122: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
126-126: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
151-151: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
155-155: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
alebedev87
force-pushed
the
add-router-show-commands
branch
from
123ef94 to
b5b8c93
Compare
This commit adds three new commands to the OpenShift plugin for inspecting router configuration and runtime data from router pods: - /openshift:router-show-config - Display router configuration - /openshift:router-show-sessions - Display active sessions - /openshift:router-show-info - Display process information To eliminate code duplication, two shared skills were created: - oc-prereqs - Generic OpenShift CLI and cluster connectivity checks - router-pod-helper - Router-specific pod discovery and validation All three commands reference these skills for common prerequisites and pod discovery logic, making the implementation maintainable and reusable for future router-related commands. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
alebedev87
force-pushed
the
add-router-show-commands
branch
from
b5b8c93 to
5802cc1
Compare
|
New PR after I was added to |
1 participant
This commit adds three new commands to the OpenShift plugin for inspecting router configuration and runtime data from router pods:
To eliminate code duplication, two shared skills were created:
All three commands reference these skills for common prerequisites and pod discovery logic, making the implementation maintainable and reusable for future router-related commands.
🤖 Generated with Claude Code
What this PR does / why we need it:
Which issue(s) this PR fixes:
N/A
Special notes for your reviewer:
Checklist:
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.