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
Open
zstack-robot-2
wants to merge
1
commit into
5.5.12
Choose a base branch
from
sync/yaohua.wu/bugfix/ZSTAC-71706
base: 5.5.12
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
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
160 changes: 160 additions & 0 deletions
160
...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,160 @@ | ||
| package org.zstack.test.integration.storage.primary.addon | ||
|
|
||
| import org.zstack.header.storage.backup.BackupStorageVO | ||
| import org.zstack.storage.addon.primary.ExternalPrimaryStorage | ||
| import org.zstack.test.integration.storage.StorageTest | ||
| import org.zstack.testlib.EnvSpec | ||
| import org.zstack.testlib.SubCase | ||
|
|
||
| /** | ||
| * Test for ZSTAC-71706: ExternalPrimaryStorage backup storage selection logic. | ||
| * | ||
| * Bug: List.indexOf() returns -1 for types not in preferBsTypes, | ||
| * causing ascending sort to place non-preferred types (e.g. VCenterBackupStorage) | ||
| * before preferred types, leading to wrong backup storage selection. | ||
| * | ||
| * Fix: Filter out non-preferred backup storage types before sorting by preference. | ||
| * The filter+sort logic is extracted into ExternalPrimaryStorage.filterAndSortByPreferredTypes() | ||
| * which this test validates directly. | ||
| */ | ||
| class ExternalPrimaryStorageSelectBackupStorageCase extends SubCase { | ||
| EnvSpec env | ||
|
|
||
| @Override | ||
| void clean() { | ||
| env.delete() | ||
| } | ||
|
|
||
| @Override | ||
| void setup() { | ||
| useSpring(StorageTest.springSpec) | ||
| } | ||
|
|
||
| @Override | ||
| void environment() { | ||
| env = makeEnv {} | ||
| } | ||
|
|
||
| @Override | ||
| void test() { | ||
| env.create { | ||
| testBugReproduction() | ||
| testFilterThenSortSelectsPreferredType() | ||
| testPreferenceOrderingAmongMultipleTypes() | ||
| testEmptyResultWhenNoMatchingTypes() | ||
| testAllTypesArePreferred() | ||
| testSinglePreferredTypeAmongMany() | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Reproduces the original bug: indexOf returns -1 for non-preferred types, | ||
| * causing them to sort before preferred ones in ascending order. | ||
| * With the fix, non-preferred types are filtered out entirely. | ||
| */ | ||
| void testBugReproduction() { | ||
| def vcenterBs = makeBackupStorageVO("vcenter-uuid", "VCenterBackupStorage") | ||
| def imagestoreBs = makeBackupStorageVO("imagestore-uuid", "ImageStoreBackupStorage") | ||
|
|
||
| def candidates = [imagestoreBs, vcenterBs] | ||
| def preferBsTypes = ["ImageStoreBackupStorage"] | ||
|
|
||
| def result = ExternalPrimaryStorage.filterAndSortByPreferredTypes(candidates, preferBsTypes) | ||
|
|
||
| assert result.size() == 1 : "VCenterBackupStorage should be filtered out" | ||
| assert result[0].type == "ImageStoreBackupStorage" | ||
| } | ||
|
|
||
| /** | ||
| * Verifies the fix: filter first, then sort. | ||
| * Non-preferred types should be excluded, preferred type selected. | ||
| */ | ||
| void testFilterThenSortSelectsPreferredType() { | ||
| def vcenterBs = makeBackupStorageVO("vcenter-uuid", "VCenterBackupStorage") | ||
| def imagestoreBs = makeBackupStorageVO("imagestore-uuid", "ImageStoreBackupStorage") | ||
|
|
||
| def candidates = [vcenterBs, imagestoreBs] | ||
| def preferBsTypes = ["ImageStoreBackupStorage"] | ||
|
|
||
| def result = ExternalPrimaryStorage.filterAndSortByPreferredTypes(candidates, preferBsTypes) | ||
|
|
||
| assert !result.isEmpty() : "Should have matching backup storages after filtering" | ||
| assert result.size() == 1 | ||
| assert result[0].type == "ImageStoreBackupStorage" | ||
| } | ||
|
|
||
| /** | ||
| * Multiple preferred types: verifies ordering matches preferBsTypes list order. | ||
| */ | ||
| void testPreferenceOrderingAmongMultipleTypes() { | ||
| def vcenterBs = makeBackupStorageVO("vcenter-uuid", "VCenterBackupStorage") | ||
| def cephBs = makeBackupStorageVO("ceph-uuid", "CephBackupStorage") | ||
| def imagestoreBs = makeBackupStorageVO("imagestore-uuid", "ImageStoreBackupStorage") | ||
|
|
||
| def candidates = [vcenterBs, cephBs, imagestoreBs] | ||
| def preferBsTypes = ["ImageStoreBackupStorage", "CephBackupStorage"] | ||
|
|
||
| def result = ExternalPrimaryStorage.filterAndSortByPreferredTypes(candidates, preferBsTypes) | ||
|
|
||
| assert result.size() == 2 : "VCenterBackupStorage should be filtered out" | ||
| assert result[0].type == "ImageStoreBackupStorage" : "First preference should be first" | ||
| assert result[1].type == "CephBackupStorage" : "Second preference should be second" | ||
| } | ||
|
|
||
| /** | ||
| * No matching types: result should be empty (caller returns error). | ||
| */ | ||
| void testEmptyResultWhenNoMatchingTypes() { | ||
| def vcenterBs = makeBackupStorageVO("vcenter-uuid", "VCenterBackupStorage") | ||
|
|
||
| def candidates = [vcenterBs] | ||
| def preferBsTypes = ["ImageStoreBackupStorage", "CephBackupStorage"] | ||
|
|
||
| def result = ExternalPrimaryStorage.filterAndSortByPreferredTypes(candidates, preferBsTypes) | ||
|
|
||
| assert result.isEmpty() : "Should be empty when no backup storage matches preferred types" | ||
| } | ||
|
|
||
| /** | ||
| * All candidates are preferred: all should be retained and sorted by preference. | ||
| */ | ||
| void testAllTypesArePreferred() { | ||
| def cephBs = makeBackupStorageVO("ceph-uuid", "CephBackupStorage") | ||
| def imagestoreBs = makeBackupStorageVO("imagestore-uuid", "ImageStoreBackupStorage") | ||
|
|
||
| def candidates = [cephBs, imagestoreBs] | ||
| def preferBsTypes = ["ImageStoreBackupStorage", "CephBackupStorage"] | ||
|
|
||
| def result = ExternalPrimaryStorage.filterAndSortByPreferredTypes(candidates, preferBsTypes) | ||
|
|
||
| assert result.size() == 2 | ||
| assert result[0].type == "ImageStoreBackupStorage" | ||
| assert result[1].type == "CephBackupStorage" | ||
| } | ||
|
|
||
| /** | ||
| * Single preferred type among many candidates: only matching ones retained. | ||
| */ | ||
| void testSinglePreferredTypeAmongMany() { | ||
| def vcenter1 = makeBackupStorageVO("vcenter-uuid-1", "VCenterBackupStorage") | ||
| def vcenter2 = makeBackupStorageVO("vcenter-uuid-2", "VCenterBackupStorage") | ||
| def imagestoreBs = makeBackupStorageVO("imagestore-uuid", "ImageStoreBackupStorage") | ||
| def cephBs = makeBackupStorageVO("ceph-uuid", "CephBackupStorage") | ||
|
|
||
| def candidates = [vcenter1, cephBs, vcenter2, imagestoreBs] | ||
| def preferBsTypes = ["ImageStoreBackupStorage"] | ||
|
|
||
| def result = ExternalPrimaryStorage.filterAndSortByPreferredTypes(candidates, preferBsTypes) | ||
|
|
||
| assert result.size() == 1 | ||
| assert result[0].uuid == "imagestore-uuid" | ||
| assert result[0].type == "ImageStoreBackupStorage" | ||
| } | ||
|
|
||
| private static BackupStorageVO makeBackupStorageVO(String uuid, String type) { | ||
| def vo = new BackupStorageVO() | ||
| vo.setUuid(uuid) | ||
| vo.setType(type) | ||
| return vo | ||
| } | ||
| } |
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: 3335
将
filterAndSortByPreferredTypes改为public，或将测试类移到同包。方法
filterAndSortByPreferredTypes目前是包级可见（无访问修饰符），但测试类在不同包org.zstack.test.integration.storage.primary.addon中调用它。根据 Java 可见性规则，包级可见方法不应被不同包中的类访问。虽然 Groovy 的运行时反射机制可能允许此类访问，但这违反了 Java 封装原则，且易导致维护问题。建议：
public static（添加注释说明仅供测试和工具方法使用），或org.zstack.storage.addon.primary.test或同包位置。🤖 Prompt for AI Agents