<fix>[ceph]: support thirdparty_ceph to bm root volume#3225
Open
zstack-robot-1 wants to merge 2 commits into5.5.6from
Open
<fix>[ceph]: support thirdparty_ceph to bm root volume#3225zstack-robot-1 wants to merge 2 commits into5.5.6from
zstack-robot-1 wants to merge 2 commits into5.5.6from
Conversation
added 2 commits
January 19, 2026 11:33
support thirdparty_ceph to bm root volume Resolves: ZSTAC-73396 Change-Id: I6f6f616777677a6f74696c6c736c737a626f7863 (cherry picked from commit 827e880)
Resolves: ZSTAC-81282 Change-Id: I69776a6b6f6673726e6b6376627978776f757765
概览该变更引入了数据库迁移脚本用于更新BareMetal2实例配置表结构，扩展了Ceph存储卷克隆响应的字段，新增扩展点接口用于块卷处理，并补充相关错误代码常量。 变更详情
代码审查工作量评估🎯 3 (中等) | ⏱️ ~20 分钟 诗歌
🚥 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
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.40.5)plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageBase.javautils/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
🤖 Fix all issues with AI agents
In `@conf/db/upgrade/V5.5.6__schema.sql`:
- Around line 39-43: The DROP FOREIGN KEY should be guarded by an existence
check against information_schema to make the migration idempotent: before
calling ALTER TABLE `BareMetal2InstanceProvisionNicVO` DROP FOREIGN KEY
`fkBareMetal2InstanceProvisionNicVOInstanceVO`, query
information_schema.table_constraints / key_column_usage to confirm the
constraint `fkBareMetal2InstanceProvisionNicVOInstanceVO` on table
`BareMetal2InstanceProvisionNicVO` exists, and only then execute the ALTER;
likewise, before calling CALL ADD_CONSTRAINT('BareMetal2InstanceProvisionNicVO',
'fkBareMetal2InstanceProvisionNicVOInstanceVO', ...) check that the foreign key
does not already exist to avoid duplicate-constraint errors. Use a
conditional/select-into or dynamic SQL block to perform these checks and then
run ALTER TABLE or CALL ADD_CONSTRAINT accordingly.
In
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageCheckInstanceTypeExtensionPoint.java`:
- Around line 5-12: 该接口 CephPrimaryStorageCheckInstanceTypeExtensionPoint 及其方法缺少
Javadoc 注释；请为接口本身及所有方法（isSupportCloneByThirdParty, convertToBlockVolume,
isBlockVolume, populateBlockVolumeDetails）补充规范的
Javadoc，说明接口用途、每个方法的功能、参数含义（uuid、vo、volumeId、volumeStatus）、返回值含义和可能抛出的异常或空值语义，并使用项目约定的
`@param` 和 `@return` 标签及简短示例/注意事项以便调用者理解行为与期望输入输出。
Comment on lines
+39
to
+43
| ALTER TABLE `zstack`.`BareMetal2InstanceProvisionNicVO` | ||
| DROP FOREIGN KEY `fkBareMetal2InstanceProvisionNicVOInstanceVO`; | ||
|
|
||
| CALL ADD_CONSTRAINT('BareMetal2InstanceProvisionNicVO', 'fkBareMetal2InstanceProvisionNicVOInstanceVO', | ||
| 'instanceUuid', 'BareMetal2InstanceVO', 'uuid', 'CASCADE'); |
There was a problem hiding this comment.
外键删除缺少存在性检查，可能导致脚本不可重复执行。
根据 ZStack 数据库升级脚本的最佳实践，应通过 information_schema 检查约束是否存在后再删除，以确保脚本的幂等性。如果外键不存在（如部分执行后重跑），此语句会抛出异常。
🔧 建议添加外键存在性检查
+ SET `@fk_exists` = (SELECT COUNT(*) FROM INFORMATION_SCHEMA.TABLE_CONSTRAINTS
+ WHERE CONSTRAINT_SCHEMA = 'zstack'
+ AND TABLE_NAME = 'BareMetal2InstanceProvisionNicVO'
+ AND CONSTRAINT_NAME = 'fkBareMetal2InstanceProvisionNicVOInstanceVO'
+ AND CONSTRAINT_TYPE = 'FOREIGN KEY');
+
+ IF `@fk_exists` > 0 THEN
ALTER TABLE `zstack`.`BareMetal2InstanceProvisionNicVO`
DROP FOREIGN KEY `fkBareMetal2InstanceProvisionNicVOInstanceVO`;
+ END IF;Based on learnings，ZStack 升级脚本需要通过 information_schema 检查约束的存在性来确保脚本可以安全重复执行。
📝 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
| ALTER TABLE `zstack`.`BareMetal2InstanceProvisionNicVO` | |
| DROP FOREIGN KEY `fkBareMetal2InstanceProvisionNicVOInstanceVO`; | |
| CALL ADD_CONSTRAINT('BareMetal2InstanceProvisionNicVO', 'fkBareMetal2InstanceProvisionNicVOInstanceVO', | |
| 'instanceUuid', 'BareMetal2InstanceVO', 'uuid', 'CASCADE'); | |
| SET `@fk_exists` = (SELECT COUNT(*) FROM INFORMATION_SCHEMA.TABLE_CONSTRAINTS | |
| WHERE CONSTRAINT_SCHEMA = 'zstack' | |
| AND TABLE_NAME = 'BareMetal2InstanceProvisionNicVO' | |
| AND CONSTRAINT_NAME = 'fkBareMetal2InstanceProvisionNicVOInstanceVO' | |
| AND CONSTRAINT_TYPE = 'FOREIGN KEY'); | |
| IF `@fk_exists` > 0 THEN | |
| ALTER TABLE `zstack`.`BareMetal2InstanceProvisionNicVO` | |
| DROP FOREIGN KEY `fkBareMetal2InstanceProvisionNicVOInstanceVO`; | |
| END IF; | |
| CALL ADD_CONSTRAINT('BareMetal2InstanceProvisionNicVO', 'fkBareMetal2InstanceProvisionNicVOInstanceVO', | |
| 'instanceUuid', 'BareMetal2InstanceVO', 'uuid', 'CASCADE'); |
🤖 Prompt for AI Agents
In `@conf/db/upgrade/V5.5.6__schema.sql` around lines 39 - 43, The DROP FOREIGN
KEY should be guarded by an existence check against information_schema to make
the migration idempotent: before calling ALTER TABLE
`BareMetal2InstanceProvisionNicVO` DROP FOREIGN KEY
`fkBareMetal2InstanceProvisionNicVOInstanceVO`, query
information_schema.table_constraints / key_column_usage to confirm the
constraint `fkBareMetal2InstanceProvisionNicVOInstanceVO` on table
`BareMetal2InstanceProvisionNicVO` exists, and only then execute the ALTER;
likewise, before calling CALL ADD_CONSTRAINT('BareMetal2InstanceProvisionNicVO',
'fkBareMetal2InstanceProvisionNicVOInstanceVO', ...) check that the foreign key
does not already exist to avoid duplicate-constraint errors. Use a
conditional/select-into or dynamic SQL block to perform these checks and then
run ALTER TABLE or CALL ADD_CONSTRAINT accordingly.
Comment on lines
+5
to
+12
| public interface CephPrimaryStorageCheckInstanceTypeExtensionPoint { | ||
| Boolean isSupportCloneByThirdParty(String uuid); | ||
|
|
||
| void convertToBlockVolume(VolumeVO vo); | ||
|
|
||
| Boolean isBlockVolume(String uuid); | ||
|
|
||
| void populateBlockVolumeDetails(String uuid, String volumeId, String volumeStatus); |
There was a problem hiding this comment.
为接口及方法补充 Javadoc。
当前接口与方法缺少 Javadoc，违背项目规范，建议补齐说明与参数/返回值描述。
💡 示例补充
+/**
+ * Extension point for Ceph primary storage instance-type checks.
+ */
public interface CephPrimaryStorageCheckInstanceTypeExtensionPoint {
+ /**
+ * `@param` uuid resource UUID
+ * `@return` whether third-party clone is supported
+ */
Boolean isSupportCloneByThirdParty(String uuid);
+ /**
+ * Convert a volume to a block-volume representation if needed.
+ */
void convertToBlockVolume(VolumeVO vo);
+ /**
+ * `@param` uuid volume UUID
+ * `@return` whether the volume is a block volume
+ */
Boolean isBlockVolume(String uuid);
+ /**
+ * Populate block-volume details after clone.
+ */
void populateBlockVolumeDetails(String uuid, String volumeId, String volumeStatus);
}根据编码规范。
🤖 Prompt for AI Agents
In
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageCheckInstanceTypeExtensionPoint.java`
around lines 5 - 12, 该接口 CephPrimaryStorageCheckInstanceTypeExtensionPoint
及其方法缺少 Javadoc 注释；请为接口本身及所有方法（isSupportCloneByThirdParty, convertToBlockVolume,
isBlockVolume, populateBlockVolumeDetails）补充规范的
Javadoc，说明接口用途、每个方法的功能、参数含义（uuid、vo、volumeId、volumeStatus）、返回值含义和可能抛出的异常或空值语义，并使用项目约定的
`@param` 和 `@return` 标签及简短示例/注意事项以便调用者理解行为与期望输入输出。
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 thirdparty_ceph to bm root volume
Resolves: ZSTAC-73396
Change-Id: I6f6f616777677a6f74696c6c736c737a626f7863
(cherry picked from commit 827e880)
sync from gitlab !9044