OU-1389: update devspace setup#1073
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@PeterYurkovich: This pull request references OU-1389 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 task 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
approved
WalkthroughDevelopment support now covers Monitoring and Cluster Observability plugin workflows through plugin-aware builds, dedicated backend targets, operator lifecycle scripts, selectable Devspace pipelines, updated documentation, and a Go 1.26 development image. ChangesDual plugin Devspace workflow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Developer
participant Devspace
participant install_coo.sh
participant create_uiplugin.sh
participant app-mp_or_app-mcp
participant Makefile
Developer->>Devspace: Select mp or mcp and run devspace dev
Devspace->>install_coo.sh: Install COO for mcp
Devspace->>create_uiplugin.sh: Configure monitoring UIPlugin for mcp
Devspace->>app-mp_or_app-mcp: Start selected development container
app-mp_or_app-mcp->>Makefile: Run matching backend target
Developer->>Devspace: Run devspace purge
Devspace->>app-mp_or_app-mcp: Stop development containers
Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 6
🤖 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 `@devspace.yaml`:
- Around line 16-28: Validate PLUGIN before the branch in the dev flow,
accepting only the supported mp and mcp values. If it is overridden with any
other value, fail immediately before running installation, UIPlugin, scaling, or
start_dev commands; preserve the existing branch behavior for valid values.
In `@hack/create-uiplugin.sh`:
- Around line 16-21: Align the UIPlugin `incidents` setting with the backend
feature configuration by either enabling `incidents` in the
`MCP_DEVSPACE_FEATURES` Makefile target or disabling `incidents` in this CR.
Ensure the MCP DevSpace UI does not expose a feature unavailable in the backend.
In `@hack/install-coo.sh`:
- Around line 40-49: The COO wait loop currently checks an arbitrary CSV via
`.items[0]`. Update the loop to read `.status.installedCSV` from the
`cluster-observability-operator` Subscription in
`openshift-cluster-observability-operator`, then query that named CSV’s
status.phase and continue waiting until it is Succeeded while preserving the
existing timeout behavior.
In `@hack/scale-up-cmo.sh`:
- Around line 4-6: Update the ClusterVersion restoration logic in the scale-up
script so it does not remove /spec/overrides wholesale. Restore the original
overrides array captured during scale-down, or remove only the monitoring-plugin
override introduced by scale-down-cmo.sh, while preserving all unrelated
existing overrides.
In `@README.md`:
- Around line 69-71: Correct the grammar in the DevSpace documentation: change
“it's own pod” to “its own pod” and “they will be rebuild” to “they will be
rebuilt,” without altering the surrounding technical content.
In `@web/webpack.config.ts`:
- Around line 107-109: Update the Perses route construction in the webpack
configuration to use the package plugin name when CONSOLE_PLUGIN_NAME is empty
as well as unset. Replace the nullish fallback in the PERSES_PROXY_BASE_URL
definition with the same non-empty fallback used nearby, preserving alignment
with pkg.consolePlugin.name.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: f45561fd-d29c-4a7c-b95e-810f2d0d3ff9
📒 Files selected for processing (11)
Dockerfile.devspaceMakefileREADME.mddevspace.yamlhack/create-uiplugin.shhack/install-coo.shhack/scale-down-cmo.shhack/scale-down-coo.shhack/scale-up-cmo.shhack/scale-up-coo.shweb/webpack.config.ts
2224822 to
1f14609
Compare
|
/label qe-approved |
openshift-ci
Bot
added
the
qe-approved
d9b0b7a to
1f14609
Compare
|
/retest |
|
|
||
| .PHONY: start-mcp-frontend | ||
| start-mcp-frontend: | ||
| cd web && CONSOLE_PLUGIN_NAME=monitoring-console-plugin npm run start |
There was a problem hiding this comment.
should we set the I18N_NAMESPACE here as well?
There was a problem hiding this comment.
The initial answer to this was no due to the naming of our translation files, but you helped me realize that we can actually improve our build process more. The initial change in the webpack config that this PR contained already performed the first of 2 actions that the update-plugin-name.sh script performed, and I added the second as well. Now rather than need to run that script before building the monitoring-console-plugin in our dockerfiles we can just set the ENV variable and let webpack take over handling it
I've also combined the two ENV variable to just the CONSOLE_PLUGIN_NAME and have the old usage of I18N_NAMESPACE in the webpack file be constructed from the CONSOLE_PLUGIN_NAME.
When we create the next konflux update including these changes we will need to remove the script running and just pass in the ENV
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
5688a10 to
411dd57
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@scripts/start-console.sh`:
- Line 9: Update the proxy configuration near BRIDGE_PLUGIN_PROXY so its plugin
path follows the selected CONSOLE_PLUGIN_NAME, keeping BRIDGE_PLUGINS and
BRIDGE_I18N_NAMESPACES consistent; alternatively, explicitly enable this proxy
only for MCP. Apply the correction to all proxy-routing branches currently
hardcoding monitoring-console-plugin.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: c426cbdd-03cc-4f0e-a45a-1513ee0c9186
📒 Files selected for processing (10)
Dockerfile.dev-mcpDockerfile.mcpMakefiledevspace-deployment.yamlhack/install-coo.shhack/scale-down-cmo.shscripts/start-console.shscripts/update-plugin-name.shweb/cypress.config.tsweb/webpack.config.ts
💤 Files with no reviewable changes (3)
- devspace-deployment.yaml
- scripts/update-plugin-name.sh
- Makefile
🚧 Files skipped from review as they are similar to previous changes (6)
- Dockerfile.mcp
- hack/scale-down-cmo.sh
- web/cypress.config.ts
- Dockerfile.dev-mcp
- web/webpack.config.ts
- hack/install-coo.sh
| PLUGIN_PORT=${PLUGIN_PORT:=9443} | ||
| CONSOLE_IMAGE_PLATFORM=${CONSOLE_IMAGE_PLATFORM:="linux/amd64"} | ||
| npm_package_consolePlugin_name=${npm_package_consolePlugin_name:="monitoring-plugin"} | ||
| npm_package_consolePlugin_name=${CONSOLE_PLUGIN_NAME:-"monitoring-plugin"} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep proxy routing consistent with the selected plugin.
When CONSOLE_PLUGIN_NAME=monitoring-plugin, BRIDGE_PLUGINS and BRIDGE_I18N_NAMESPACES use that plugin, but BRIDGE_PLUGIN_PROXY still routes to /api/proxy/plugin/monitoring-console-plugin/perses/ at Lines 38, 45, and 53. Either derive the proxy path from the selected plugin or explicitly gate this proxy to MCP; otherwise the CMO/MP workflow can target the wrong plugin endpoint.
🤖 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 `@scripts/start-console.sh` at line 9, Update the proxy configuration near
BRIDGE_PLUGIN_PROXY so its plugin path follows the selected CONSOLE_PLUGIN_NAME,
keeping BRIDGE_PLUGINS and BRIDGE_I18N_NAMESPACES consistent; alternatively,
explicitly enable this proxy only for MCP. Apply the correction to all
proxy-routing branches currently hardcoding monitoring-console-plugin.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jgbernalp, PeterYurkovich 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 |
|
/retest |
|
@PeterYurkovich: all tests passed! Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
3 participants
This PR looks to update devspace support for the new development and feature setup. It adds a new Makefile command (
make start-mcp-frontend) which injects the correct package name into the webpack build and splits the devspace command to take over either the CMO or COO deployments. If the COO deployment is selected then the command will attempt to install COO if it doesn't already exist and create themonitoring-console-plugindeployment.Summary by CodeRabbit
Summary by CodeRabbit
New Features
start-mcp-frontendworkflow.Documentation
Bug Fixes
Chores