OSC plugins#22
Conversation
When an MCP request to OpenStack is received with incorrect arguments we fail to return the error in the `stderr` field so it's always an empty string. This is because argparse ignores the `stderr` argument we pass to the shell itself. With this patch we fix it by ensuring we redirect the whole stderr when running the command.
We are currently seeing some errors logged multiple times. With this patch we remove the cliff log handlers when running commands removing duplicated errors on the console.
Some LLMs don't understand that available tools are not shell commands, so they send shell commands with pipes. In this patch we try to guard against such explicitly errors by explicitly stating that the tools are not shell.
The `get-crc-creds.sh` script expects the currently selected project to be `openstack`, but that's unlikely to happen if we are doing things on the `openstack-lightspeed` namespace. In this patch we update the script so it explicit sets the namespace on the `oc` commands.
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe PR parameterizes the container build, adds an OSC plugin dependency group, updates OpenStack CLI shell behavior for plugin discovery and stderr handling, expands command filtering for new plugin namespaces, and scopes CRC credential copies to the ChangesOSC Plugin Support and Container Build Restructure
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/rhos_ls_mcps/osc.py (1)
195-220: 🩺 Stability & Availability | 🟡 MinorRestore a real docstring here
An f-string in the first statement does not populate
__doc__, so the tool description andget_installed_plugins()output are dropped for docstring-based tool registration. Use a plain docstring and assign the rendered plugin text afterward, or setopenstack_cli_mcp_tool.__doc__explicitly.🤖 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 `@src/rhos_ls_mcps/osc.py` around lines 195 - 220, The first statement in the `openstack_cli_mcp_tool` definition is an f-string, so it does not become the function’s `__doc__` and the tool registration loses the description and plugin text. Change it to a plain docstring for the static documentation, then append the `get_installed_plugins()` output separately or assign `openstack_cli_mcp_tool.__doc__` explicitly after definition so the full tool description is preserved.
🤖 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 `@src/rhos_ls_mcps/osc.py`:
- Around line 156-195: Update the docstring in get_installed_plugins to fix the
wording typos: change “if there are not OSC plugins” to “if there are no OSC
plugins,” and make “The list ready to be injected” read “The list is ready to be
injected.” Keep the rest of the docstring unchanged and ensure the wording
remains clear and grammatical.
- Around line 183-190: The get_installed_plugins() logic in osc.py assumes every
entry point name exists in plugin_names, which can raise a KeyError for unknown
or future extensions. Update the list construction around
entry_points(group="openstack.cli.extension") to look up names safely and skip
or fall back for missing keys so only known plugin names are included. Keep the
existing return format in get_installed_plugins(), but ensure unknown entry
point names do not crash the docstring rendering path.
---
Outside diff comments:
In `@src/rhos_ls_mcps/osc.py`:
- Around line 195-220: The first statement in the `openstack_cli_mcp_tool`
definition is an f-string, so it does not become the function’s `__doc__` and
the tool registration loses the description and plugin text. Change it to a
plain docstring for the static documentation, then append the
`get_installed_plugins()` output separately or assign
`openstack_cli_mcp_tool.__doc__` explicitly after definition so the full tool
description is preserved.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b2695a13-b02c-4ee7-aa5e-83a1f83788c2
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
Containerfilepyproject.tomlscripts/allow-deny-list.pyscripts/allow-deny-list.yamlscripts/get-crc-creds.shsrc/rhos_ls_mcps/oc.pysrc/rhos_ls_mcps/osc.py
| def get_installed_plugins(): | ||
| """List installed OSC plugins | ||
|
|
||
| Return a string with a list of installed OSC plugins in an LLM friendly | ||
| format. The list ready to be injected into a docstring. | ||
|
|
||
| Return an empty string if there are not OSC plugins. | ||
| """ | ||
|
|
||
| from importlib.metadata import entry_points | ||
|
|
||
| plugin_names = { | ||
| "metric": "AODH (metrics)", | ||
| "placement": "Placement", | ||
| "key_manager": "Barbican (key manager)", | ||
| "rating": "CloudKitty (rating)", | ||
| "dns": "Designate (DNS)", | ||
| "orchestration": "Heat (orchestration)", | ||
| "baremetal-introsp": "Ironic Inspector (baremetal introspection)", | ||
| "baremetal": "Ironic (baremetal)", | ||
| "share": "Manila (shared file systems)", | ||
| "neutronclient": "Neutron (networking)", | ||
| "observabilityclient": "Prometheus (observability)", | ||
| "load_balancer": "Octavia (load balancer)", | ||
| "infra_optim": "Watcher (infra optimization)", | ||
| } | ||
|
|
||
| names = [ | ||
| plugin_names[ep.name] | ||
| for ep in entry_points(group="openstack.cli.extension") | ||
| ] | ||
|
|
||
| if names: | ||
| return "\nInstalled extra plugins:\n- " + "\n- ".join(names) | ||
| return "" | ||
|
|
||
|
|
||
| @tool_logger | ||
| async def openstack_cli_mcp_tool(command_str: str, ctx: Context) -> str: | ||
| """Run an OpenStackClient (OSC) CLI command | ||
| f"""Run an OpenStackClient (OSC) CLI command |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Minor: docstring typo — "if there are not OSC plugins" should read "if there are no OSC plugins", and "The list ready to be injected" should be "The list is ready to be injected".
🧰 Tools
🪛 GitHub Actions: Pre-commit checks / 0_pre-commit.txt
[error] 181-181: Pre-commit hook 'ruff-format' failed (files were modified). Ruff formatted code in this file and CI exited with code 1. Diff indicates a list-comprehension was reformatted.
🤖 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 `@src/rhos_ls_mcps/osc.py` around lines 156 - 195, Update the docstring in
get_installed_plugins to fix the wording typos: change “if there are not OSC
plugins” to “if there are no OSC plugins,” and make “The list ready to be
injected” read “The list is ready to be injected.” Keep the rest of the
docstring unchanged and ensure the wording remains clear and grammatical.
This commit changes our container image because it's currently missing almost all the OSC plugins, unlike the `openstackclient` image. In this patch we ensure the image includes the plugins by: * Adding the plugins to the project dependencies in the `osc-plugin` dependency group. * Changing the source for all our openstack related packages, so instead of using PyPi to install them we use RPMs by using the `openstackclient` container image as the base. * We use the Centos 10 base image of the `openstackclient` container so it has Python 3.12 instead of 3.9 that is in the Centos 9 container (quay.io/podified-antelope-centos9/openstack-openstackclient:current-podified) because the MCP python library has a minimum requirement of Python 3.10. This not only resolves the missing plugins, but also ensures that the packages used are not coming from PyPi, facilitating a more secure downstream release. Available plugins are sent as part of the `openstack-cli` MCP tool description to help LLMs know what is installed, as they may assume only basic commands are available. We also update the allowed commands on read-only mode.
1 participant
This commit changes our container image because it's currently missing
almost all the OSC plugins, unlike the
openstackclientimage.In this patch we ensure the image includes the plugins by:
Adding the plugins to the project dependencies in the
osc-plugindependency group.
Changing the source for all our openstack related packages, so instead
of using PyPi to install them we use RPMs by using the
openstackclientcontainer image as the base.We use the Centos 10 base image of the
openstackclientcontainer soit has Python 3.12 instead of 3.9 that is in the Centos 9 container
(quay.io/podified-antelope-centos9/openstack-openstackclient:current-podified)
because the MCP python library has a minimum requirement of Python
3.10.
This not only resolves the missing plugins, but also ensures that the
packages used are not coming from PyPi, facilitating a more
secure downstream release.
Available plugins are sent as part of the
openstack-cliMCP tooldescription to help LLMs know what is installed, as they may assume only
basic commands are available.
We also update the allowed commands on read-only mode.
In adition the PR includes a couple of minor improvements to the MCP tool.
Summary by CodeRabbit
New Features
ocCLI for smoother startup.Bug Fixes
Chores / Build