Skip to content

Commit ca6e07e

Browse files
sathvikaragisathvikaragi
committed
CSTACKEX-261: resolving comments
1 parent 57f98f2 commit ca6e07e

4 files changed

Lines changed: 57 additions & 62 deletions

File tree

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java‎

Lines changed: 42 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,9 @@
6868
import org.jetbrains.annotations.Nullable;
6969

7070
import com.cloud.agent.api.Answer;
71-
import com.cloud.agent.api.storage.ResizeVolumeCommand;
7271
import com.cloud.agent.api.to.DataObjectType;
7372
import com.cloud.agent.api.to.DataStoreTO;
7473
import com.cloud.agent.api.to.DataTO;
75-
import com.cloud.agent.api.to.StorageFilerTO;
7674
import com.cloud.exception.InvalidParameterValueException;
7775
import com.cloud.host.Host;
7876
import com.cloud.host.HostVO;
@@ -604,54 +602,52 @@ public boolean canCopy(DataObject srcData, DataObject destData) {
604602
public void resize(DataObject data, AsyncCompletionCallback<CreateCmdResult> callback) {
605603
CreateCmdResult result = null;
606604
try {
607-
if (!(data instanceof VolumeInfo)) {
608-
throw new CloudRuntimeException("resize: Expected VolumeInfo but received " +
609-
(data != null ? data.getClass().getSimpleName() : "null"));
610-
}
611-
VolumeInfo volumeInfo = (VolumeInfo) data;
612-
Object rawPayload = volumeInfo.getpayload();
613-
ResizeVolumePayload payload = (rawPayload instanceof ResizeVolumePayload)
614-
? (ResizeVolumePayload) rawPayload : null;
615-
if (payload == null || payload.newSize == null) {
616-
throw new CloudRuntimeException("Invalid resize payload for volume " + volumeInfo.getId());
617-
}
618-
if (volumeInfo.getDataStore() == null) {
619-
throw new CloudRuntimeException("Data store not found for volume " + volumeInfo.getId());
620-
}
621-
622-
StoragePoolVO storagePool = storagePoolDao.findById(volumeInfo.getDataStore().getId());
623-
if (storagePool == null) {
624-
throw new CloudRuntimeException("Storage pool not found for volume " + volumeInfo.getId());
625-
}
626-
Map<String, String> details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId());
605+
if (data != null && data.getType() == DataObjectType.VOLUME) {
606+
if (!(data instanceof VolumeInfo)) {
607+
throw new CloudRuntimeException("Expected VolumeInfo but received " +
608+
data.getClass().getSimpleName());
609+
}
610+
VolumeInfo volumeInfo = (VolumeInfo) data;
611+
Object rawPayload = volumeInfo.getpayload();
612+
ResizeVolumePayload payload = (rawPayload instanceof ResizeVolumePayload)
613+
? (ResizeVolumePayload) rawPayload : null;
614+
if (payload == null || payload.newSize == null) {
615+
throw new CloudRuntimeException("Invalid resize payload for volume " + volumeInfo.getId());
616+
}
617+
if (volumeInfo.getDataStore() == null) {
618+
throw new CloudRuntimeException("Data store not found for volume " + volumeInfo.getId());
619+
}
627620

628-
VolumeVO volumeVO = volumeDao.findById(volumeInfo.getId());
629-
if (volumeVO == null) {
630-
throw new CloudRuntimeException("Volume not found for id " + volumeInfo.getId());
631-
}
632-
if (payload.newSize < volumeVO.getSize()) {
633-
throw new CloudRuntimeException(String.format(
634-
"Storage pool %s does not support shrinking a volume.", storagePool.getName()));
635-
}
621+
StoragePoolVO storagePool = storagePoolDao.findById(volumeInfo.getDataStore().getId());
622+
if (storagePool == null) {
623+
throw new CloudRuntimeException("Storage pool not found for volume " + volumeInfo.getId());
624+
}
625+
Map<String, String> details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId());
636626

637-
StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(details);
638-
CloudStackVolume cloudStackVolume = new CloudStackVolume();
639-
cloudStackVolume.setVolumeInfo(volumeInfo);
640-
storageStrategy.resizeCloudStackVolume(cloudStackVolume, payload.newSize);
627+
VolumeVO volumeVO = volumeDao.findById(volumeInfo.getId());
628+
if (volumeVO == null) {
629+
throw new CloudRuntimeException("Volume not found for id " + volumeInfo.getId());
630+
}
631+
if (payload.newSize < volumeVO.getSize()) {
632+
throw new CloudRuntimeException("Unable to shrink volume. Volume shrink is not supported.");
633+
}
641634

642-
long currentSize = volumeVO.getSize();
643-
volumeVO.setSize(payload.newSize);
644-
if (!volumeDao.update(volumeVO.getId(), volumeVO)) {
645-
throw new CloudRuntimeException("Failed to update volume " + volumeVO.getId()
646-
+ " after resizing the ONTAP backing object");
635+
StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(details);
636+
CloudStackVolume cloudStackVolume = new CloudStackVolume();
637+
cloudStackVolume.setVolumeInfo(volumeInfo);
638+
storageStrategy.resizeCloudStackVolume(cloudStackVolume, payload.newSize);
639+
640+
volumeVO.setSize(payload.newSize);
641+
if (!volumeDao.update(volumeVO.getId(), volumeVO)) {
642+
throw new CloudRuntimeException("Failed to update volume " + volumeVO.getId()
643+
+ " after resizing the ONTAP backing object");
644+
}
645+
result = new CreateCmdResult(volumeVO.getPath(), new Answer(null, true, null));
646+
logger.info("resize: Successfully resized volume [{}] to [{}] bytes", volumeInfo.getId(), payload.newSize);
647+
} else {
648+
throw new CloudRuntimeException("Expected a VOLUME DataObject but received " +
649+
(data != null ? data.getType() : "null"));
647650
}
648-
String instanceName = payload.instanceName != null ? payload.instanceName : "none";
649-
650-
ResizeVolumeCommand resizeCmd = new ResizeVolumeCommand(volumeVO.getPath(),
651-
new StorageFilerTO(storagePool), currentSize, payload.newSize,
652-
false, instanceName);
653-
result = new CreateCmdResult(volumeVO.getPath(), new Answer(resizeCmd, true, null));
654-
logger.info("resize: Successfully resized volume [{}] to [{}] bytes", volumeInfo.getId(), payload.newSize);
655651
} catch (Exception e) {
656652
String errMsg = e.getMessage();
657653
logger.error("resize: Failed for volume [{}]: {}", data != null ? data.getId() : null, errMsg, e);

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
import java.util.Map;
2525
import java.util.Objects;
2626

27+
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
28+
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
2729
import org.apache.cloudstack.storage.feign.FeignClientFactory;
2830
import org.apache.cloudstack.storage.feign.client.AggregateFeignClient;
2931
import org.apache.cloudstack.storage.feign.client.JobFeignClient;
@@ -43,12 +45,12 @@
4345
import org.apache.cloudstack.storage.feign.model.Volume;
4446
import org.apache.cloudstack.storage.feign.model.response.JobResponse;
4547
import org.apache.cloudstack.storage.feign.model.response.OntapResponse;
46-
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
4748
import org.apache.cloudstack.storage.service.model.AccessGroup;
4849
import org.apache.cloudstack.storage.service.model.CloudStackVolume;
4950
import org.apache.cloudstack.storage.service.model.ProtocolType;
5051
import org.apache.cloudstack.storage.utils.OntapStorageConstants;
5152
import org.apache.cloudstack.storage.utils.OntapStorageUtils;
53+
import org.apache.commons.lang3.StringUtils;
5254
import org.apache.logging.log4j.LogManager;
5355
import org.apache.logging.log4j.Logger;
5456

@@ -57,8 +59,6 @@
5759

5860
import feign.FeignException;
5961

60-
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
61-
6262
/**
6363
* Storage Strategy represents the communication path for all the ONTAP storage options
6464
*
@@ -438,19 +438,15 @@ public void deleteStorageVolume(Volume volume) {
438438
}
439439

440440
/**
441-
* Gets ONTAP Flex-Volume
442-
* Eligible only for Unified ONTAP storage
443-
* throw exception in case of disaggregated ONTAP storage
441+
* Gets ONTAP Flex-Volume by UUID.
442+
* Eligible only for Unified ONTAP storage.
443+
* Throws exception in case of disaggregated ONTAP storage.
444444
*
445-
* @param volume the volume to retrieve
446-
* @return the retrieved Volume object
445+
* @param uuid the UUID of the volume to retrieve
446+
* @return the retrieved Volume object, or null if not found
447447
*/
448-
public Volume getStorageVolume(Volume volume) {
449-
return getStorageVolume(volume.getUuid());
450-
}
451-
452448
public Volume getStorageVolume(String uuid) {
453-
if (uuid == null || uuid.isBlank()) {
449+
if (StringUtils.isBlank(uuid)) {
454450
throw new CloudRuntimeException("Cannot fetch ONTAP volume: UUID is null or empty");
455451
}
456452
logger.info("getStorageVolume: Fetching ONTAP volume by UUID: {}", uuid);

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,7 @@ public void resizeCloudStackVolume(CloudStackVolume cloudstackVolume, long sizeI
299299

300300
// Resolve LUN UUID from volume details when not pre-populated on the cloudstackVolume
301301
if (cloudstackVolume.getLun() == null || cloudstackVolume.getLun().getUuid() == null) {
302+
logger.debug("LUN details not present on cloudstackVolume, resolving UUID from volume details");
302303
long volumeId = cloudstackVolume.getVolumeInfo().getId();
303304
VolumeDetailVO lunUuidDetail = volumeDetailsDao.findDetail(volumeId, OntapStorageConstants.LUN_DOT_UUID);
304305
if (lunUuidDetail == null || StringUtils.isBlank(lunUuidDetail.getValue())) {
@@ -307,6 +308,8 @@ public void resizeCloudStackVolume(CloudStackVolume cloudstackVolume, long sizeI
307308
Lun resolvedLun = new Lun();
308309
resolvedLun.setUuid(lunUuidDetail.getValue());
309310
cloudstackVolume.setLun(resolvedLun);
311+
} else {
312+
logger.debug("resizeCloudStackVolume: LUN UUID [{}] already present on cloudstackVolume, skipping volume details lookup", cloudstackVolume.getLun().getUuid());
310313
}
311314

312315
String lunUuid = cloudstackVolume.getLun().getUuid();

‎plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1520,7 +1520,7 @@ void testResize_NullData_FailsAndCompletesCallbackOnce() {
15201520
ArgumentCaptor<CreateCmdResult> resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class);
15211521
verify(createCallback, times(1)).complete(resultCaptor.capture());
15221522
assertFalse(resultCaptor.getValue().isSuccess());
1523-
assertTrue(resultCaptor.getValue().getResult().contains("Expected VolumeInfo"));
1523+
assertTrue(resultCaptor.getValue().getResult().contains("Expected a VOLUME DataObject"));
15241524
}
15251525

15261526
@Test
@@ -1532,7 +1532,7 @@ void testResize_NonVolumeInfo_FailsAndCompletesCallbackOnce() {
15321532
ArgumentCaptor<CreateCmdResult> resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class);
15331533
verify(createCallback, times(1)).complete(resultCaptor.capture());
15341534
assertFalse(resultCaptor.getValue().isSuccess());
1535-
assertTrue(resultCaptor.getValue().getResult().contains("Expected VolumeInfo"));
1535+
assertTrue(resultCaptor.getValue().getResult().contains("Expected a VOLUME DataObject"));
15361536
}
15371537

15381538
@Test
@@ -1644,7 +1644,7 @@ void testResize_ShrinkAttempt_Fails() {
16441644
ArgumentCaptor<CreateCmdResult> resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class);
16451645
verify(createCallback).complete(resultCaptor.capture());
16461646
assertFalse(resultCaptor.getValue().isSuccess());
1647-
assertTrue(resultCaptor.getValue().getResult().contains("does not support shrinking"));
1647+
assertTrue(resultCaptor.getValue().getResult().contains("Unable to shrink volume"));
16481648
verify(sanStrategy, never()).resizeCloudStackVolume(any(), anyLong());
16491649
}
16501650
}

0 commit comments

Comments
 (0)