<feature>[utils]: Network group high availability strategy#3402
<feature>[utils]: Network group high availability strategy#3402zstack-robot-2 wants to merge 1 commit intofeature-5.5.6-dpu-net-ha2from
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough新增 HA 网络组相关数据库表、KVM 代理命令与路径常量、模拟器端点与配置字段；在 StartVmCmd 中加入 enableHa；扩展测试库大量监控/告警 API 辅助方法，并追加大量运维错误码常量。 Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,220,255,0.5)
participant User
end
rect rgba(200,255,200,0.5)
participant Mgmt as ManagementNode
end
rect rgba(255,220,200,0.5)
participant DB as Database
end
rect rgba(255,200,220,0.5)
participant KVM as KVMAgent
end
User->>Mgmt: 调用 管理 API（如 创建/查询 HaNetworkGroup 或 StartVm 包含 enableHa）
Mgmt->>DB: 写入/读取 HaNetworkGroup 与关联表
Mgmt->>KVM: POST /host/vm/updateEnableHa/live (UpdateVmEnableHaLiveCmd)
KVM-->>Mgmt: 返回 AgentResponse
Mgmt->>KVM: POST /host/vm/reconcileEnableHa/live (ReconcileVmEnableHaOnHostCmd)
KVM-->>Mgmt: 返回 AgentResponse
Mgmt->>User: 返回 API 响应
Estimated code review effort🎯 4 (复杂) | ⏱️ ~45 分钟 Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java`:
- Around line 2273-2275: The `@GrayVersion` on StartVmCmd.enableHa is set too low
(5.0.0) and risks being sent to older agents; update the annotation to the
actual version this field was introduced in for this branch (use the
branch/release version constant or the correct semantic version), and apply the
same correction to the other occurrences referenced (the similar field block
around the 2752-2758 range). Locate the StartVmCmd.enableHa field and any
duplicate fields annotated with `@GrayVersion` and replace the version string with
the correct introduced-version value so grayscaling won't dispatch the new field
to incompatible older agents.
ℹ️ Review info
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (12)
sdk/src/main/java/SourceClassMap.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/ChangeHaNetworkGroupStateAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/ChangeHaNetworkGroupStateResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/CreateHaNetworkGroupAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/CreateHaNetworkGroupResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DeleteHaNetworkGroupAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DeleteHaNetworkGroupResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/HaNetworkGroupInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/QueryHaNetworkGroupAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/QueryHaNetworkGroupResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/UpdateHaNetworkGroupAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/UpdateHaNetworkGroupResult.javais excluded by!sdk/**
📒 Files selected for processing (5)
conf/db/upgrade/V5.5.12__schema.sqlplugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.javatestlib/src/main/java/org/zstack/testlib/ApiHelper.groovyutils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
MatheMatrix
force-pushed
the
sync/haoyu.ding/fv-81413@@3
branch
2 times, most recently
from
4cb34c8 to
98aaeef
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java`:
- Around line 2272-2273: Add `@GrayVersion` to the two new command classes
(UpdateVmEnableHaLiveCmd and ReconcileVmEnableHaOnHostCmd) to match project
convention, change StartVmCmd.enableHa's `@GrayVersion` from "5.5.12" to the
correct initial rollout version (use "5.5.0" to align with project current
version), and make null handling consistent by updating
UpdateVmEnableHaLiveCmd.setEnableHa(...) to mirror StartVmCmd.setEnableHa(...)
(convert null to false via enableHa == null ? false : enableHa) so both setter
behaviors are identical.
ℹ️ Review info
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (12)
sdk/src/main/java/SourceClassMap.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/ChangeHaNetworkGroupStateAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/ChangeHaNetworkGroupStateResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/CreateHaNetworkGroupAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/CreateHaNetworkGroupResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DeleteHaNetworkGroupAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DeleteHaNetworkGroupResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/HaNetworkGroupInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/QueryHaNetworkGroupAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/QueryHaNetworkGroupResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/UpdateHaNetworkGroupAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/UpdateHaNetworkGroupResult.javais excluded by!sdk/**
📒 Files selected for processing (7)
conf/db/upgrade/V5.5.12__schema.sqlplugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.javasimulator/simulatorImpl/src/main/java/org/zstack/simulator/kvm/KVMSimulatorConfig.javasimulator/simulatorImpl/src/main/java/org/zstack/simulator/kvm/KVMSimulatorController.javatestlib/src/main/java/org/zstack/testlib/ApiHelper.groovyutils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
🚧 Files skipped from review as they are similar to previous changes (1)
- conf/db/upgrade/V5.5.12__schema.sql
| @GrayVersion(value = "5.5.12") | ||
| private Boolean enableHa = false; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== StartVmCmd enableHa block =="
rg -n "enableHa|@GrayVersion" plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java -C2
echo
echo "== 该文件中的 5.5.x GrayVersion 分布 =="
rg -n "@GrayVersion\(value = \"5\.5\.[0-9]+\"\)" plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java
echo
echo "== 仓库内 POM 版本线（抽样前10个）== "
fd pom.xml -x head -20 {} \; 2>/dev/null | grep -E "version|revision" | head -30 || trueRepository: MatheMatrix/zstack
Length of output: 48882
🏁 Script executed:
#!/bin/bash
# 检查目标分支信息和当前版本配置
echo "== 检查git分支信息 =="
git branch -a 2>/dev/null | grep -E "5\.[0-9]+\.[0-9]+" || echo "未找到版本相关分支"
echo
echo "== 检查启用HA相关的版本标注 =="
rg -n "enableHa|@GrayVersion" plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java -B1 -A1 | head -40Repository: MatheMatrix/zstack
Length of output: 1696
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== UpdateVmEnableHaLiveCmd 完整定义 =="
rg -n "class UpdateVmEnableHaLiveCmd" plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java -A15
echo
echo "== ReconcileVmEnableHaOnHostCmd 完整定义 =="
rg -n "class ReconcileVmEnableHaOnHostCmd" plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java -A15
echo
echo "== 检查这些新增命令类是否被关联的消息类引用 =="
rg -r "UpdateVmEnableHaLiveCmd|ReconcileVmEnableHaOnHostCmd" . --type java | head -20
echo
echo "== 查看 StartVmCmd 的 getter/setter 完整实现 =="
rg -n "setEnableHa|getEnableHa" plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java -B1 -A3Repository: MatheMatrix/zstack
Length of output: 30493
新增命令类缺少版本标注，且 enableHa 处理逻辑不一致。
-
UpdateVmEnableHaLiveCmd和ReconcileVmEnableHaOnHostCmd均缺少@GrayVersion注解。新增的命令类应遵循项目规范，明确标注其首次可用版本。 -
StartVmCmd.enableHa标注为@GrayVersion("5.5.12")，但项目当前版本为 5.5.0，且同文件其他 5.5.x 特性标注为 5.5.0 或 5.5.6。版本号应与实际功能首次可用版本对齐，避免灰度阶段低版本 agent 接收不到该字段。 -
UpdateVmEnableHaLiveCmd.setEnableHa()无 null 处理，而StartVmCmd.setEnableHa()做了enableHa == null ? false : enableHa的转换。两者应保持一致的 null 处理策略，以确保灰度下的行为预期一致。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java` around lines
2272 - 2273, Add `@GrayVersion` to the two new command classes
(UpdateVmEnableHaLiveCmd and ReconcileVmEnableHaOnHostCmd) to match project
convention, change StartVmCmd.enableHa's `@GrayVersion` from "5.5.12" to the
correct initial rollout version (use "5.5.0" to align with project current
version), and make null handling consistent by updating
UpdateVmEnableHaLiveCmd.setEnableHa(...) to mirror StartVmCmd.setEnableHa(...)
(convert null to false via enableHa == null ? false : enableHa) so both setter
behaviors are identical.
05389f8 to
92ed618
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java (1)
1019-1041:⚠️ Potential issue | 🟠 Major
@GrayVersion版本号高于当前分支版本线，存在灰度下字段被过滤风险。
UpdateVmEnableHaLiveCmd.enableHa、ReconcileVmEnableHaOnHostCmd.neverStopVmUuids、StartVmCmd.enableHa当前都标注为5.5.12。在feature-5.5.6-dpu-net-ha2版本线下，这会导致新字段在混部/灰度时可能不下发到对应 agent，从而使 HA 开关与对账参数失效。🔧 建议修复
- `@GrayVersion`(value = "5.5.12") + `@GrayVersion`(value = "5.5.6") private Boolean enableHa = false; ... - `@GrayVersion`(value = "5.5.12") + `@GrayVersion`(value = "5.5.6") private List<String> neverStopVmUuids; ... - `@GrayVersion`(value = "5.5.12") + `@GrayVersion`(value = "5.5.6") private Boolean enableHa = false;As per coding guidelines "向后兼容原则：之前的代码产生的行为不要直接去改动... 实在需要改动已有行为，应当对原状态做好记录，做好二次确认，做好回退准备，这个非常重要"。
Also applies to: 2274-2275
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java` around lines 1019 - 1041, The `@GrayVersion` on new fields is higher than the branch line causing fields like UpdateVmEnableHaLiveCmd.enableHa, ReconcileVmEnableHaOnHostCmd.neverStopVmUuids and StartVmCmd.enableHa to be filtered out during mixed-version/gray deployments; fix by aligning the annotation to the branch-compatible version (or removing the `@GrayVersion`) so the fields are present for this branch (e.g. change "5.5.12" to the current branch baseline such as "5.5.6" or drop the annotation) and ensure setters/getters remain unchanged to preserve backward compatibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@simulator/simulatorImpl/src/main/java/org/zstack/simulator/kvm/KVMSimulatorController.java`:
- Around line 708-724: The handlers updateVmEnableHaLive and
reconcileVmEnableHaOnHost currently call
config.updateVmEnableHaLiveCmds.add(cmd) and
config.reconcileVmEnableHaOnHostCmds.add(cmd) on plain ArrayLists without
synchronization; make these writes thread-safe by either replacing those lists
with a thread-safe collection (e.g., Collections.synchronizedList or
CopyOnWriteArrayList) or synchronizing around the add (e.g.,
synchronized(config.updateVmEnableHaLiveCmds){...}), and apply the same
protection for config.reconcileVmEnableHaOnHostCmds so concurrent HTTP requests
cannot cause races.
---
Duplicate comments:
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java`:
- Around line 1019-1041: The `@GrayVersion` on new fields is higher than the
branch line causing fields like UpdateVmEnableHaLiveCmd.enableHa,
ReconcileVmEnableHaOnHostCmd.neverStopVmUuids and StartVmCmd.enableHa to be
filtered out during mixed-version/gray deployments; fix by aligning the
annotation to the branch-compatible version (or removing the `@GrayVersion`) so
the fields are present for this branch (e.g. change "5.5.12" to the current
branch baseline such as "5.5.6" or drop the annotation) and ensure
setters/getters remain unchanged to preserve backward compatibility.
ℹ️ Review info
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (12)
sdk/src/main/java/SourceClassMap.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/ChangeHaNetworkGroupStateAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/ChangeHaNetworkGroupStateResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/CreateHaNetworkGroupAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/CreateHaNetworkGroupResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DeleteHaNetworkGroupAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DeleteHaNetworkGroupResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/HaNetworkGroupInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/QueryHaNetworkGroupAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/QueryHaNetworkGroupResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/UpdateHaNetworkGroupAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/UpdateHaNetworkGroupResult.javais excluded by!sdk/**
📒 Files selected for processing (7)
conf/db/upgrade/V5.5.12__schema.sqlplugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.javasimulator/simulatorImpl/src/main/java/org/zstack/simulator/kvm/KVMSimulatorConfig.javasimulator/simulatorImpl/src/main/java/org/zstack/simulator/kvm/KVMSimulatorController.javatestlib/src/main/java/org/zstack/testlib/ApiHelper.groovyutils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
🚧 Files skipped from review as they are similar to previous changes (1)
- conf/db/upgrade/V5.5.12__schema.sql
simulator/simulatorImpl/src/main/java/org/zstack/simulator/kvm/KVMSimulatorController.java
Outdated
Show resolved
Hide resolved
MatheMatrix
force-pushed
the
sync/haoyu.ding/fv-81413@@3
branch
4 times, most recently
from
d5f2e45 to
aa3fc92
Compare
|
Comment from yaohua.wu: Review: MR !9250 — ZSTAC-81413Feature: Network group high availability strategy (DPU 多业务网络 HA 精细化控制) 🔴 Critical1. 本 MR 目标分支为
同文件中同期引入的 5.5.6 版本特性使用的是 建议修复：将上述三处 🟡 Warning2.
建议：为 3.
-- 当前：仅有两个单列索引
INDEX `idxHostHaNetworkGroupStatusVOhostUuid` (`hostUuid`),
INDEX `idxHostHaNetworkGroupStatusVOnetworkGroupUuid` (`networkGroupUuid`),从语义上看，一个 host 对一个 networkGroup 应该只有一条状态记录。如果缺少联合唯一约束，并发场景下可能产生重复记录，导致查询结果不确定。 建议：添加 4. // 新增（无 volatile）：
public List<UpdateVmEnableHaLiveCmd> updateVmEnableHaLiveCmds = new ArrayList<>();
public List<ReconcileVmEnableHaOnHostCmd> reconcileVmEnableHaOnHostCmds = new ArrayList<>();
public List<String> syncHaNetworkGroupConfigCmds = new ArrayList<>();
// 现有同类字段（有 volatile）：
public volatile List<StopVmCmd> stopVmCmds = new ArrayList<>();
public volatile List<RebootVmCmd> rebootVmCmds = new ArrayList<>();KVMSimulatorController 中的写操作已经用 🟢 Suggestion5. 该命令只有 6.
7. 现有的大多数 simulator handler 都有对应的 Verdict: REVISION_REQUIRED主要阻塞点：Critical #1（ 🤖 Robot Reviewer |
MatheMatrix
force-pushed
the
sync/haoyu.ding/fv-81413@@3
branch
from
aa3fc92 to
acafa83
Compare
DBImpact Resolves: ZSTAC-81413 Change-Id: I6a7574656467636e686377756f716d67707a7063
MatheMatrix
force-pushed
the
sync/haoyu.ding/fv-81413@@3
branch
from
acafa83 to
57f9210
Compare
3 participants
DBImpact
Resolves: ZSTAC-81413
Change-Id: I6a7574656467636e686377756f716d67707a7063
sync from gitlab !9250