forked from zstackio/zstack
-
Notifications
You must be signed in to change notification settings - Fork 0
<fix>[storage]: filter non-preferred backup storage types before sorting #3395
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
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
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
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
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
200 changes: 200 additions & 0 deletions
200
...st/integration/storage/primary/addon/ExternalPrimaryStorageSelectBackupStorageCase.groovy
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,200 @@ | ||
| package org.zstack.test.integration.storage.primary.addon | ||
|
|
||
| import org.zstack.core.Platform | ||
| import org.zstack.core.cloudbus.CloudBus | ||
| import org.zstack.core.db.DatabaseFacade | ||
| import org.zstack.core.db.SQL | ||
| import org.zstack.header.message.MessageReply | ||
| import org.zstack.header.storage.backup.BackupStorageEO | ||
| import org.zstack.header.storage.backup.BackupStorageState | ||
| import org.zstack.header.storage.backup.BackupStorageStatus | ||
| import org.zstack.header.storage.backup.BackupStorageZoneRefVO | ||
| import org.zstack.header.storage.backup.BackupStorageZoneRefVO_ | ||
| import org.zstack.header.storage.primary.PrimaryStorageConstant | ||
| import org.zstack.header.storage.primary.SelectBackupStorageMsg | ||
| import org.zstack.header.storage.primary.SelectBackupStorageReply | ||
| import org.zstack.sdk.PrimaryStorageInventory | ||
| import org.zstack.sdk.ZoneInventory | ||
| import org.zstack.test.integration.storage.StorageTest | ||
| import org.zstack.testlib.EnvSpec | ||
| import org.zstack.testlib.SubCase | ||
| import org.zstack.utils.data.SizeUnit | ||
|
|
||
| /** | ||
| * ZSTAC-71706: ExternalPrimaryStorage backup storage selection in mixed environment. | ||
| * | ||
| * Bug: List.indexOf() returns -1 for types not in preferBsTypes, | ||
| * causing ascending sort to place non-preferred types (e.g. VCenterBackupStorage) | ||
| * before preferred types in the sorted result. | ||
| * | ||
| * Fix: Filter out non-preferred backup storage types before sorting by preference. | ||
| * | ||
| * This case sets up a ZBS ExternalPrimaryStorage (preferBsTypes = [ImageStoreBackupStorage]) | ||
| * with multiple backup storage types attached to the zone, then sends SelectBackupStorageMsg | ||
| * via CloudBus to verify the handler selects the correct preferred backup storage. | ||
| */ | ||
| class ExternalPrimaryStorageSelectBackupStorageCase extends SubCase { | ||
| EnvSpec env | ||
| ZoneInventory zone | ||
| PrimaryStorageInventory ps | ||
| DatabaseFacade dbf | ||
| CloudBus bus | ||
| List<String> manualBsUuids = [] | ||
|
|
||
| @Override | ||
| void clean() { | ||
| manualBsUuids.each { uuid -> | ||
| SQL.New(BackupStorageZoneRefVO.class) | ||
| .eq(BackupStorageZoneRefVO_.backupStorageUuid, uuid) | ||
| .hardDelete() | ||
| dbf.removeByPrimaryKey(uuid, BackupStorageEO.class) | ||
| } | ||
| env.delete() | ||
| } | ||
|
|
||
| @Override | ||
| void setup() { | ||
| useSpring(StorageTest.springSpec) | ||
| } | ||
|
|
||
| @Override | ||
| void environment() { | ||
| env = makeEnv { | ||
| sftpBackupStorage { | ||
| name = "sftp" | ||
| url = "/sftp" | ||
| username = "root" | ||
| password = "password" | ||
| hostname = "127.0.0.2" | ||
|
|
||
| image { | ||
| name = "image" | ||
| url = "http://zstack.org/download/test.qcow2" | ||
| size = SizeUnit.GIGABYTE.toByte(1) | ||
| virtio = true | ||
| } | ||
| } | ||
|
|
||
| zone { | ||
| name = "zone" | ||
| description = "test" | ||
|
|
||
| cluster { | ||
| name = "cluster" | ||
| hypervisorType = "KVM" | ||
|
|
||
| kvm { | ||
| name = "kvm" | ||
| managementIp = "127.0.0.1" | ||
| username = "root" | ||
| password = "password" | ||
| } | ||
|
|
||
| attachL2Network("l2") | ||
| } | ||
|
|
||
| l2NoVlanNetwork { | ||
| name = "l2" | ||
| physicalInterface = "eth0" | ||
|
|
||
| l3Network { | ||
| name = "l3" | ||
|
|
||
| ip { | ||
| startIp = "192.168.100.10" | ||
| endIp = "192.168.100.100" | ||
| netmask = "255.255.255.0" | ||
| gateway = "192.168.100.1" | ||
| } | ||
| } | ||
| } | ||
|
|
||
| externalPrimaryStorage { | ||
| name = "zbs-ps" | ||
| identity = "zbs" | ||
| defaultOutputProtocol = "CBD" | ||
| config = '{"mdsUrls":["root:password@127.0.1.1","root:password@127.0.1.2","root:password@127.0.1.3"],"logicalPoolName":"lpool1"}' | ||
| url = "zbs" | ||
| } | ||
|
|
||
| attachBackupStorage("sftp") | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| void test() { | ||
| env.create { | ||
| zone = env.inventoryByName("zone") as ZoneInventory | ||
| ps = env.inventoryByName("zbs-ps") as PrimaryStorageInventory | ||
| dbf = bean(DatabaseFacade.class) | ||
| bus = bean(CloudBus.class) | ||
|
|
||
| testErrorWhenNoPreferredTypeAvailable() | ||
| testSelectPreferredOverNonPreferred() | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * When only non-preferred backup storage types exist in the zone, | ||
| * the selection should return an error (no matching preferred types). | ||
| * Zone has SftpBackupStorage (from env) and VCenterBackupStorage, | ||
| * neither of which is in zbs's preferBsTypes [ImageStoreBackupStorage]. | ||
| */ | ||
| void testErrorWhenNoPreferredTypeAvailable() { | ||
| createAndAttachBackupStorage("vcenter-bs", "VCenterBackupStorage") | ||
|
|
||
| SelectBackupStorageMsg msg = new SelectBackupStorageMsg() | ||
| msg.setPrimaryStorageUuid(ps.uuid) | ||
| msg.setRequiredSize(SizeUnit.MEGABYTE.toByte(1)) | ||
| bus.makeTargetServiceIdByResourceUuid(msg, PrimaryStorageConstant.SERVICE_ID, ps.uuid) | ||
| MessageReply reply = bus.call(msg) | ||
|
|
||
| assert !reply.isSuccess() : "Should fail when no preferred BS type is available" | ||
| } | ||
|
|
||
| /** | ||
| * Reproduces ZSTAC-71706: zone has both ImageStoreBackupStorage (preferred) | ||
| * and VCenterBackupStorage (non-preferred, created in previous test). | ||
| * Before the fix, indexOf() returns -1 for VCenterBackupStorage causing | ||
| * it to sort before ImageStoreBackupStorage. After the fix, non-preferred | ||
| * types are filtered out entirely, and ImageStoreBackupStorage is correctly selected. | ||
| */ | ||
| void testSelectPreferredOverNonPreferred() { | ||
| createAndAttachBackupStorage("imagestore-bs", "ImageStoreBackupStorage") | ||
|
|
||
| SelectBackupStorageMsg msg = new SelectBackupStorageMsg() | ||
| msg.setPrimaryStorageUuid(ps.uuid) | ||
| msg.setRequiredSize(SizeUnit.MEGABYTE.toByte(1)) | ||
| bus.makeTargetServiceIdByResourceUuid(msg, PrimaryStorageConstant.SERVICE_ID, ps.uuid) | ||
| MessageReply reply = bus.call(msg) | ||
|
|
||
| assert reply.isSuccess() : "SelectBackupStorageMsg should succeed" | ||
| SelectBackupStorageReply bsReply = reply as SelectBackupStorageReply | ||
| assert bsReply.inventory != null | ||
| assert bsReply.inventory.type == "ImageStoreBackupStorage" : | ||
| "Should select preferred ImageStoreBackupStorage, but got ${bsReply.inventory.type}" | ||
| } | ||
|
|
||
| private void createAndAttachBackupStorage(String name, String type) { | ||
| String uuid = Platform.getUuid() | ||
|
|
||
| def bsEo = new BackupStorageEO() | ||
| bsEo.setUuid(uuid) | ||
| bsEo.setName(name) | ||
| bsEo.setType(type) | ||
| bsEo.setState(BackupStorageState.Enabled) | ||
| bsEo.setStatus(BackupStorageStatus.Connected) | ||
| bsEo.setTotalCapacity(SizeUnit.TERABYTE.toByte(100)) | ||
| bsEo.setAvailableCapacity(SizeUnit.TERABYTE.toByte(100)) | ||
| bsEo.setUrl("http://test-" + name) | ||
| dbf.persist(bsEo) | ||
|
|
||
| def ref = new BackupStorageZoneRefVO() | ||
| ref.setBackupStorageUuid(uuid) | ||
| ref.setZoneUuid(zone.uuid) | ||
| dbf.persist(ref) | ||
|
|
||
| manualBsUuids.add(uuid) | ||
| } | ||
| } |
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: MatheMatrix/zstack
Length of output: 422
🏁 Script executed:
Repository: MatheMatrix/zstack
Length of output: 144
🏁 Script executed:
Repository: MatheMatrix/zstack
Length of output: 1664
🏁 Script executed:
Repository: MatheMatrix/zstack
Length of output: 1185
🏁 Script executed:
Repository: MatheMatrix/zstack
Length of output: 144
🏁 Script executed:
Repository: MatheMatrix/zstack
Length of output: 96
🏁 Script executed:
Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
Repository: MatheMatrix/zstack
Length of output: 50375
🏁 Script executed:
Repository: MatheMatrix/zstack
Length of output: 3356
🏁 Script executed:
Repository: MatheMatrix/zstack
Length of output: 50375
移除可选的格式优化建议，因其会破坏国际化一致性。
错误码
ORG_ZSTACK_STORAGE_ADDON_PRIMARY_10015已正确定义。然而，使用String.join(", ", preferBsTypes)的建议应被撤销。虽然
String.join()在视觉上更整洁（避免方括号），但它改变了传递给国际化系统的参数格式。i18n 文件中已定义的翻译（如中文："匹配首选类型 %s"）期望接收的是 List 的原始格式。改变参数格式将导致翻译的一致性问题，违反 ZStack 维持 i18n 兼容性的原则。保留当前实现，直接使用preferBsTypes作为参数。🤖 Prompt for AI Agents