<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
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml) Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (12)
📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (5)
Walkthrough新增 HA 网络组持久化表与字段，扩展 KVM 代理命令和路由常量，添加模拟器端点与内存命令队列，更新测试帮助器以跟踪 API 路径，并新增一组 HA 错误码常量。 ChangesHA网络组支持
Sequence Diagram(s)sequenceDiagram
participant Agent as KVM Agent
participant Simulator as KVMSimulatorController
participant Config as KVMSimulatorConfig
Agent->>Simulator: POST /setup-ha-enabled-metadata
Simulator->>Config: append SetupVmHaEnabledMetadataLiveCmd (synchronized)
Simulator->>Agent: AgentResponse
Agent->>Simulator: POST /reconcile-ha-enabled-metadata
Simulator->>Config: append ReconcileVmHaEnabledMetadataLiveCmd (synchronized)
Simulator->>Agent: AgentResponse
Agent->>Simulator: POST /ha-network-group/sync
Simulator->>Config: append sync command string (synchronized)
Simulator->>Agent: AgentResponse
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 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.javaComment |
![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.
MatheMatrix
force-pushed
the
sync/haoyu.ding/fv-81413@@3
branch
from
db04ee4 to
e09a677
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 `@testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy`:
- Around line 31804-31805: The queryHaNetworkGroup code calls
a.conditions.collect { it.toString() } unguarded and will NPE if a.conditions is
null; update the logic in queryHaNetworkGroup to null-safe the conversion by
first checking if a.conditions is null (or empty) and treating it as an empty
list before calling collect (e.g., replace the direct collect with a
null-coalescing or conditional that yields an empty list when a.conditions is
null), ensuring a.conditions is always a list of strings afterwards.
🪄 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: 5d141ce1-f4c2-4cf3-934c-c36e0943e533
⛔ 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
✅ Files skipped from review due to trivial changes (3)
- testlib/src/main/java/org/zstack/testlib/EnvSpec.groovy
- plugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.java
- utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
🚧 Files skipped from review as they are similar to previous changes (2)
- testlib/src/main/java/org/zstack/testlib/KVMSimulator.groovy
- simulator/simulatorImpl/src/main/java/org/zstack/simulator/kvm/KVMSimulatorController.java
Comment on lines
+31804
to
+31805
| a.conditions = a.conditions.collect { it.toString() } | ||
|
|
There was a problem hiding this comment.
queryHaNetworkGroup 在无查询条件时可能抛出 NPE
Line 31804 直接对 a.conditions 调用 collect，当调用方未设置 conditions 时会在运行时失败，导致查询接口不可用。
建议修复
- a.conditions = a.conditions.collect { it.toString() }
+ a.conditions = a.conditions?.collect { it?.toString() } ?: []📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| a.conditions = a.conditions.collect { it.toString() } | |
| a.conditions = a.conditions?.collect { it?.toString() } ?: [] |
🤖 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 31804
- 31805, The queryHaNetworkGroup code calls a.conditions.collect { it.toString()
} unguarded and will NPE if a.conditions is null; update the logic in
queryHaNetworkGroup to null-safe the conversion by first checking if
a.conditions is null (or empty) and treating it as an empty list before calling
collect (e.g., replace the direct collect with a null-coalescing or conditional
that yields an empty list when a.conditions is null), ensuring a.conditions is
always a list of strings afterwards.
Collaborator
|
Comment from yaohua.wu: Review: MR !9779 — ZSTAC-81413（Network Group HA — zstack 基础层）
关联 MR
🔴 Critical1.
|
MatheMatrix
force-pushed
the
sync/haoyu.ding/fv-81413@@3
branch
from
e09a677 to
3734fad
Compare
DBImpact Resolves: ZSTAC-81413 Change-Id: I6a7574656467636e686377756f716d67707a7063
MatheMatrix
force-pushed
the
sync/haoyu.ding/fv-81413@@3
branch
from
3734fad to
96fe476
Compare
Collaborator
|
Comment from gitlab: 自上次添加REVIEWED标签(2026-05-09 13:48:10.000Z)后, 有新的COMMIT更新(2026-05-09 16:00:06.408Z), 所以移除了REVIEWED标签 |
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
3 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