feat: route and audit cluster runtime operations by selected instance - #1244
feat: route and audit cluster runtime operations by selected instance#1244Aias00 wants to merge 5 commits into
Conversation
RockteMQ-AI
left a comment
There was a problem hiding this comment.
Summary
Adds instance-aware routing to cluster runtime operations — when an instanceId is provided, admin API calls are routed through RuntimeAdminClientResolver instead of the static NameServer config. Falls back cleanly to legacy behavior when instanceId is absent.
Strengths:
- Backward compatible: all new parameters optional, default methods in
ClusterProviderinterface - Clean routing abstraction via
RuntimeAdminClientResolver - Frontend correctly uses ref pattern (
selectedInstanceIdRef) to avoid stale closures in async callbacks - Test mocks properly updated for new constructor dependencies
- Frontend correctly filters to
APACHEvendor instances only
One minor observation:
ClusterService.listClusters(String instanceId)log says "Listing all clusters" without including the instanceId — see inline comment.
Overall a well-structured change. LGTM.
Automated review by github-manager-bot
| return List.of(); | ||
| } | ||
|
|
||
| public List<ClusterVO> listClusters(String instanceId) { |
There was a problem hiding this comment.
[Info] Minor logging improvement: the log message "Listing all clusters" doesn't include the instanceId parameter. Consider adding it for observability, e.g. log.info("Listing clusters for instance: {}", instanceId) when instanceId is present.
|
This PR has conflicts with the base branch and cannot be merged. Please rebase or merge the base branch into your branch and resolve the conflicts: git fetch origin
git checkout feat/studio-instance-scoped-cluster-runtime
git rebase origin/main
# resolve conflicts, then:
git push --force-with-leaseThis is a one-time reminder. Feel free to @mention me for a re-review after conflicts are resolved. Automated notification by github-manager-bot |
5f5794d to
52948b6
Compare
RockteMQ-AI
left a comment
There was a problem hiding this comment.
Summary
This PR adds instance-aware routing to cluster runtime operations, enabling the dashboard to target specific RocketMQ instances when listing clusters, viewing cluster details, and updating broker configurations. The changes span both backend (controller → service → provider) and frontend (instance selector UI).
Changes reviewed:
- Added
instanceIdparameter toClusterControllerendpoints (listClusters,getCluster) as optional request parameter - Extended
ClusterProviderinterface with instance-aware method signatures - Updated
ApacheClusterProviderto resolve cluster discovery and broker config through selected instance endpoint - Added instance selector component to Cluster page in frontend
- Proper backward compatibility: legacy calls without instanceId retain existing behavior
Assessment:
- ✅ Clean layered architecture: controller → service → provider with proper separation of concerns
- ✅ Backward compatible: optional
instanceIdparameter withrequired = false - ✅ Consistent pattern with other instance-aware operations (Consumer Group, Topic)
- ✅ Frontend integration adds instance selector for user-facing cluster operations
- ✅ Good documentation in PR body with verification steps
LGTM — solid multi-instance support for cluster runtime operations.
Automated review by "github-manager-bot"
Closes #1243
Closes #1262
Summary
Validation
JAVA_HOME=$(/usr/libexec/java_home -v 21) PATH="$JAVA_HOME/bin:$PATH" mvn -Dtest=ClusterServiceTest,RocketMQBrokerConfigServiceTest,RocketMQClusterProviderTest,AuditServiceTest testnpm test -- --run src/api/cluster.test.ts src/api/clusterContract.test.ts src/services/clusterService.test.tsnpm run build