Copilot commented on code in PR #14389:
URL: https://github.com/apache/cloudstack/pull/14389#discussion_r4247217048
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java:
##########
@@ -1367,6 +1377,120 @@ public Pair<Long, Long> getVolumeStats(StoragePool
storagePool, String volumeId)
return null;
}
+ /**
+ * Lists StorPool volumes available for import (globalId name, no "cs" tag
unless
+ * {@code forUnmanage}). When {@code volumePath} is given, looks up that
one volume directly
+ * instead of browsing the whole pool; when {@code forUnmanage} is also
true, an already
+ * "cs"-tagged volume is still returned, since the caller is
unmanageVolume() looking for the
+ * very volume it is about to drop from CloudStack.
+ */
+ @Override
+ public List<VolumeOnStorageTO> listVolumesForImport(StoragePool pool,
String volumePath, String keyword, boolean forUnmanage) {
+
StorPoolUtil.spLog("StorPoolPrimaryDataStoreDriver.listVolumesForImport:
pool=%s, volumePath=%s, keyword=%s, forUnmanage=%s", pool.getName(),
volumePath, keyword, forUnmanage);
+ SpConnectionDesc conn = StorPoolUtil.getSpConnection(pool.getUuid(),
pool.getId(), storagePoolDetailsDao, primaryStoreDao);
+ List<VolumeOnStorageTO> volumes = new ArrayList<>();
+
+ if (StringUtils.isNotBlank(volumePath)) {
+ return getVolumeOnStorageByPath(pool, volumePath, conn, volumes,
forUnmanage);
+ }
+
+ SpApiResponse resp = StorPoolUtil.volumesListResponse(conn);
+ if (resp.getError() != null) {
+
StorPoolUtil.spLog("StorPoolPrimaryDataStoreDriver.listVolumesForImport: could
not list volumes for pool=%s. Error: %s", pool.getName(), resp.getError());
+ return volumes;
+ }
+ return getVolumeOnStorage(pool, keyword, resp, volumes);
+ }
+
+ @NotNull
+ private List<VolumeOnStorageTO> getVolumeOnStorage(StoragePool pool,
String keyword, SpApiResponse resp, List<VolumeOnStorageTO> volumes) {
+ JsonArray volumesList =
resp.fullJson.getAsJsonObject().getAsJsonArray("data");
+
+ for (int i = 0; i < volumesList.size(); i++) {
+ JsonObject volume = volumesList.get(i).getAsJsonObject();
+ String rawName = volume.get("name").getAsString();
+ if (isVolumeDeleted(volume) || !isEligibleForBulkImport(volume,
rawName)) {
+ continue;
+ }
+
+ String name = rawName.split("~")[1];
+
+ if (StringUtils.isNotBlank(keyword) && !name.contains(keyword)) {
+ continue;
+ }
+ volumes.add(toVolumeOnStorageTO(volume, name));
+ }
+
StorPoolUtil.spLog("StorPoolPrimaryDataStoreDriver.listVolumesForImport: found
%s volume(s) for pool=%s", volumes.size(), pool.getName());
+ return volumes;
+ }
+
+ @NotNull
+ private List<VolumeOnStorageTO> getVolumeOnStorageByPath(StoragePool pool,
String volumePath, SpConnectionDesc conn, List<VolumeOnStorageTO> volumes,
boolean forUnmanage) {
+ String name = StorPoolStorageAdaptor.getVolumeNameFromPath(volumePath,
true);
+ SpApiResponse resp = StorPoolUtil.volumeDescribeResponse(name, conn);
+ if (resp.getError() != null) {
+
StorPoolUtil.spLog("StorPoolPrimaryDataStoreDriver.listVolumesForImport: could
not describe volume=%s on pool=%s. Error: %s", name, pool.getName(),
resp.getError());
+ return volumes;
+ }
+ JsonArray data =
resp.fullJson.getAsJsonObject().getAsJsonArray("data");
+ if (data == null || data.isEmpty()) {
+ return volumes;
+ }
+ JsonObject volume = data.get(0).getAsJsonObject();
+ String rawName = volume.get("name").getAsString();
+ if (isVolumeDeleted(volume) || !isGlobalIdVolumeName(rawName) ||
(!forUnmanage && isTaggedCs(volume))) {
+ return volumes;
+ }
+ volumes.add(toVolumeOnStorageTO(volume, rawName.split("~")[1]));
+ return volumes;
+ }
+
+ private static boolean isVolumeDeleted(JsonObject volume) {
+ return volume.has("deleted") && volume.get("deleted").getAsBoolean();
+ }
+
+ private static boolean isGlobalIdVolumeName(String rawName) {
+ return !rawName.startsWith("*") && !rawName.contains("@") &&
rawName.startsWith("~");
+ }
+
+ private static boolean isTaggedCs(JsonObject volume) {
+ JsonObject tags = volume.has("tags") &&
!volume.get("tags").isJsonNull() ? volume.getAsJsonObject("tags") : null;
+ return tags != null && tags.has("cs");
+ }
+
+ private static boolean isEligibleForBulkImport(JsonObject volume, String
rawName) {
+ return isGlobalIdVolumeName(rawName) && !isTaggedCs(volume);
+ }
Review Comment:
`isTaggedCs` treats any presence of the `cs` key as 'managed'. However this
PR’s unmanage path sets `cs` to an empty string (see
`volumeManagementChanged(..., imported=false)`), which would still be
considered managed here and would prevent unmanaged volumes from reappearing in
`listVolumesForImport`. Update the check to treat blank/empty `cs` values as
*not managed* (e.g., require a non-blank value, or explicitly require `cs ==
\"volume\"`).
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckVolumeCommandWrapper.java:
##########
@@ -51,7 +51,15 @@ public final class LibvirtCheckVolumeCommandWrapper extends
CommandWrapper<Check
Storage.StoragePoolType.Filesystem,
Storage.StoragePoolType.NetworkFilesystem,
Storage.StoragePoolType.SharedMountPoint,
- Storage.StoragePoolType.RBD);
+ Storage.StoragePoolType.RBD,
+ Storage.StoragePoolType.StorPool);
+
+ private static final List<Storage.StoragePoolType>
RAW_BLOCK_STORAGE_POOL_TYPES = Arrays.asList(
+ Storage.StoragePoolType.RBD,
+ Storage.StoragePoolType.StorPool);
Review Comment:
StorPool volumes are treated as `RAW_BLOCK_STORAGE_POOL_TYPES`, which skips
QCOW2 validation and uses `vol.getVirtualSize()` instead of reading the virtual
size from the volume path. This is inconsistent with the rest of the PR which
treats StorPool imports as QCOW2 (and the volume path appears usable, e.g.,
`/dev/storpool-byid/...`). If StorPool volumes are QCOW2, this logic can cause
incorrect backing-file detection/format checks and potentially incorrect size
reporting. Consider removing `StorPool` from `RAW_BLOCK_STORAGE_POOL_TYPES` and
only applying the RBD-specific behavior to RBD.
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckVolumeCommandWrapper.java:
##########
@@ -63,27 +71,37 @@ public Answer execute(final CheckVolumeCommand command,
final LibvirtComputingRe
try {
if
(STORAGE_POOL_TYPES_SUPPORTED.contains(storageFilerTO.getType())) {
- final KVMPhysicalDisk vol = pool.getPhysicalDisk(srcFile);
- final String path = vol.getPath();
- final boolean isRbd =
Storage.StoragePoolType.RBD.equals(storageFilerTO.getType());
-
- Map<VolumeOnStorageTO.Detail, String> volumeDetails =
getVolumeDetails(pool, vol);
- if (MapUtils.isEmpty(volumeDetails)) {
- return new Answer(command, false, "Unable to read the
volume on the storage pool");
+ final boolean shouldAttachTheVolume =
POOL_NEEDS_TO_EXPOSE_THE_VOLUME.contains(storageFilerTO.getType());
+ if (shouldAttachTheVolume &&
!pool.connectPhysicalDisk(srcFile, new HashMap<>())) {
+ return new Answer(command, false, "Unable to attach the
volume on the storage pool");
}
+ try {
+ final KVMPhysicalDisk vol = pool.getPhysicalDisk(srcFile);
+ final String path = vol.getPath();
+ final boolean isRawBlockPool =
RAW_BLOCK_STORAGE_POOL_TYPES.contains(storageFilerTO.getType());
+
+ Map<VolumeOnStorageTO.Detail, String> volumeDetails =
getVolumeDetails(pool, vol);
+ if (MapUtils.isEmpty(volumeDetails)) {
+ return new Answer(command, false, "Unable to read the
volume on the storage pool");
+ }
- if (!isRbd) {
- try {
- KVMPhysicalDisk.checkQcow2File(path);
- } catch (final CloudRuntimeException e) {
- return new CheckVolumeAnswer(command, false, "", 0,
volumeDetails);
+ if (!isRawBlockPool) {
+ try {
+ KVMPhysicalDisk.checkQcow2File(path);
+ } catch (final CloudRuntimeException e) {
+ return new CheckVolumeAnswer(command, false, "",
0, volumeDetails);
+ }
}
- }
- // Images on RBD are raw and the path is an image name that
qemu-img cannot open
- // without the rbd: URI, so take the size libvirt already
reported for the volume.
- long size = isRbd ? vol.getVirtualSize() :
KVMPhysicalDisk.getVirtualSizeFromFile(path);
- return new CheckVolumeAnswer(command, true, "", size,
volumeDetails);
+ // Images on RBD are raw and the path is an image name
that qemu-img cannot open
+ // without the rbd: URI, so take the size libvirt already
reported for the volume
+ long size = isRawBlockPool ? vol.getVirtualSize() :
KVMPhysicalDisk.getVirtualSizeFromFile(path);
Review Comment:
StorPool volumes are treated as `RAW_BLOCK_STORAGE_POOL_TYPES`, which skips
QCOW2 validation and uses `vol.getVirtualSize()` instead of reading the virtual
size from the volume path. This is inconsistent with the rest of the PR which
treats StorPool imports as QCOW2 (and the volume path appears usable, e.g.,
`/dev/storpool-byid/...`). If StorPool volumes are QCOW2, this logic can cause
incorrect backing-file detection/format checks and potentially incorrect size
reporting. Consider removing `StorPool` from `RAW_BLOCK_STORAGE_POOL_TYPES` and
only applying the RBD-specific behavior to RBD.
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java:
##########
@@ -1367,6 +1377,120 @@ public Pair<Long, Long> getVolumeStats(StoragePool
storagePool, String volumeId)
return null;
}
+ /**
+ * Lists StorPool volumes available for import (globalId name, no "cs" tag
unless
+ * {@code forUnmanage}). When {@code volumePath} is given, looks up that
one volume directly
+ * instead of browsing the whole pool; when {@code forUnmanage} is also
true, an already
+ * "cs"-tagged volume is still returned, since the caller is
unmanageVolume() looking for the
+ * very volume it is about to drop from CloudStack.
+ */
+ @Override
+ public List<VolumeOnStorageTO> listVolumesForImport(StoragePool pool,
String volumePath, String keyword, boolean forUnmanage) {
+
StorPoolUtil.spLog("StorPoolPrimaryDataStoreDriver.listVolumesForImport:
pool=%s, volumePath=%s, keyword=%s, forUnmanage=%s", pool.getName(),
volumePath, keyword, forUnmanage);
+ SpConnectionDesc conn = StorPoolUtil.getSpConnection(pool.getUuid(),
pool.getId(), storagePoolDetailsDao, primaryStoreDao);
+ List<VolumeOnStorageTO> volumes = new ArrayList<>();
+
+ if (StringUtils.isNotBlank(volumePath)) {
+ return getVolumeOnStorageByPath(pool, volumePath, conn, volumes,
forUnmanage);
+ }
+
+ SpApiResponse resp = StorPoolUtil.volumesListResponse(conn);
+ if (resp.getError() != null) {
+
StorPoolUtil.spLog("StorPoolPrimaryDataStoreDriver.listVolumesForImport: could
not list volumes for pool=%s. Error: %s", pool.getName(), resp.getError());
+ return volumes;
+ }
+ return getVolumeOnStorage(pool, keyword, resp, volumes);
+ }
+
+ @NotNull
+ private List<VolumeOnStorageTO> getVolumeOnStorage(StoragePool pool,
String keyword, SpApiResponse resp, List<VolumeOnStorageTO> volumes) {
+ JsonArray volumesList =
resp.fullJson.getAsJsonObject().getAsJsonArray("data");
+
+ for (int i = 0; i < volumesList.size(); i++) {
+ JsonObject volume = volumesList.get(i).getAsJsonObject();
+ String rawName = volume.get("name").getAsString();
+ if (isVolumeDeleted(volume) || !isEligibleForBulkImport(volume,
rawName)) {
+ continue;
+ }
+
+ String name = rawName.split("~")[1];
Review Comment:
This uses `split(\"~\")` just to drop the first character. Since `split`
allocates an array and uses regex semantics, it’s heavier and less explicit
than needed. Prefer a simple substring (e.g., `rawName.substring(1)`) after the
`isGlobalIdVolumeName` check to make the assumption clear and avoid unnecessary
allocations.
##########
server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java:
##########
@@ -828,9 +833,16 @@ private Pair<DiskProfile, StoragePool>
importKVMSharedDisk(VirtualMachine vm, Di
DiskProfile profile = volumeManager.updateImportedVolume(type,
diskOffering, vm, template, deviceId,
poolId, storagePool.getPoolType(), diskPath, null,
diskProfile, format);
+ notifyStorageDriverOfVolumeImport(storagePool, profile.getVolumeId());
+
return new Pair<>(profile, storagePool);
}
+ private void notifyStorageDriverOfVolumeImport(StoragePool pool, long
volumeId) {
+ Volume volume = volumeDao.findById(volumeId);
Review Comment:
`volumeDao.findById(volumeId)` can return null (e.g., if the DB write
failed/was rolled back or the ID is invalid in an unexpected path), which would
lead to a `NullPointerException` when notifying the service. Add a null-guard
(and ideally log at least a warning) before calling
`notifyStorageDriverOfVolumeManagementChange`.
##########
plugins/storage/volume/storpool/src/test/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriverTest.java:
##########
@@ -214,6 +236,345 @@ public void testCopyVolumeNotAttachedToVmPassed() {
Assert.assertNull(response.getError());
}
+ @Test
+ public void testListVolumesForImportReturnsGlobalIdVolume() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ StorPoolUtil.SpApiResponse resp =
volumesListResponse(volumeJson("~t.t.t", null, false, 1073741824L, 524288000L));
+ when(StorPoolUtil.volumesListResponse(conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool, null, null,
false);
+
+ Assert.assertEquals(1, volumes.size());
+ VolumeOnStorageTO volume = volumes.get(0);
+ Assert.assertEquals("t.t.t", volume.getName());
+ Assert.assertEquals("/dev/storpool-byid/t.t.t", volume.getPath());
+ Assert.assertEquals("/dev/storpool-byid/t.t.t", volume.getFullPath());
+ Assert.assertEquals(Storage.ImageFormat.QCOW2.toString(),
volume.getFormat());
+ Assert.assertEquals(524288000L, volume.getSize());
+ Assert.assertEquals(1073741824L, volume.getVirtualSize());
+ }
+
+ @Test
+ public void testListVolumesForImportSkipsVolumeTaggedCs() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ JsonObject tagged = volumeJsonObject("~t.t.t", 1073741824L,
1073741824L);
+ JsonObject tags = new JsonObject();
+ tags.addProperty("cs", "volume");
+ tagged.add("tags", tags);
+ StorPoolUtil.SpApiResponse resp = volumesListResponse(tagged);
+ when(StorPoolUtil.volumesListResponse(conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool, null, null,
false);
+
+ Assert.assertTrue(volumes.isEmpty());
+ }
+
+ @Test
+ public void testListVolumesForImportSkipsDeletedVolume() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ StorPoolUtil.SpApiResponse resp =
volumesListResponse(volumeJson("~t.t.t", null, true, 1073741824L, 1073741824L));
+ when(StorPoolUtil.volumesListResponse(conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool, null, null,
false);
+
+ Assert.assertTrue(volumes.isEmpty());
+ }
+
+ @Test
+ public void testListVolumesForImportSkipsNonGlobalIdAndCloneNames() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ JsonArray data = new JsonArray();
+ data.add(volumeJsonObject("friendlyname", 1073741824L, 1073741824L));
+ data.add(volumeJsonObject("*s.s.s", 1073741824L, 1073741824L));
+ data.add(volumeJsonObject("[email protected]", 1073741824L, 1073741824L));
+ StorPoolUtil.SpApiResponse resp = volumesListResponse(data);
+ when(StorPoolUtil.volumesListResponse(conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool, null, null,
false);
+
+ Assert.assertTrue(volumes.isEmpty());
+ }
+
+ @Test
+ public void testListVolumesForImportFiltersByKeyword() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ JsonArray data = new JsonArray();
+ data.add(volumeJsonObject("~t.t.t", 1073741824L, 1073741824L));
+ data.add(volumeJsonObject("~s.s.s", 1073741824L, 1073741824L));
+ StorPoolUtil.SpApiResponse resp = volumesListResponse(data);
+ when(StorPoolUtil.volumesListResponse(conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool, null, "s.s.s",
false);
+
+ Assert.assertEquals(1, volumes.size());
+ Assert.assertEquals("s.s.s", volumes.get(0).getName());
+ }
+
+ @Test
+ public void testListVolumesForImportApiErrorReturnsEmptyList() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ StorPoolUtil.SpApiResponse resp = new StorPoolUtil.SpApiResponse();
+ setResponseError(resp);
+ when(StorPoolUtil.volumesListResponse(conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool, null, null,
false);
+
+ Assert.assertNotNull(volumes);
+ Assert.assertTrue(volumes.isEmpty());
+ }
+
+ @Test
+ public void testListVolumesForImportWithPathReturnsVolume() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ StorPoolUtil.SpApiResponse resp =
volumesListResponse(volumeJson("~t.t.t", null, false, 1073741824L, 524288000L));
+ when(StorPoolUtil.volumeDescribeResponse("~t.t.t",
conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool,
"/dev/storpool-byid/t.t.t", null, false);
+
+ Assert.assertEquals(1, volumes.size());
+ VolumeOnStorageTO volume = volumes.get(0);
+ Assert.assertEquals("t.t.t", volume.getName());
+ Assert.assertEquals("/dev/storpool-byid/t.t.t", volume.getPath());
+ Assert.assertEquals("/dev/storpool-byid/t.t.t", volume.getFullPath());
+ Assert.assertEquals(Storage.ImageFormat.QCOW2.toString(),
volume.getFormat());
+ Assert.assertEquals(524288000L, volume.getSize());
+ Assert.assertEquals(1073741824L, volume.getVirtualSize());
+ }
+
+ @Test
+ public void testListVolumesForImportWithPathExcludesVolumeTaggedCs() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ JsonObject tagged = volumeJsonObject("~t.t.t", 1073741824L,
1073741824L);
+ JsonObject tags = new JsonObject();
+ tags.addProperty("cs", "volume");
+ tagged.add("tags", tags);
+ StorPoolUtil.SpApiResponse resp = volumesListResponse(tagged);
+ when(StorPoolUtil.volumeDescribeResponse("~t.t.t",
conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool,
"/dev/storpool-byid/t.t.t", null, false);
+
+ Assert.assertTrue(volumes.isEmpty());
+ }
+
+ @Test
+ public void
testListVolumesForImportWithPathForUnmanageReturnsVolumeEvenIfTaggedCs() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ JsonObject tagged = volumeJsonObject("~t.t.t", 1073741824L,
1073741824L);
+ JsonObject tags = new JsonObject();
+ tags.addProperty("cs", "volume");
+ tagged.add("tags", tags);
+ StorPoolUtil.SpApiResponse resp = volumesListResponse(tagged);
+ when(StorPoolUtil.volumeDescribeResponse("~t.t.t",
conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool,
"/dev/storpool-byid/t.t.t", null, true);
+
+ Assert.assertEquals(1, volumes.size());
+ Assert.assertEquals("t.t.t", volumes.get(0).getName());
+ }
+
+ @Test
+ public void
testListVolumesForImportWithPathForUnmanageSkipsNonGlobalIdName() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ StorPoolUtil.SpApiResponse resp =
volumesListResponse(volumeJsonObject("[email protected]", 1073741824L, 1073741824L));
+ when(StorPoolUtil.volumeDescribeResponse("~t.t.t",
conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool,
"/dev/storpool-byid/t.t.t", null, true);
+
+ Assert.assertTrue(volumes.isEmpty());
+ }
+
+ @Test
+ public void testListVolumesForImportWithPathSkipsDeletedVolume() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ StorPoolUtil.SpApiResponse resp =
volumesListResponse(volumeJson("~t.t.t", null, true, 1073741824L, 1073741824L));
+ when(StorPoolUtil.volumeDescribeResponse("~t.t.t",
conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool,
"/dev/storpool-byid/t.t.t", null, false);
+
+ Assert.assertTrue(volumes.isEmpty());
+ }
+
+ @Test
+ public void testListVolumesForImportWithPathNotFoundReturnsEmptyList() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ StorPoolUtil.SpApiResponse resp = volumesListResponse(new JsonArray());
+ when(StorPoolUtil.volumeDescribeResponse("~t.t.t",
conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool,
"/dev/storpool-byid/t.t.t", null, false);
+
+ Assert.assertNotNull(volumes);
+ Assert.assertTrue(volumes.isEmpty());
+ }
+
+ @Test
+ public void testListVolumesForImportWithPathApiErrorReturnsEmptyList() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ StorPoolUtil.SpApiResponse resp = new StorPoolUtil.SpApiResponse();
+ setResponseError(resp);
+ when(StorPoolUtil.volumeDescribeResponse("~t.t.t",
conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool,
"/dev/storpool-byid/t.t.t", null, false);
+
+ Assert.assertNotNull(volumes);
+ Assert.assertTrue(volumes.isEmpty());
+ }
+
+ @Test
+ public void testCreateAsyncSetsVolumeFormatToQcow2() {
+ DataStore dataStore = mock(DataStore.class);
+
when(dataStore.getUuid()).thenReturn("SP_API_HTTP=1.1.1.1:81;SP_AUTH_TOKEN=token;SP_TEMPLATE=template_name");
+ when(dataStore.getId()).thenReturn(1L);
+
+ VolumeInfo vinfo = mock(VolumeInfo.class);
+ when(vinfo.getType()).thenReturn(DataObjectType.VOLUME);
+ when(vinfo.getUuid()).thenReturn(UUID.randomUUID().toString());
+ when(vinfo.getId()).thenReturn(101L);
+ when(vinfo.getSize()).thenReturn(1073741824L);
+ when(vinfo.getPassphraseId()).thenReturn(null);
+ when(vinfo.getInstanceId()).thenReturn(null);
+ when(vinfo.getDiskOfferingId()).thenReturn(null);
+
+ when(StorPoolUtil.getSpConnection(dataStore.getUuid(),
dataStore.getId(), detailsDao, storagePool)).thenReturn(conn);
+ StorPoolUtil.SpApiResponse resp = new StorPoolUtil.SpApiResponse();
+ when(StorPoolUtil.volumeCreate(any(StorPoolVolumeDef.class),
eq(conn))).thenReturn(resp);
+ when(StorPoolUtil.getNameFromResponse(resp,
false)).thenReturn("t.t.t");
+
+ VolumeVO volumeVO = mock(VolumeVO.class);
+ when(volumeVO.getId()).thenReturn(101L);
+ when(volumeDao.findById(101L)).thenReturn(volumeVO);
+
+ StoragePoolVO storagePoolVO = mock(StoragePoolVO.class);
+ when(storagePool.findById(1L)).thenReturn(storagePoolVO);
+
+ @SuppressWarnings("unchecked")
+ AsyncCompletionCallback<CreateCmdResult> createCallback =
mock(AsyncCompletionCallback.class);
+
+ storPoolPrimaryDataStoreDriver.createAsync(dataStore, vinfo,
createCallback);
+
+ verify(volumeVO).setFormat(Storage.ImageFormat.QCOW2);
+ verify(volumeDao).update(eq(101L), eq(volumeVO));
+ }
+
+ @Test
+ public void testVolumeImportedTagsVolumeWithCsAndUuid() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ String volumeUuid = UUID.randomUUID().toString();
+ VolumeVO volume = mock(VolumeVO.class);
+ when(volume.getPath()).thenReturn("/dev/storpool-byid/t.t.t");
+ when(volume.getUuid()).thenReturn(volumeUuid);
+ when(volume.getDiskOfferingId()).thenReturn(5L);
+ when(volume.getInstanceId()).thenReturn(null);
+ when(diskOfferingDetailsDao.findDetail(5L,
StorPoolUtil.SP_TIER)).thenReturn(null);
+ StorPoolUtil.SpApiResponse resp = new StorPoolUtil.SpApiResponse();
+ when(StorPoolUtil.volumeUpdate(any(StorPoolVolumeDef.class),
eq(conn))).thenReturn(resp);
+
+ storPoolPrimaryDataStoreDriver.volumeManagementChanged(importPool,
volume, true);
+
+ ArgumentCaptor<StorPoolVolumeDef> captor =
ArgumentCaptor.forClass(StorPoolVolumeDef.class);
+ utilities.verify(() -> StorPoolUtil.volumeUpdate(captor.capture(),
eq(conn)), times(1));
+ StorPoolVolumeDef updated = captor.getValue();
+ Assert.assertEquals("~t.t.t", updated.getName());
+ Assert.assertEquals(volumeUuid, updated.getTags().get("uuid"));
+ Assert.assertEquals("volume", updated.getTags().get("cs"));
+ Assert.assertNull(updated.getTags().get("qc"));
+ Assert.assertNull(updated.getTags().get("cvm"));
+ }
+
+ @Test
+ public void testVolumeImportedSetsCvmTagWhenAttachedToVm() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ VolumeVO volume = mock(VolumeVO.class);
+ when(volume.getPath()).thenReturn("/dev/storpool-byid/t.t.t");
+ when(volume.getUuid()).thenReturn(UUID.randomUUID().toString());
+ when(volume.getInstanceId()).thenReturn(7L);
+ VMInstanceVO vm = mock(VMInstanceVO.class);
+ String vmUuid = UUID.randomUUID().toString();
+ when(vm.getUuid()).thenReturn(vmUuid);
+ when(vmInstanceDao.findById(7L)).thenReturn(vm);
+ StorPoolUtil.SpApiResponse resp = new StorPoolUtil.SpApiResponse();
+ when(StorPoolUtil.volumeUpdate(any(StorPoolVolumeDef.class),
eq(conn))).thenReturn(resp);
+
+ storPoolPrimaryDataStoreDriver.volumeManagementChanged(importPool,
volume, true);
+
+ ArgumentCaptor<StorPoolVolumeDef> captor =
ArgumentCaptor.forClass(StorPoolVolumeDef.class);
+ utilities.verify(() -> StorPoolUtil.volumeUpdate(captor.capture(),
eq(conn)), times(1));
+ Assert.assertEquals(vmUuid, captor.getValue().getTags().get("cvm"));
+ }
+
+ @Test
+ public void testVolumeImportedIncludesQosTierFromDiskOffering() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ VolumeVO volume = mock(VolumeVO.class);
+ when(volume.getPath()).thenReturn("/dev/storpool-byid/t.t.t");
+ when(volume.getUuid()).thenReturn(UUID.randomUUID().toString());
+ when(volume.getDiskOfferingId()).thenReturn(5L);
+ when(volume.getInstanceId()).thenReturn(null);
+ DiskOfferingDetailVO tierDetail = mock(DiskOfferingDetailVO.class);
+ when(tierDetail.getValue()).thenReturn("ssd-tier");
+ when(diskOfferingDetailsDao.findDetail(5L,
StorPoolUtil.SP_TIER)).thenReturn(tierDetail);
+ StorPoolUtil.SpApiResponse resp = new StorPoolUtil.SpApiResponse();
+ when(StorPoolUtil.volumeUpdate(any(StorPoolVolumeDef.class),
eq(conn))).thenReturn(resp);
+
+ storPoolPrimaryDataStoreDriver.volumeManagementChanged(importPool,
volume, true);
+
+ ArgumentCaptor<StorPoolVolumeDef> captor =
ArgumentCaptor.forClass(StorPoolVolumeDef.class);
+ utilities.verify(() -> StorPoolUtil.volumeUpdate(captor.capture(),
eq(conn)), times(1));
+ Assert.assertEquals("ssd-tier", captor.getValue().getTags().get("qc"));
+ }
+
+ @Test
+ public void testVolumeUnmanagedClearsCsUuidAndQosTags() {
Review Comment:
Given the unmanage path leaves `cs` present but empty (`cs == \"\"`), add a
unit test to ensure `listVolumesForImport(..., forUnmanage=false)` treats an
empty `cs` value as *unmanaged* and lists the volume again. This directly
guards the behavior around the new `isTaggedCs`/eligibility logic.
##########
server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java:
##########
@@ -2959,6 +2971,10 @@ private UserVm importKvmVirtualMachineFromDisk(final
ImportSource importSource,
// check then runs against whichever pool that cluster does have.
Both are null for a zone
// wide pool, which every host can see.
StoragePoolVO importStoragePool =
primaryDataStoreDao.findById(poolId);
+ if (importStoragePool == null) {
+ cleanupFailedImportVM(userVm);
+ throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR,
String.format("Import failed for Vm: %s. Storage pool with id: %s not found",
userVm.getInstanceName(), poolId));
Review Comment:
The new error message has inconsistent capitalization (`Vm`) and is slightly
awkward/less searchable. Consider changing to `VM` and tightening the wording
(e.g., \"Import failed for VM %s: storage pool id %s not found\").
##########
plugins/storage/volume/storpool/src/test/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriverTest.java:
##########
@@ -214,6 +236,345 @@ public void testCopyVolumeNotAttachedToVmPassed() {
Assert.assertNull(response.getError());
}
+ @Test
+ public void testListVolumesForImportReturnsGlobalIdVolume() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ StorPoolUtil.SpApiResponse resp =
volumesListResponse(volumeJson("~t.t.t", null, false, 1073741824L, 524288000L));
+ when(StorPoolUtil.volumesListResponse(conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool, null, null,
false);
+
+ Assert.assertEquals(1, volumes.size());
+ VolumeOnStorageTO volume = volumes.get(0);
+ Assert.assertEquals("t.t.t", volume.getName());
+ Assert.assertEquals("/dev/storpool-byid/t.t.t", volume.getPath());
+ Assert.assertEquals("/dev/storpool-byid/t.t.t", volume.getFullPath());
+ Assert.assertEquals(Storage.ImageFormat.QCOW2.toString(),
volume.getFormat());
+ Assert.assertEquals(524288000L, volume.getSize());
+ Assert.assertEquals(1073741824L, volume.getVirtualSize());
+ }
+
+ @Test
+ public void testListVolumesForImportSkipsVolumeTaggedCs() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ JsonObject tagged = volumeJsonObject("~t.t.t", 1073741824L,
1073741824L);
+ JsonObject tags = new JsonObject();
+ tags.addProperty("cs", "volume");
+ tagged.add("tags", tags);
+ StorPoolUtil.SpApiResponse resp = volumesListResponse(tagged);
+ when(StorPoolUtil.volumesListResponse(conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool, null, null,
false);
+
+ Assert.assertTrue(volumes.isEmpty());
+ }
+
+ @Test
+ public void testListVolumesForImportSkipsDeletedVolume() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ StorPoolUtil.SpApiResponse resp =
volumesListResponse(volumeJson("~t.t.t", null, true, 1073741824L, 1073741824L));
+ when(StorPoolUtil.volumesListResponse(conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool, null, null,
false);
+
+ Assert.assertTrue(volumes.isEmpty());
+ }
+
+ @Test
+ public void testListVolumesForImportSkipsNonGlobalIdAndCloneNames() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ JsonArray data = new JsonArray();
+ data.add(volumeJsonObject("friendlyname", 1073741824L, 1073741824L));
+ data.add(volumeJsonObject("*s.s.s", 1073741824L, 1073741824L));
+ data.add(volumeJsonObject("[email protected]", 1073741824L, 1073741824L));
+ StorPoolUtil.SpApiResponse resp = volumesListResponse(data);
+ when(StorPoolUtil.volumesListResponse(conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool, null, null,
false);
+
+ Assert.assertTrue(volumes.isEmpty());
+ }
+
+ @Test
+ public void testListVolumesForImportFiltersByKeyword() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ JsonArray data = new JsonArray();
+ data.add(volumeJsonObject("~t.t.t", 1073741824L, 1073741824L));
+ data.add(volumeJsonObject("~s.s.s", 1073741824L, 1073741824L));
+ StorPoolUtil.SpApiResponse resp = volumesListResponse(data);
+ when(StorPoolUtil.volumesListResponse(conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool, null, "s.s.s",
false);
+
+ Assert.assertEquals(1, volumes.size());
+ Assert.assertEquals("s.s.s", volumes.get(0).getName());
+ }
+
+ @Test
+ public void testListVolumesForImportApiErrorReturnsEmptyList() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ StorPoolUtil.SpApiResponse resp = new StorPoolUtil.SpApiResponse();
+ setResponseError(resp);
+ when(StorPoolUtil.volumesListResponse(conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool, null, null,
false);
+
+ Assert.assertNotNull(volumes);
+ Assert.assertTrue(volumes.isEmpty());
+ }
+
+ @Test
+ public void testListVolumesForImportWithPathReturnsVolume() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ StorPoolUtil.SpApiResponse resp =
volumesListResponse(volumeJson("~t.t.t", null, false, 1073741824L, 524288000L));
+ when(StorPoolUtil.volumeDescribeResponse("~t.t.t",
conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool,
"/dev/storpool-byid/t.t.t", null, false);
+
+ Assert.assertEquals(1, volumes.size());
+ VolumeOnStorageTO volume = volumes.get(0);
+ Assert.assertEquals("t.t.t", volume.getName());
+ Assert.assertEquals("/dev/storpool-byid/t.t.t", volume.getPath());
+ Assert.assertEquals("/dev/storpool-byid/t.t.t", volume.getFullPath());
+ Assert.assertEquals(Storage.ImageFormat.QCOW2.toString(),
volume.getFormat());
+ Assert.assertEquals(524288000L, volume.getSize());
+ Assert.assertEquals(1073741824L, volume.getVirtualSize());
+ }
+
+ @Test
+ public void testListVolumesForImportWithPathExcludesVolumeTaggedCs() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ JsonObject tagged = volumeJsonObject("~t.t.t", 1073741824L,
1073741824L);
+ JsonObject tags = new JsonObject();
+ tags.addProperty("cs", "volume");
+ tagged.add("tags", tags);
+ StorPoolUtil.SpApiResponse resp = volumesListResponse(tagged);
+ when(StorPoolUtil.volumeDescribeResponse("~t.t.t",
conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool,
"/dev/storpool-byid/t.t.t", null, false);
+
+ Assert.assertTrue(volumes.isEmpty());
+ }
+
+ @Test
+ public void
testListVolumesForImportWithPathForUnmanageReturnsVolumeEvenIfTaggedCs() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ JsonObject tagged = volumeJsonObject("~t.t.t", 1073741824L,
1073741824L);
+ JsonObject tags = new JsonObject();
+ tags.addProperty("cs", "volume");
+ tagged.add("tags", tags);
+ StorPoolUtil.SpApiResponse resp = volumesListResponse(tagged);
+ when(StorPoolUtil.volumeDescribeResponse("~t.t.t",
conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool,
"/dev/storpool-byid/t.t.t", null, true);
+
+ Assert.assertEquals(1, volumes.size());
+ Assert.assertEquals("t.t.t", volumes.get(0).getName());
+ }
+
+ @Test
+ public void
testListVolumesForImportWithPathForUnmanageSkipsNonGlobalIdName() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ StorPoolUtil.SpApiResponse resp =
volumesListResponse(volumeJsonObject("[email protected]", 1073741824L, 1073741824L));
+ when(StorPoolUtil.volumeDescribeResponse("~t.t.t",
conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool,
"/dev/storpool-byid/t.t.t", null, true);
+
+ Assert.assertTrue(volumes.isEmpty());
+ }
+
+ @Test
+ public void testListVolumesForImportWithPathSkipsDeletedVolume() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ StorPoolUtil.SpApiResponse resp =
volumesListResponse(volumeJson("~t.t.t", null, true, 1073741824L, 1073741824L));
+ when(StorPoolUtil.volumeDescribeResponse("~t.t.t",
conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool,
"/dev/storpool-byid/t.t.t", null, false);
+
+ Assert.assertTrue(volumes.isEmpty());
+ }
+
+ @Test
+ public void testListVolumesForImportWithPathNotFoundReturnsEmptyList() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ StorPoolUtil.SpApiResponse resp = volumesListResponse(new JsonArray());
+ when(StorPoolUtil.volumeDescribeResponse("~t.t.t",
conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool,
"/dev/storpool-byid/t.t.t", null, false);
+
+ Assert.assertNotNull(volumes);
+ Assert.assertTrue(volumes.isEmpty());
+ }
+
+ @Test
+ public void testListVolumesForImportWithPathApiErrorReturnsEmptyList() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ StorPoolUtil.SpApiResponse resp = new StorPoolUtil.SpApiResponse();
+ setResponseError(resp);
+ when(StorPoolUtil.volumeDescribeResponse("~t.t.t",
conn)).thenReturn(resp);
+
+ List<VolumeOnStorageTO> volumes =
storPoolPrimaryDataStoreDriver.listVolumesForImport(importPool,
"/dev/storpool-byid/t.t.t", null, false);
+
+ Assert.assertNotNull(volumes);
+ Assert.assertTrue(volumes.isEmpty());
+ }
+
+ @Test
+ public void testCreateAsyncSetsVolumeFormatToQcow2() {
+ DataStore dataStore = mock(DataStore.class);
+
when(dataStore.getUuid()).thenReturn("SP_API_HTTP=1.1.1.1:81;SP_AUTH_TOKEN=token;SP_TEMPLATE=template_name");
+ when(dataStore.getId()).thenReturn(1L);
+
+ VolumeInfo vinfo = mock(VolumeInfo.class);
+ when(vinfo.getType()).thenReturn(DataObjectType.VOLUME);
+ when(vinfo.getUuid()).thenReturn(UUID.randomUUID().toString());
+ when(vinfo.getId()).thenReturn(101L);
+ when(vinfo.getSize()).thenReturn(1073741824L);
+ when(vinfo.getPassphraseId()).thenReturn(null);
+ when(vinfo.getInstanceId()).thenReturn(null);
+ when(vinfo.getDiskOfferingId()).thenReturn(null);
+
+ when(StorPoolUtil.getSpConnection(dataStore.getUuid(),
dataStore.getId(), detailsDao, storagePool)).thenReturn(conn);
+ StorPoolUtil.SpApiResponse resp = new StorPoolUtil.SpApiResponse();
+ when(StorPoolUtil.volumeCreate(any(StorPoolVolumeDef.class),
eq(conn))).thenReturn(resp);
+ when(StorPoolUtil.getNameFromResponse(resp,
false)).thenReturn("t.t.t");
+
+ VolumeVO volumeVO = mock(VolumeVO.class);
+ when(volumeVO.getId()).thenReturn(101L);
+ when(volumeDao.findById(101L)).thenReturn(volumeVO);
+
+ StoragePoolVO storagePoolVO = mock(StoragePoolVO.class);
+ when(storagePool.findById(1L)).thenReturn(storagePoolVO);
+
+ @SuppressWarnings("unchecked")
+ AsyncCompletionCallback<CreateCmdResult> createCallback =
mock(AsyncCompletionCallback.class);
+
+ storPoolPrimaryDataStoreDriver.createAsync(dataStore, vinfo,
createCallback);
+
+ verify(volumeVO).setFormat(Storage.ImageFormat.QCOW2);
+ verify(volumeDao).update(eq(101L), eq(volumeVO));
+ }
+
+ @Test
+ public void testVolumeImportedTagsVolumeWithCsAndUuid() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ String volumeUuid = UUID.randomUUID().toString();
+ VolumeVO volume = mock(VolumeVO.class);
+ when(volume.getPath()).thenReturn("/dev/storpool-byid/t.t.t");
+ when(volume.getUuid()).thenReturn(volumeUuid);
+ when(volume.getDiskOfferingId()).thenReturn(5L);
+ when(volume.getInstanceId()).thenReturn(null);
+ when(diskOfferingDetailsDao.findDetail(5L,
StorPoolUtil.SP_TIER)).thenReturn(null);
+ StorPoolUtil.SpApiResponse resp = new StorPoolUtil.SpApiResponse();
+ when(StorPoolUtil.volumeUpdate(any(StorPoolVolumeDef.class),
eq(conn))).thenReturn(resp);
+
+ storPoolPrimaryDataStoreDriver.volumeManagementChanged(importPool,
volume, true);
+
+ ArgumentCaptor<StorPoolVolumeDef> captor =
ArgumentCaptor.forClass(StorPoolVolumeDef.class);
+ utilities.verify(() -> StorPoolUtil.volumeUpdate(captor.capture(),
eq(conn)), times(1));
+ StorPoolVolumeDef updated = captor.getValue();
+ Assert.assertEquals("~t.t.t", updated.getName());
+ Assert.assertEquals(volumeUuid, updated.getTags().get("uuid"));
+ Assert.assertEquals("volume", updated.getTags().get("cs"));
+ Assert.assertNull(updated.getTags().get("qc"));
+ Assert.assertNull(updated.getTags().get("cvm"));
+ }
+
+ @Test
+ public void testVolumeImportedSetsCvmTagWhenAttachedToVm() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ VolumeVO volume = mock(VolumeVO.class);
+ when(volume.getPath()).thenReturn("/dev/storpool-byid/t.t.t");
+ when(volume.getUuid()).thenReturn(UUID.randomUUID().toString());
+ when(volume.getInstanceId()).thenReturn(7L);
+ VMInstanceVO vm = mock(VMInstanceVO.class);
+ String vmUuid = UUID.randomUUID().toString();
+ when(vm.getUuid()).thenReturn(vmUuid);
+ when(vmInstanceDao.findById(7L)).thenReturn(vm);
+ StorPoolUtil.SpApiResponse resp = new StorPoolUtil.SpApiResponse();
+ when(StorPoolUtil.volumeUpdate(any(StorPoolVolumeDef.class),
eq(conn))).thenReturn(resp);
+
+ storPoolPrimaryDataStoreDriver.volumeManagementChanged(importPool,
volume, true);
+
+ ArgumentCaptor<StorPoolVolumeDef> captor =
ArgumentCaptor.forClass(StorPoolVolumeDef.class);
+ utilities.verify(() -> StorPoolUtil.volumeUpdate(captor.capture(),
eq(conn)), times(1));
+ Assert.assertEquals(vmUuid, captor.getValue().getTags().get("cvm"));
+ }
+
+ @Test
+ public void testVolumeImportedIncludesQosTierFromDiskOffering() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ VolumeVO volume = mock(VolumeVO.class);
+ when(volume.getPath()).thenReturn("/dev/storpool-byid/t.t.t");
+ when(volume.getUuid()).thenReturn(UUID.randomUUID().toString());
+ when(volume.getDiskOfferingId()).thenReturn(5L);
+ when(volume.getInstanceId()).thenReturn(null);
+ DiskOfferingDetailVO tierDetail = mock(DiskOfferingDetailVO.class);
+ when(tierDetail.getValue()).thenReturn("ssd-tier");
+ when(diskOfferingDetailsDao.findDetail(5L,
StorPoolUtil.SP_TIER)).thenReturn(tierDetail);
+ StorPoolUtil.SpApiResponse resp = new StorPoolUtil.SpApiResponse();
+ when(StorPoolUtil.volumeUpdate(any(StorPoolVolumeDef.class),
eq(conn))).thenReturn(resp);
+
+ storPoolPrimaryDataStoreDriver.volumeManagementChanged(importPool,
volume, true);
+
+ ArgumentCaptor<StorPoolVolumeDef> captor =
ArgumentCaptor.forClass(StorPoolVolumeDef.class);
+ utilities.verify(() -> StorPoolUtil.volumeUpdate(captor.capture(),
eq(conn)), times(1));
+ Assert.assertEquals("ssd-tier", captor.getValue().getTags().get("qc"));
+ }
+
+ @Test
+ public void testVolumeUnmanagedClearsCsUuidAndQosTags() {
+ when(StorPoolUtil.getSpConnection(importPool.getUuid(),
importPool.getId(), detailsDao, storagePool)).thenReturn(conn);
+ VolumeVO volume = mock(VolumeVO.class);
+ when(volume.getPath()).thenReturn("/dev/storpool-byid/t.t.t");
+ StorPoolUtil.SpApiResponse resp = new StorPoolUtil.SpApiResponse();
+ when(StorPoolUtil.volumeUpdate(any(StorPoolVolumeDef.class),
eq(conn))).thenReturn(resp);
+
+ storPoolPrimaryDataStoreDriver.volumeManagementChanged(importPool,
volume, false);
+
+ ArgumentCaptor<StorPoolVolumeDef> captor =
ArgumentCaptor.forClass(StorPoolVolumeDef.class);
+ utilities.verify(() -> StorPoolUtil.volumeUpdate(captor.capture(),
eq(conn)), times(1));
+ StorPoolVolumeDef updated = captor.getValue();
+ Assert.assertEquals("~t.t.t", updated.getName());
+ Assert.assertEquals("", updated.getTags().get("cs"));
Review Comment:
Given the unmanage path leaves `cs` present but empty (`cs == \"\"`), add a
unit test to ensure `listVolumesForImport(..., forUnmanage=false)` treats an
empty `cs` value as *unmanaged* and lists the volume again. This directly
guards the behavior around the new `isTaggedCs`/eligibility logic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
