<fix>[sdk]: refactor HA network group validation#3770
Closed
zstack-robot-2 wants to merge 1 commit into
Closed
Conversation
Walkthrough本次变更调整了 HA 网络组相关的数据库架构（删除两列、修改索引、添加全局版本表并插入初始记录），新增了四个 HA 相关的错误码常量，并在测试环境清理白名单中加入了新表的 VO 类型。 Changes
Sequence Diagram(s)(无 — 更改为数据库模式、常量和测试白名单，未引入跨组件的新控制流。) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
conf/db/upgrade/V5.5.12__schema.sql (1)
1-24:⚠️ Potential issue | 🟡 Minor将 TIMESTAMP 默认值改为 CURRENT_TIMESTAMP
第 8、17、31 行的
lastOpDate和createDate列使用了DEFAULT '0000-00-00 00:00:00'，违反编码指南规范。应改为DEFAULT CURRENT_TIMESTAMP。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@conf/db/upgrade/V5.5.12__schema.sql` around lines 1 - 24, The TIMESTAMP columns use the invalid zero-date default; update the DEFAULT for lastOpDate and createDate in HaNetworkGroupVO and HaNetworkGroupL3NetworkRefVO: replace "DEFAULT '0000-00-00 00:00:00'" with "DEFAULT CURRENT_TIMESTAMP" and keep the ON UPDATE CURRENT_TIMESTAMP only on lastOpDate (i.e., HaNetworkGroupVO.lastOpDate and HaNetworkGroupL3NetworkRefVO.lastOpDate should have DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, while HaNetworkGroupVO.createDate and HaNetworkGroupL3NetworkRefVO.createDate should have DEFAULT CURRENT_TIMESTAMP without ON UPDATE).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@conf/db/upgrade/V5.5.12__schema.sql`:
- Around line 1-24: The TIMESTAMP columns use the invalid zero-date default;
update the DEFAULT for lastOpDate and createDate in HaNetworkGroupVO and
HaNetworkGroupL3NetworkRefVO: replace "DEFAULT '0000-00-00 00:00:00'" with
"DEFAULT CURRENT_TIMESTAMP" and keep the ON UPDATE CURRENT_TIMESTAMP only on
lastOpDate (i.e., HaNetworkGroupVO.lastOpDate and
HaNetworkGroupL3NetworkRefVO.lastOpDate should have DEFAULT CURRENT_TIMESTAMP ON
UPDATE CURRENT_TIMESTAMP, while HaNetworkGroupVO.createDate and
HaNetworkGroupL3NetworkRefVO.createDate should have DEFAULT CURRENT_TIMESTAMP
without ON UPDATE).
ℹ️ 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: 74be4c8c-3ce6-4b3b-a34d-a46971c83f27
⛔ Files ignored due to path filters (2)
sdk/src/main/java/org/zstack/sdk/CreateHaNetworkGroupAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/HaNetworkGroupInventory.javais excluded by!sdk/**
📒 Files selected for processing (2)
conf/db/upgrade/V5.5.12__schema.sqlutils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
MatheMatrix
force-pushed
the
sync/haoyu.ding/fix-84164@@2
branch
from
af329bc to
7e7f9c0
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
conf/db/upgrade/V5.5.12__schema.sql (1)
21-48:⚠️ Potential issue | 🟠 Major将结构变更放在历史版本脚本中，可能导致已升级环境缺失本次变更。
这里的唯一索引调整和新表创建都写在
V5.5.12__schema.sql。对已经执行过该版本升级的数据库，这些变更通常不会再次生效，可能造成：
l3NetworkUuid唯一约束未落地；HaNetworkGroupGlobalConfigVersionVO与初始化记录缺失。建议将这些变更放到新的更高版本升级脚本，并在添加唯一约束前补充重复数据清理与幂等保护。
As per coding guidelines
**/*.sql: “Upgrading scene has been carefully handled”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@conf/db/upgrade/V5.5.12__schema.sql` around lines 21 - 48, The schema changes (unique index `ukHaNetworkGroupL3NetworkRefVOl3NetworkUuid`, tables `HostHaNetworkGroupStatusVO`, `HaNetworkGroupGlobalConfigVersionVO` and the INSERT for `'ha-network-group'`) must not live in the historical file `V5.5.12__schema.sql`; move them into a new higher-version migration script (e.g., V5.5.13+), make the unique-index addition idempotent by first deduplicating rows in the `HaNetworkGroupL3NetworkRefVO` source table and/or skipping index creation if it already exists, and ensure the `HaNetworkGroupGlobalConfigVersionVO` initialization uses an idempotent INSERT/UPDATE path so re-running the migration is safe; reference the constraints `fkHaNetworkGroupL3NetworkRefVOHaNetworkGroupVO` / `fkHaNetworkGroupL3NetworkRefVOL3NetworkEO` when verifying referential integrity during the migration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@conf/db/upgrade/V5.5.12__schema.sql`:
- Around line 21-48: The schema changes (unique index
`ukHaNetworkGroupL3NetworkRefVOl3NetworkUuid`, tables
`HostHaNetworkGroupStatusVO`, `HaNetworkGroupGlobalConfigVersionVO` and the
INSERT for `'ha-network-group'`) must not live in the historical file
`V5.5.12__schema.sql`; move them into a new higher-version migration script
(e.g., V5.5.13+), make the unique-index addition idempotent by first
deduplicating rows in the `HaNetworkGroupL3NetworkRefVO` source table and/or
skipping index creation if it already exists, and ensure the
`HaNetworkGroupGlobalConfigVersionVO` initialization uses an idempotent
INSERT/UPDATE path so re-running the migration is safe; reference the
constraints `fkHaNetworkGroupL3NetworkRefVOHaNetworkGroupVO` /
`fkHaNetworkGroupL3NetworkRefVOL3NetworkEO` when verifying referential integrity
during the migration.
ℹ️ 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: 8253e10d-fa79-4194-b648-4842872a0141
⛔ Files ignored due to path filters (2)
sdk/src/main/java/org/zstack/sdk/CreateHaNetworkGroupAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/HaNetworkGroupInventory.javais excluded by!sdk/**
📒 Files selected for processing (2)
conf/db/upgrade/V5.5.12__schema.sqlutils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
✅ Files skipped from review due to trivial changes (1)
- utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
Collaborator
Author
|
Comment from ye.zou: Code Review
结论: APPROVE ✅（条件性） |
MatheMatrix
force-pushed
the
sync/haoyu.ding/fix-84164@@2
branch
from
7e7f9c0 to
dac798d
Compare
DBImpact Resolves: ZSTAC-84164 Change-Id: I736967737a6963726b666776746b677a69717278
MatheMatrix
force-pushed
the
sync/haoyu.ding/fix-84164@@2
branch
from
dac798d to
d1459fb
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 `@testlib/src/main/java/org/zstack/testlib/EnvSpec.groovy`:
- Around line 710-711: 不要把整个 HaNetworkGroupGlobalConfigVersionVO 整表跳过；在
EnvSpec.groovy 中，移除 "HaNetworkGroupGlobalConfigVersionVO" 从忽略名单（当前列在那段数组里），改为在
AllowedDBRemaining 里添加一条只放行 name = 'ha-network-group' 的记录，从而保留
makeSureAllEntitiesDeleted() 对该 VO
其它意外记录的检查；参考相关符号：AllowedDBRemaining、makeSureAllEntitiesDeleted 和
HaNetworkGroupGlobalConfigVersionVO 以定位并修改逻辑。
🪄 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: cc88bb68-51e3-4064-942e-4a76e786533d
⛔ Files ignored due to path filters (2)
sdk/src/main/java/org/zstack/sdk/CreateHaNetworkGroupAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/HaNetworkGroupInventory.javais excluded by!sdk/**
📒 Files selected for processing (3)
conf/db/upgrade/V5.5.12__schema.sqltestlib/src/main/java/org/zstack/testlib/EnvSpec.groovyutils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
🚧 Files skipped from review as they are similar to previous changes (1)
- utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
Comment on lines
+710
to
+711
| "HostNetworkLabelVO", "L3NetworkSequenceNumberVO", | ||
| "HaNetworkGroupGlobalConfigVersionVO"]) { |
There was a problem hiding this comment.
不要把整个 HaNetworkGroupGlobalConfigVersionVO 直接加入忽略名单。
这里只需要放行迁移种下的固定记录；现在按 VO 名字整表跳过后，后续如果逻辑误写出额外行，makeSureAllEntitiesDeleted() 也不会再报。更稳妥的是用 AllowedDBRemaining 只允许 name = 'ha-network-group' 这一条。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@testlib/src/main/java/org/zstack/testlib/EnvSpec.groovy` around lines 710 -
711, 不要把整个 HaNetworkGroupGlobalConfigVersionVO 整表跳过；在 EnvSpec.groovy 中，移除
"HaNetworkGroupGlobalConfigVersionVO" 从忽略名单（当前列在那段数组里），改为在 AllowedDBRemaining
里添加一条只放行 name = 'ha-network-group' 的记录，从而保留 makeSureAllEntitiesDeleted() 对该 VO
其它意外记录的检查；参考相关符号：AllowedDBRemaining、makeSureAllEntitiesDeleted 和
HaNetworkGroupGlobalConfigVersionVO 以定位并修改逻辑。
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-84164
Change-Id: I736967737a6963726b666776746b677a69717278
sync from gitlab !9639