Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions conf/db/upgrade/V5.5.6__schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,78 @@ ALTER TABLE `GuestVmScriptExecutedRecordDetailVO`

CALL ADD_COLUMN('PciDeviceVO', 'vmPciDeviceAddress', 'varchar(32)', 1, NULL);
CALL ADD_COLUMN('MdevDeviceVO', 'mdevDeviceAddress', 'varchar(32)', 1, NULL);

DELIMITER $$

CREATE PROCEDURE UpdateBareMetal2InstanceProvisionNicVO()
BEGIN
DECLARE instanceUuid_exists INT;
DECLARE isPrimaryProvisionNic_exists_in_ProvisionNicVO INT;
DECLARE isPrimaryProvisionNic_exists_in_ChassisNicVO INT;

DECLARE EXIT HANDLER FOR SQLEXCEPTION
BEGIN
ROLLBACK;
SIGNAL SQLSTATE '45000' SET MESSAGE_TEXT = 'An error occurred during the update process.';
END;

START TRANSACTION;

SELECT COUNT(*)
INTO instanceUuid_exists
FROM INFORMATION_SCHEMA.COLUMNS
WHERE TABLE_SCHEMA = 'zstack'
AND TABLE_NAME = 'BareMetal2InstanceProvisionNicVO'
AND COLUMN_NAME = 'instanceUuid';

IF instanceUuid_exists = 0 THEN
CALL ADD_COLUMN('BareMetal2InstanceProvisionNicVO', 'instanceUuid', 'VARCHAR(32)', FALSE, '');

UPDATE `zstack`.`BareMetal2InstanceProvisionNicVO`
SET `instanceUuid` = `uuid`;

ALTER TABLE `zstack`.`BareMetal2InstanceProvisionNicVO`
DROP FOREIGN KEY `fkBareMetal2InstanceProvisionNicVOInstanceVO`;

CALL ADD_CONSTRAINT('BareMetal2InstanceProvisionNicVO', 'fkBareMetal2InstanceProvisionNicVOInstanceVO',
'instanceUuid', 'BareMetal2InstanceVO', 'uuid', 'CASCADE');
Comment on lines +39 to +43
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

外键删除缺少存在性检查,可能导致脚本不可重复执行。

根据 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.


UPDATE `zstack`.`BareMetal2InstanceProvisionNicVO`
SET `uuid` = REPLACE(UUID(), '-', '');
END IF;

SELECT COUNT(*)
INTO isPrimaryProvisionNic_exists_in_ProvisionNicVO
FROM INFORMATION_SCHEMA.COLUMNS
WHERE TABLE_SCHEMA = 'zstack'
AND TABLE_NAME = 'BareMetal2InstanceProvisionNicVO'
AND COLUMN_NAME = 'isPrimaryProvisionNic';

IF isPrimaryProvisionNic_exists_in_ProvisionNicVO = 0 THEN
CALL ADD_COLUMN('BareMetal2InstanceProvisionNicVO', 'isPrimaryProvisionNic', 'BOOLEAN', FALSE, FALSE);

UPDATE `zstack`.`BareMetal2InstanceProvisionNicVO`
SET `isPrimaryProvisionNic` = TRUE;
END IF;

SELECT COUNT(*)
INTO isPrimaryProvisionNic_exists_in_ChassisNicVO
FROM INFORMATION_SCHEMA.COLUMNS
WHERE TABLE_SCHEMA = 'zstack'
AND TABLE_NAME = 'BareMetal2ChassisNicVO'
AND COLUMN_NAME = 'isPrimaryProvisionNic';

IF isPrimaryProvisionNic_exists_in_ChassisNicVO = 0 THEN
CALL ADD_COLUMN('BareMetal2ChassisNicVO', 'isPrimaryProvisionNic', 'BOOLEAN', FALSE, FALSE);

UPDATE `zstack`.`BareMetal2ChassisNicVO`
SET `isPrimaryProvisionNic` = TRUE
WHERE `isProvisionNic` = TRUE;
END IF;

COMMIT;
END$$

DELIMITER ;
CALL UpdateBareMetal2InstanceProvisionNicVO();
DROP PROCEDURE IF EXISTS UpdateBareMetal2InstanceProvisionNicVO;
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,12 @@
import org.zstack.header.storage.primary.*;
import org.zstack.header.storage.primary.VolumeSnapshotCapability.VolumeSnapshotArrangementType;
import org.zstack.header.storage.snapshot.*;
import org.zstack.header.tag.SystemTagVO;
import org.zstack.header.tag.SystemTagVO_;
import org.zstack.header.vm.VmInstanceSpec;
import org.zstack.header.vm.VmInstanceSpec.ImageSpec;
import org.zstack.header.vm.VmInstanceVO;
import org.zstack.header.vm.VmInstanceVO_;
import org.zstack.header.vo.ResourceVO;
import org.zstack.header.volume.*;
import org.zstack.identity.AccountManager;
Expand Down Expand Up @@ -485,6 +489,8 @@ public static class CloneRsp extends AgentResponse {
public Long size;
public Long actualSize;
public String installPath;
public String volumeId;
public String volumeStatus;

public String getInstallPath() {
return installPath;
Expand All @@ -493,6 +499,22 @@ public String getInstallPath() {
public void setInstallPath(String installPath) {
this.installPath = installPath;
}

public String getVolumeId() {
return volumeId;
}

public void setVolumeId(String volumeId) {
this.volumeId = volumeId;
}

public String getVolumeStatus() {
return volumeStatus;
}

public void setVolumeStatus(String volumeStatus) {
this.volumeStatus = volumeStatus;
}
}

public static class FlattenCmd extends AgentCommand implements HasThreadContext {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package org.zstack.storage.ceph.primary;

import org.zstack.header.volume.VolumeVO;

public interface CephPrimaryStorageCheckInstanceTypeExtensionPoint {
Boolean isSupportCloneByThirdParty(String uuid);

void convertToBlockVolume(VolumeVO vo);

Boolean isBlockVolume(String uuid);

void populateBlockVolumeDetails(String uuid, String volumeId, String volumeStatus);
Comment on lines +5 to +12
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

为接口及方法补充 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` 标签及简短示例/注意事项以便调用者理解行为与期望输入输出。

}
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@ public java.lang.Boolean getIsProvisionNic() {
return this.isProvisionNic;
}

public java.lang.Boolean isPrimaryProvisionNic;
public void setIsPrimaryProvisionNic(java.lang.Boolean isPrimaryProvisionNic) {
this.isPrimaryProvisionNic = isPrimaryProvisionNic;
}
public java.lang.Boolean getIsPrimaryProvisionNic() {
return this.isPrimaryProvisionNic;
}

public java.sql.Timestamp createDate;
public void setCreateDate(java.sql.Timestamp createDate) {
this.createDate = createDate;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package org.zstack.sdk;

import org.zstack.sdk.BareMetal2InstanceProvisionNicInventory;


public class BareMetal2InstanceInventory extends org.zstack.sdk.VmInstanceInventory {

Expand Down Expand Up @@ -84,12 +84,12 @@ public boolean getIsLatestAgent() {
return this.isLatestAgent;
}

public BareMetal2InstanceProvisionNicInventory provisionNic;
public void setProvisionNic(BareMetal2InstanceProvisionNicInventory provisionNic) {
this.provisionNic = provisionNic;
public java.util.List provisionNics;
public void setProvisionNics(java.util.List provisionNics) {
this.provisionNics = provisionNics;
}
public BareMetal2InstanceProvisionNicInventory getProvisionNic() {
return this.provisionNic;
public java.util.List getProvisionNics() {
return this.provisionNics;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,22 @@ public java.lang.String getGateway() {
return this.gateway;
}

public java.lang.String instanceUuid;
public void setInstanceUuid(java.lang.String instanceUuid) {
this.instanceUuid = instanceUuid;
}
public java.lang.String getInstanceUuid() {
return this.instanceUuid;
}

public java.lang.Boolean isPrimaryProvisionNic;
public void setIsPrimaryProvisionNic(java.lang.Boolean isPrimaryProvisionNic) {
this.isPrimaryProvisionNic = isPrimaryProvisionNic;
}
public java.lang.Boolean getIsPrimaryProvisionNic() {
return this.isPrimaryProvisionNic;
}

public java.lang.String metadata;
public void setMetadata(java.lang.String metadata) {
this.metadata = metadata;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7678,6 +7678,10 @@ public class CloudOperationsErrorCode {

public static final String ORG_ZSTACK_BAREMETAL2_INSTANCE_10087 = "ORG_ZSTACK_BAREMETAL2_INSTANCE_10087";

public static final String ORG_ZSTACK_BAREMETAL2_INSTANCE_10088 = "ORG_ZSTACK_BAREMETAL2_INSTANCE_10088";

public static final String ORG_ZSTACK_BAREMETAL2_INSTANCE_10089 = "ORG_ZSTACK_BAREMETAL2_INSTANCE_10089";

public static final String ORG_ZSTACK_CRYPTO_SECURITYMACHINE_SECRETRESOURCEPOOL_10000 = "ORG_ZSTACK_CRYPTO_SECURITYMACHINE_SECRETRESOURCEPOOL_10000";

public static final String ORG_ZSTACK_CRYPTO_SECURITYMACHINE_SECRETRESOURCEPOOL_10001 = "ORG_ZSTACK_CRYPTO_SECURITYMACHINE_SECRETRESOURCEPOOL_10001";
Expand Down Expand Up @@ -12824,6 +12828,8 @@ public class CloudOperationsErrorCode {

public static final String ORG_ZSTACK_STORAGE_CEPH_PRIMARY_10052 = "ORG_ZSTACK_STORAGE_CEPH_PRIMARY_10052";

public static final String ORG_ZSTACK_STORAGE_CEPH_PRIMARY_10053 = "ORG_ZSTACK_STORAGE_CEPH_PRIMARY_10053";

public static final String ORG_ZSTACK_EXTERNALSERVICE_CRONJOB_10000 = "ORG_ZSTACK_EXTERNALSERVICE_CRONJOB_10000";

public static final String ORG_ZSTACK_EXTERNALSERVICE_CRONJOB_10001 = "ORG_ZSTACK_EXTERNALSERVICE_CRONJOB_10001";
Expand Down Expand Up @@ -13706,6 +13712,10 @@ public class CloudOperationsErrorCode {

public static final String ORG_ZSTACK_BAREMETAL2_CHASSIS_10023 = "ORG_ZSTACK_BAREMETAL2_CHASSIS_10023";

public static final String ORG_ZSTACK_BAREMETAL2_CHASSIS_10024 = "ORG_ZSTACK_BAREMETAL2_CHASSIS_10024";

public static final String ORG_ZSTACK_BAREMETAL2_CHASSIS_10025 = "ORG_ZSTACK_BAREMETAL2_CHASSIS_10025";

public static final String ORG_ZSTACK_BAREMETAL2_CLUSTER_10000 = "ORG_ZSTACK_BAREMETAL2_CLUSTER_10000";

public static final String ORG_ZSTACK_BAREMETAL2_CLUSTER_10001 = "ORG_ZSTACK_BAREMETAL2_CLUSTER_10001";
Expand Down