<feature>[dpu-bm2]: support dpu baremetal2 instance#3492
Open
MatheMatrix wants to merge 1 commit into5.5.12from
Open
<feature>[dpu-bm2]: support dpu baremetal2 instance#3492MatheMatrix wants to merge 1 commit into5.5.12from
MatheMatrix wants to merge 1 commit into5.5.12from
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向 Changes
Estimated code review effort🎯 1 (微不足道) | ⏱️ ~2 分钟 Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (1 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![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 (3)
conf/db/upgrade/V5.5.12__schema.sql (3)
44-44:⚠️ Potential issue | 🟡 Minor
createDate使用了禁止的默认值'0000-00-00 00:00:00'。根据编码规范，不应使用
DEFAULT 0000-00-00 00:00:00，应改用DEFAULT CURRENT_TIMESTAMP。🔧 建议的修复
- `createDate` timestamp NOT NULL DEFAULT '0000-00-00 00:00:00', + `createDate` timestamp NOT NULL 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` at line 44, The column definition for createDate uses the disallowed literal default '0000-00-00 00:00:00'; update the schema to use a proper timestamp default by changing the createDate column definition to use DEFAULT CURRENT_TIMESTAMP (keeping NOT NULL) so new rows get the current time; if this is part of a migration/ALTER, ensure the ALTER TABLE/CREATE TABLE statement for createDate is updated accordingly and consider backfilling existing rows if necessary.
76-76:⚠️ Potential issue | 🟡 Minor
createDate使用了禁止的默认值'0000-00-00 00:00:00'。与上述问题相同，此处也应修改为
DEFAULT CURRENT_TIMESTAMP。🔧 建议的修复
- `createDate` TIMESTAMP NOT NULL DEFAULT '0000-00-00 00:00:00', + `createDate` TIMESTAMP NOT NULL 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` at line 76, The column definition for `createDate` in V5.5.12__schema.sql currently uses the prohibited default '0000-00-00 00:00:00'; update the `createDate` column definition to use a valid timestamp default (e.g., DEFAULT CURRENT_TIMESTAMP) and ensure the column remains NOT NULL; locate the `createDate` column definition in the migration file and replace the literal zero-datetime default with DEFAULT CURRENT_TIMESTAMP so migrations use a valid timestamp value.
112-125:⚠️ Potential issue | 🟠 MajorDROP FOREIGN KEY 缺少存在性检查，升级脚本无法安全重复执行。
直接使用
DROP FOREIGN KEY在外键不存在时会立即报错中止。这会导致以下场景升级失败：
- 全新数据库安装（表存在但约束从未创建）
- 升级脚本被重复执行
- 从某些历史版本升级时外键不存在
项目在 V5.5.0 中已建立的模式（使用
information_schema.TABLE_CONSTRAINTS检查）应当在此处保持一致。建议改用存储过程包装，添加存在性检查后再执行 DROP：修复方案
DROP PROCEDURE IF EXISTS DROP_FK_IF_EXISTS; DELIMITER $$ CREATE PROCEDURE DROP_FK_IF_EXISTS() BEGIN IF EXISTS (SELECT 1 FROM information_schema.TABLE_CONSTRAINTS WHERE CONSTRAINT_NAME = 'fkBareMetal2InstanceVOGatewayVO' AND TABLE_SCHEMA = 'zstack' AND TABLE_NAME = 'BareMetal2InstanceVO') THEN ALTER TABLE `zstack`.`BareMetal2InstanceVO` DROP FOREIGN KEY `fkBareMetal2InstanceVOGatewayVO`; END IF; IF EXISTS (SELECT 1 FROM information_schema.TABLE_CONSTRAINTS WHERE CONSTRAINT_NAME = 'fkBareMetal2InstanceVOGatewayVO1' AND TABLE_SCHEMA = 'zstack' AND TABLE_NAME = 'BareMetal2InstanceVO') THEN ALTER TABLE `zstack`.`BareMetal2InstanceVO` DROP FOREIGN KEY `fkBareMetal2InstanceVOGatewayVO1`; END IF; END$$ DELIMITER ; CALL DROP_FK_IF_EXISTS(); DROP PROCEDURE IF EXISTS DROP_FK_IF_EXISTS; ALTER TABLE `zstack`.`BareMetal2InstanceVO` ADD CONSTRAINT `fkBareMetal2InstanceVOGatewayVO` FOREIGN KEY (`gatewayUuid`) REFERENCES `HostEO` (`uuid`) ON DELETE SET NULL, ADD CONSTRAINT `fkBareMetal2InstanceVOGatewayVO1` FOREIGN KEY (`lastGatewayUuid`) REFERENCES `HostEO` (`uuid`) ON DELETE SET NULL;🤖 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 112 - 125, The ALTER TABLE DROP FOREIGN KEY statements for BareMetal2InstanceVO (constraints fkBareMetal2InstanceVOGatewayVO and fkBareMetal2InstanceVOGatewayVO1) lack existence checks and will error if rerun or missing; wrap the drops in a safe stored-procedure pattern (e.g., DROP_FK_IF_EXISTS) that queries information_schema.TABLE_CONSTRAINTS for CONSTRAINT_NAME = 'fkBareMetal2InstanceVOGatewayVO' / 'fkBareMetal2InstanceVOGatewayVO1' and TABLE_SCHEMA='zstack' and TABLE_NAME='BareMetal2InstanceVO' and only then executes ALTER TABLE ... DROP FOREIGN KEY, call and drop the procedure, then proceed to ADD CONSTRAINT for fkBareMetal2InstanceVOGatewayVO and fkBareMetal2InstanceVOGatewayVO1 as before.
🤖 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`:
- Line 44: The column definition for createDate uses the disallowed literal
default '0000-00-00 00:00:00'; update the schema to use a proper timestamp
default by changing the createDate column definition to use DEFAULT
CURRENT_TIMESTAMP (keeping NOT NULL) so new rows get the current time; if this
is part of a migration/ALTER, ensure the ALTER TABLE/CREATE TABLE statement for
createDate is updated accordingly and consider backfilling existing rows if
necessary.
- Line 76: The column definition for `createDate` in V5.5.12__schema.sql
currently uses the prohibited default '0000-00-00 00:00:00'; update the
`createDate` column definition to use a valid timestamp default (e.g., DEFAULT
CURRENT_TIMESTAMP) and ensure the column remains NOT NULL; locate the
`createDate` column definition in the migration file and replace the literal
zero-datetime default with DEFAULT CURRENT_TIMESTAMP so migrations use a valid
timestamp value.
- Around line 112-125: The ALTER TABLE DROP FOREIGN KEY statements for
BareMetal2InstanceVO (constraints fkBareMetal2InstanceVOGatewayVO and
fkBareMetal2InstanceVOGatewayVO1) lack existence checks and will error if rerun
or missing; wrap the drops in a safe stored-procedure pattern (e.g.,
DROP_FK_IF_EXISTS) that queries information_schema.TABLE_CONSTRAINTS for
CONSTRAINT_NAME = 'fkBareMetal2InstanceVOGatewayVO' /
'fkBareMetal2InstanceVOGatewayVO1' and TABLE_SCHEMA='zstack' and
TABLE_NAME='BareMetal2InstanceVO' and only then executes ALTER TABLE ... DROP
FOREIGN KEY, call and drop the procedure, then proceed to ADD CONSTRAINT for
fkBareMetal2InstanceVOGatewayVO and fkBareMetal2InstanceVOGatewayVO1 as before.
ℹ️ 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: 25c75c8f-47e5-4b62-b8a4-df3936a249b9
📒 Files selected for processing (1)
conf/db/upgrade/V5.5.12__schema.sql
MatheMatrix
force-pushed
the
sync/shan.wu/feature-dpu-baremetal@@2
branch
2 times, most recently
from
5402f68 to
d6e611a
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
header/src/main/java/org/zstack/header/host/APIQueryHostReply.java (1)
33-33: 预存在问题：setClusterUuid被重复调用。第 33 行和第 38 行都调用了
hi.setClusterUuid(uuid())，第二次调用会覆盖第一次的值。这可能是复制粘贴错误，第 38 行或许应该设置其他字段。♻️ 建议移除重复调用
hi.setClusterUuid(uuid()); hi.setZoneUuid(uuid()); hi.setUuid(uuid()); - hi.setClusterUuid(uuid());Also applies to: 38-38
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/host/APIQueryHostReply.java` at line 33, The code calls hi.setClusterUuid(uuid()) twice (duplicate setter on the same object), which overwrites the first value; in class APIQueryHostReply locate the twin calls to hi.setClusterUuid(uuid()) and remove the redundant one (or replace the second call with the correct setter if a different field was intended), ensuring hi only has setClusterUuid called once and that any intended other field is set via its proper setter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@header/src/main/java/org/zstack/header/host/APIQueryHostReply.java`:
- Line 33: The code calls hi.setClusterUuid(uuid()) twice (duplicate setter on
the same object), which overwrites the first value; in class APIQueryHostReply
locate the twin calls to hi.setClusterUuid(uuid()) and remove the redundant one
(or replace the second call with the correct setter if a different field was
intended), ensuring hi only has setClusterUuid called once and that any intended
other field is set via its proper setter.
ℹ️ 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: b49904a1-a8c1-461b-a4a3-c1cae6e1e05e
⛔ Files ignored due to path filters (1)
sdk/src/main/java/org/zstack/sdk/BareMetal2DpuChassisInventory.javais excluded by!sdk/**
📒 Files selected for processing (1)
header/src/main/java/org/zstack/header/host/APIQueryHostReply.java
MatheMatrix
force-pushed
the
sync/shan.wu/feature-dpu-baremetal@@2
branch
6 times, most recently
from
5dc7d83 to
472ed51
Compare
support L2 no vlan network Resolves/Related: ZSTAC-82781 Change-Id: I636d637a7168656a6c726c6769777a726e616974
MatheMatrix
force-pushed
the
sync/shan.wu/feature-dpu-baremetal@@2
branch
from
472ed51 to
fc6c37f
Compare
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
1 participant
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.
support L2 no vlan network
Resolves/Related: ZSTAC-82781
Change-Id: I636d637a7168656a6c726c6769777a726e616974
sync from gitlab !9351