<feature>[utils]: Network group high availability strategy#3897
Open
ZStack-Robot wants to merge 1 commit into5.5.22from
Open
<feature>[utils]: Network group high availability strategy#3897ZStack-Robot wants to merge 1 commit into5.5.22from
ZStack-Robot wants to merge 1 commit into5.5.22from
Conversation
DBImpact Resolves: ZSTAC-81413 Change-Id: I6a7574656467636e686377756f716d67707a7063
概览该PR为ZStack平台的HA网络组功能添加基础支持。变更包括四个新数据库表、两个KVM代理命令、三个模拟器REST端点、22个HA错误代码常量以及五个API测试辅助方法的增强。 变更HA网络组支持
序列图sequenceDiagram
participant Agent as KVM Agent
participant Simulator as Simulator Controller
participant Config as Simulator Config
Agent->>Simulator: setupHaEnabledMetadata/live
Simulator->>Config: append SetupVmHaEnabledMetadataLiveCmd
Config->>Config: synchronized list update
Simulator->>Agent: AgentResponse
Agent->>Simulator: reconcileHaEnabledMetadata/live
Simulator->>Config: append ReconcileVmHaEnabledMetadataLiveCmd
Simulator->>Agent: AgentResponse
Agent->>Simulator: ha/networkgroup/sync
Simulator->>Config: append sync command
Simulator->>Agent: AgentResponse
🎯 3 (中等) | ⏱️ ~20 分钟
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.42.1)utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java🔧 Microsoft Presidio Analyzer (2.2.362)testlib/src/main/java/org/zstack/testlib/ApiHelper.groovyMicrosoft Presidio Analyzer failed to scan this file 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
🧹 Nitpick comments (1)
testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy (1)
5663-5687: 🏗️ Heavy lift抽取通用
apipath调用模板，避免 5 处复制逻辑后续漂移当前
apiId补齐、ApiPathTracker创建、a.call()、Test.apiPaths写入完全重复；后续修改容易漏改一处。建议提取成统一 helper。♻️ 可参考的重构方向
+ private def callWithApiPathTracking(def a) { + if (System.getProperty("apipath") != null) { + if (a.apiId == null) { + a.apiId = Platform.uuid + } + def tracker = new ApiPathTracker(a.apiId) + def out = errorOut(a.call()) + def path = tracker.getApiPath() + if (!path.isEmpty()) { + Test.apiPaths[a.class.name] = path.join(" --->\n") + } + return out + } + return errorOut(a.call()) + }然后每个方法收敛为：
- if (System.getProperty("apipath") != null) { - ... - } else { - return errorOut(a.call()) - } + return callWithApiPathTracking(a)Also applies to: 9686-9710, 14870-14894, 31780-31795, 45201-45216
🤖 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 `@testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy` around lines 5663 - 5687, Extract the repeated apipath handling into a single helper (e.g., a private method like handleApiCall(Action a) or wrapApiCall) and replace the duplicated blocks in methods such as changeHaNetworkGroupState by calling that helper; the helper should: ensure a.apiId is set (Platform.uuid when null), instantiate ApiPathTracker with a.apiId, invoke a.call() through errorOut, record the path into Test.apiPaths keyed by a.class.name when non-empty, and return the errorOut result—replace the duplicated logic in other locations (the other listed ranges) to call this new helper and remove the inline duplication.
🤖 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 `@conf/db/upgrade/V5.5.22__schema.sql`:
- Around line 8-9: Replace the invalid zero-datetime defaults with
CURRENT_TIMESTAMP: update the column definitions that use `DEFAULT '0000-00-00
00:00:00'` (e.g., `lastOpDate`, `createDate` and the other two occurrences) to
use `DEFAULT CURRENT_TIMESTAMP` (and keep `ON UPDATE CURRENT_TIMESTAMP` only on
columns that should auto-update, e.g., `lastOpDate`), ensuring the NOT NULL
semantics remain intact so the DDL will succeed in strict SQL modes.
In
`@utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java`:
- Around line 1875-1918: The new constants ORG_ZSTACK_HA_10019 through
ORG_ZSTACK_HA_10040 in CloudOperationsErrorCode are declared but lack message
definitions and are not referenced; either add message entries for each code in
the errorCode resource files and wire them into the ErrorCode/ResourceLoader
mechanism, then replace placeholder or create usage sites in business logic to
throw or return these codes (search for usages patterns like
ErrorCode/CLOUD_OPERATION_* or methods that create ErrorCode instances) or
remove the unused constants; if they are intended as reserved, add a clear
Javadoc comment on the constants explaining their planned purpose and why they
are unused.
---
Nitpick comments:
In `@testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy`:
- Around line 5663-5687: Extract the repeated apipath handling into a single
helper (e.g., a private method like handleApiCall(Action a) or wrapApiCall) and
replace the duplicated blocks in methods such as changeHaNetworkGroupState by
calling that helper; the helper should: ensure a.apiId is set (Platform.uuid
when null), instantiate ApiPathTracker with a.apiId, invoke a.call() through
errorOut, record the path into Test.apiPaths keyed by a.class.name when
non-empty, and return the errorOut result—replace the duplicated logic in other
locations (the other listed ranges) to call this new helper and remove the
inline duplication.
🪄 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: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
Run ID: 1451bf12-1c77-4c28-b4dc-25391e49b304
⛔ 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 (9)
conf/db/upgrade/V5.5.22__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.groovytestlib/src/main/java/org/zstack/testlib/EnvSpec.groovytestlib/src/main/java/org/zstack/testlib/KVMSimulator.groovyutils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
Comment on lines
+8
to
+9
| `lastOpDate` TIMESTAMP NOT NULL DEFAULT '0000-00-00 00:00:00' ON UPDATE CURRENT_TIMESTAMP, | ||
| `createDate` TIMESTAMP NOT NULL DEFAULT '0000-00-00 00:00:00', |
There was a problem hiding this comment.
修复非法零时间默认值，避免升级脚本在严格模式下失败
Line 8、Line 9、Line 17、Line 18、Line 31、Line 32 使用了 DEFAULT '0000-00-00 00:00:00'，这在 MySQL 8/GreatSQL 常见严格 SQL 模式下会导致 DDL 执行失败，属于升级阻断问题。
🛠️ 建议修复 diff
CREATE TABLE IF NOT EXISTS `zstack`.`HaNetworkGroupVO` (
@@
- `lastOpDate` TIMESTAMP NOT NULL DEFAULT '0000-00-00 00:00:00' ON UPDATE CURRENT_TIMESTAMP,
- `createDate` TIMESTAMP NOT NULL DEFAULT '0000-00-00 00:00:00',
+ `lastOpDate` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
+ `createDate` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
@@
CREATE TABLE IF NOT EXISTS `zstack`.`HaNetworkGroupL3NetworkRefVO` (
@@
- `lastOpDate` TIMESTAMP NOT NULL DEFAULT '0000-00-00 00:00:00' ON UPDATE CURRENT_TIMESTAMP,
- `createDate` TIMESTAMP NOT NULL DEFAULT '0000-00-00 00:00:00',
+ `lastOpDate` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
+ `createDate` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
@@
CREATE TABLE IF NOT EXISTS `zstack`.`HostHaNetworkGroupStatusVO` (
@@
- `lastOpDate` TIMESTAMP NOT NULL DEFAULT '0000-00-00 00:00:00' ON UPDATE CURRENT_TIMESTAMP,
- `createDate` TIMESTAMP NOT NULL DEFAULT '0000-00-00 00:00:00',
+ `lastOpDate` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
+ `createDate` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,As per coding guidelines **/*.sql: Do not use DEFAULT 0000-00-00 00:00:00, use DEFAULT CURRENT_TIMESTAMP instead.
Also applies to: 17-18, 31-32
🤖 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 `@conf/db/upgrade/V5.5.22__schema.sql` around lines 8 - 9, Replace the invalid
zero-datetime defaults with CURRENT_TIMESTAMP: update the column definitions
that use `DEFAULT '0000-00-00 00:00:00'` (e.g., `lastOpDate`, `createDate` and
the other two occurrences) to use `DEFAULT CURRENT_TIMESTAMP` (and keep `ON
UPDATE CURRENT_TIMESTAMP` only on columns that should auto-update, e.g.,
`lastOpDate`), ensuring the NOT NULL semantics remain intact so the DDL will
succeed in strict SQL modes.
Comment on lines
+1875
to
+1918
| public static final String ORG_ZSTACK_HA_10019 = "ORG_ZSTACK_HA_10019"; | ||
|
|
||
| public static final String ORG_ZSTACK_HA_10020 = "ORG_ZSTACK_HA_10020"; | ||
|
|
||
| public static final String ORG_ZSTACK_HA_10021 = "ORG_ZSTACK_HA_10021"; | ||
|
|
||
| public static final String ORG_ZSTACK_HA_10022 = "ORG_ZSTACK_HA_10022"; | ||
|
|
||
| public static final String ORG_ZSTACK_HA_10023 = "ORG_ZSTACK_HA_10023"; | ||
|
|
||
| public static final String ORG_ZSTACK_HA_10024 = "ORG_ZSTACK_HA_10024"; | ||
|
|
||
| public static final String ORG_ZSTACK_HA_10025 = "ORG_ZSTACK_HA_10025"; | ||
|
|
||
| public static final String ORG_ZSTACK_HA_10026 = "ORG_ZSTACK_HA_10026"; | ||
|
|
||
| public static final String ORG_ZSTACK_HA_10027 = "ORG_ZSTACK_HA_10027"; | ||
|
|
||
| public static final String ORG_ZSTACK_HA_10028 = "ORG_ZSTACK_HA_10028"; | ||
|
|
||
| public static final String ORG_ZSTACK_HA_10029 = "ORG_ZSTACK_HA_10029"; | ||
|
|
||
| public static final String ORG_ZSTACK_HA_10030 = "ORG_ZSTACK_HA_10030"; | ||
|
|
||
| public static final String ORG_ZSTACK_HA_10031 = "ORG_ZSTACK_HA_10031"; | ||
|
|
||
| public static final String ORG_ZSTACK_HA_10032 = "ORG_ZSTACK_HA_10032"; | ||
|
|
||
| public static final String ORG_ZSTACK_HA_10033 = "ORG_ZSTACK_HA_10033"; | ||
|
|
||
| public static final String ORG_ZSTACK_HA_10034 = "ORG_ZSTACK_HA_10034"; | ||
|
|
||
| public static final String ORG_ZSTACK_HA_10035 = "ORG_ZSTACK_HA_10035"; | ||
|
|
||
| public static final String ORG_ZSTACK_HA_10036 = "ORG_ZSTACK_HA_10036"; | ||
|
|
||
| public static final String ORG_ZSTACK_HA_10037 = "ORG_ZSTACK_HA_10037"; | ||
|
|
||
| public static final String ORG_ZSTACK_HA_10038 = "ORG_ZSTACK_HA_10038"; | ||
|
|
||
| public static final String ORG_ZSTACK_HA_10039 = "ORG_ZSTACK_HA_10039"; | ||
|
|
||
| public static final String ORG_ZSTACK_HA_10040 = "ORG_ZSTACK_HA_10040"; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1. Check which of the new constants have message definitions in error-code resource files
echo "=== Searching for message definitions ==="
for code in $(seq 10019 10040); do
key="ORG_ZSTACK_HA_${code}"
count=$(rg -l "$key" --glob '*.properties' --glob '*.json' --glob '*.yaml' --glob '*.yml' 2>/dev/null | wc -l)
echo "$key: $count file(s) with definition"
done
# 2. Check which of the new constants are referenced in Java/Groovy source code
echo ""
echo "=== Searching for usages in source code ==="
for code in $(seq 10019 10040); do
key="ORG_ZSTACK_HA_${code}"
count=$(rg -l "$key" --glob '*.java' --glob '*.groovy' 2>/dev/null | wc -l)
echo "$key: $count file(s) referencing this constant"
doneRepository: MatheMatrix/zstack
Length of output: 252
🏁 Script executed:
#!/bin/bash
# Find error code message definitions and usages for ORG_ZSTACK_HA_10019-10040
echo "=== Searching for message definitions in properties/JSON/YAML files ==="
python3 << 'PYTHON_EOF'
import subprocess
import os
codes_without_defs = []
codes_with_defs = []
for code in range(10019, 10041): # 10019 to 10040 inclusive
key = f"ORG_ZSTACK_HA_{code}"
result = subprocess.run(['rg', key, '-g', '*.properties', '-g', '*.json', '-g', '*.yaml', '-g', '*.yml'],
capture_output=True, text=True)
matches = result.stdout.strip()
if matches:
codes_with_defs.append((key, len(matches.split('\n'))))
else:
codes_without_defs.append(key)
if codes_with_defs:
print(f"Codes WITH message definitions ({len(codes_with_defs)}):")
for key, count in codes_with_defs:
print(f" {key}")
else:
print("No message definitions found for any of the new codes")
if codes_without_defs:
print(f"\nCodes WITHOUT message definitions ({len(codes_without_defs)}):")
for key in codes_without_defs:
print(f" {key}")
PYTHON_EOF
echo ""
echo "=== Searching for usages in Java/Groovy source code ==="
python3 << 'PYTHON_EOF'
import subprocess
codes_with_usage = []
codes_without_usage = []
for code in range(10019, 10041): # 10019 to 10040 inclusive
key = f"ORG_ZSTACK_HA_{code}"
result = subprocess.run(['rg', key, '-g', '*.java', '-g', '*.groovy'],
capture_output=True, text=True)
matches = result.stdout.strip()
if matches:
codes_with_usage.append((key, len(matches.split('\n'))))
else:
codes_without_usage.append(key)
if codes_with_usage:
print(f"Codes WITH usages in source ({len(codes_with_usage)}):")
for key, count in codes_with_usage[:10]: # Show first 10
print(f" {key}: {count} match(es)")
if len(codes_with_usage) > 10:
print(f" ... and {len(codes_with_usage) - 10} more")
else:
print("No usages found for any of the new codes in source files")
if codes_without_usage:
print(f"\nCodes NOT used in source ({len(codes_without_usage)}):")
if len(codes_without_usage) <= 22:
for key in codes_without_usage:
print(f" {key}")
PYTHON_EOF
# Also check the structure of error code message files
echo ""
echo "=== Finding error code message resource files ==="
find . -type f \( -name "*errorCode*" -o -name "*error*code*" \) \( -name "*.properties" -o -name "*.json" -o -name "*.yaml" -o -name "*.yml" \) | head -20Repository: MatheMatrix/zstack
Length of output: 1394
新增的 22 个错误码缺少消息定义且未被使用
验证结果表明：
- 所有 22 个新错误码（ORG_ZSTACK_HA_10019 ~ ORG_ZSTACK_HA_10040）均未在任何资源文件中定义消息
- 所有 22 个新错误码在源代码中均没有被引用
这是一个关键问题：
- 错误码缺少对应的消息定义，运行时将无法获取到错误描述信息，影响用户体验
- 所有新增的错误码当前都是死代码，需要：
- 补充消息定义（在 errorCode 相关资源文件中），且
- 确保在实际业务代码中被引用使用
建议：如果这些错误码是预留性的，应在代码注释中明确说明其预期用途；否则应补充完整的定义和使用，或移除这些未使用的常量。
🤖 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
`@utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java`
around lines 1875 - 1918, The new constants ORG_ZSTACK_HA_10019 through
ORG_ZSTACK_HA_10040 in CloudOperationsErrorCode are declared but lack message
definitions and are not referenced; either add message entries for each code in
the errorCode resource files and wire them into the ErrorCode/ResourceLoader
mechanism, then replace placeholder or create usage sites in business logic to
throw or return these codes (search for usages patterns like
ErrorCode/CLOUD_OPERATION_* or methods that create ErrorCode instances) or
remove the unused constants; if they are intended as reserved, add a clear
Javadoc comment on the constants explaining their planned purpose and why they
are unused.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DBImpact
Resolves: ZSTAC-81413
Change-Id: I6a7574656467636e686377756f716d67707a7063
sync from gitlab !9779