winterhazel commented on code in PR #13053:
URL: https://github.com/apache/cloudstack/pull/13053#discussion_r3191253147
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java:
##########
@@ -77,7 +72,22 @@ public void setOntapStorage(OntapStorage ontapStorage) {
@Override
public CloudStackVolume createCloudStackVolume(CloudStackVolume
cloudstackVolume) {
- return null;
+ logger.info("createCloudStackVolume: Create cloudstack volume " +
cloudstackVolume);
+ try {
+ // Step 1: set cloudstack volume metadata
+ String volumeUuid =
updateCloudStackVolumeMetadata(cloudstackVolume.getDatastoreId(),
cloudstackVolume.getVolumeInfo());
Review Comment:
`volumeUuid` is not used
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java:
##########
@@ -96,24 +119,40 @@ public void copyCloudStackVolume(CloudStackVolume
cloudstackVolume) {
@Override
public CloudStackVolume getCloudStackVolume(Map<String, String>
cloudStackVolumeMap) {
- return null;
+ logger.info("getCloudStackVolume: Get cloudstack volume " +
cloudStackVolumeMap);
+ CloudStackVolume cloudStackVolume = null;
+ FileInfo fileInfo =
getFile(cloudStackVolumeMap.get(OntapStorageConstants.VOLUME_UUID),cloudStackVolumeMap.get(OntapStorageConstants.FILE_PATH));
+
+ if(fileInfo != null){
Review Comment:
```suggestion
if (fileInfo != null) {
```
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java:
##########
@@ -300,4 +402,153 @@ private ExportPolicy
createExportPolicyRequest(AccessGroup accessGroup,String sv
return exportPolicy;
}
+
+ private String updateCloudStackVolumeMetadata(String dataStoreId,
DataObject volumeInfo) {
+ logger.info("updateCloudStackVolumeMetadata called with datastoreID:
{} volumeInfo: {} ", dataStoreId, volumeInfo );
+ try {
+ VolumeObject volumeObject = (VolumeObject) volumeInfo;
+ long volumeId = volumeObject.getId();
+ logger.info("updateCloudStackVolumeMetadata: VolumeInfo ID from
VolumeObject: {}", volumeId);
+ VolumeVO volume = volumeDao.findById(volumeId);
+ if (volume == null) {
+ throw new CloudRuntimeException("Volume not found with id: " +
volumeId);
+ }
+ String volumeUuid = volumeInfo.getUuid();
+ volume.setPoolType(Storage.StoragePoolType.NetworkFilesystem);
+ volume.setPoolId(Long.parseLong(dataStoreId));
+ volume.setPath(volumeUuid); // Filename for qcow2 file
+ volumeDao.update(volume.getId(), volume);
+ logger.info("Updated volume path to {} for volume ID {}",
volumeUuid, volumeId);
+ return volumeUuid;
+ }catch (Exception e){
+ logger.error("updateCloudStackVolumeMetadata: Exception while
updating volumeInfo: {} in volume: {}", dataStoreId, volumeInfo.getUuid(), e);
+ throw new CloudRuntimeException("Exception while updating
volumeInfo: " + e.getMessage());
+ }
Review Comment:
Adjust the indentation of this try block. All lines are missing a space
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java:
##########
@@ -252,19 +285,88 @@ private void assignExportPolicyToVolume(String
volumeUuid, String policyName) {
Thread.sleep(OntapStorageConstants.CREATE_VOLUME_CHECK_SLEEP_TIME);
}
} catch (Exception e) {
- logger.error("Exception while updating volume: ", e);
+ logger.error("assignExportPolicyToVolume: Exception while
updating volume: ", e);
throw new CloudRuntimeException("Failed to update volume: " +
e.getMessage());
}
- logger.info("Export policy successfully assigned to volume: {}",
volumeUuid);
+ logger.info("assignExportPolicyToVolume: Export policy
successfully assigned to volume: {}", volumeUuid);
} catch (FeignException e) {
- logger.error("Failed to assign export policy to volume: {}",
volumeUuid, e);
+ logger.error("assignExportPolicyToVolume: Failed to assign export
policy to volume: {}", volumeUuid, e);
throw new CloudRuntimeException("Failed to assign export policy: "
+ e.getMessage());
} catch (Exception e) {
- logger.error("Exception while assigning export policy to volume:
{}", volumeUuid, e);
+ logger.error("assignExportPolicyToVolume: Exception while
assigning export policy to volume: {}", volumeUuid, e);
throw new CloudRuntimeException("Failed to assign export policy: "
+ e.getMessage());
}
}
+ private boolean createFile(String volumeUuid, String filePath, FileInfo
fileInfo) {
+ logger.info("createFile: Creating file: {} in volume: {}", filePath,
volumeUuid);
+ try {
+ String authHeader =
OntapStorageUtils.generateAuthHeader(storage.getUsername(),
storage.getPassword());
+ nasFeignClient.createFile(authHeader, volumeUuid, filePath,
fileInfo);
+ logger.info("createFile: File created successfully: {} in volume:
{}", filePath, volumeUuid);
+ return true;
+ } catch (FeignException e) {
+ logger.error("createFile: Failed to create file: {} in volume:
{}", filePath, volumeUuid, e);
+ return false;
+ } catch (Exception e) {
+ logger.error("createFile: Exception while creating file: {} in
volume: {}", filePath, volumeUuid, e);
+ return false;
+ }
+ }
+
+ private boolean deleteFile(String volumeUuid, String filePath) {
+ logger.info("deleteFile: Deleting file: {} from volume: {}", filePath,
volumeUuid);
+ try {
+ String authHeader =
OntapStorageUtils.generateAuthHeader(storage.getUsername(),
storage.getPassword());
+ nasFeignClient.deleteFile(authHeader, volumeUuid, filePath);
+ logger.info("deleteFile: File deleted successfully: {} from
volume: {}", filePath, volumeUuid);
+ return true;
+ } catch (FeignException e) {
+ logger.error("deleteFile: Failed to delete file: {} from volume:
{}", filePath, volumeUuid, e);
+ return false;
+ } catch (Exception e) {
+ logger.error("deleteFile: Exception while deleting file: {} from
volume: {}", filePath, volumeUuid, e);
+ return false;
+ }
+ }
+
+ private OntapResponse<FileInfo> getFileInfo(String volumeUuid, String
filePath) {
+ logger.debug("getFileInfo: Getting file info for: {} in volume: {}",
filePath, volumeUuid);
+ try {
+ String authHeader =
OntapStorageUtils.generateAuthHeader(storage.getUsername(),
storage.getPassword());
+ OntapResponse<FileInfo> response =
nasFeignClient.getFileResponse(authHeader, volumeUuid, filePath);
+ logger.debug("getFileInfo: Retrieved file info for: {} in volume:
{}", filePath, volumeUuid);
+ return response;
+ } catch (FeignException e){
+ if (e.status() == 404) {
+ logger.debug("getFileInfo: File not found: {} in volume: {}",
filePath, volumeUuid);
+ return null;
+ }
+ logger.error("getFileInfo: Failed to get file info: {} in volume:
{}", filePath, volumeUuid, e);
+ throw new CloudRuntimeException("Failed to get file info: " +
e.getMessage());
+ } catch (Exception e){
+ logger.error("getFileInfo: Exception while getting file info: {}
in volume: {}", filePath, volumeUuid, e);
+ throw new CloudRuntimeException("Failed to get file info: " +
e.getMessage());
+ }
+ }
+
+ private boolean updateFile(String volumeUuid, String filePath, FileInfo
fileInfo) {
+ logger.info("updateFile: Updating file: {} in volume: {}", filePath,
volumeUuid);
+ try {
+ String authHeader =
OntapStorageUtils.generateAuthHeader(storage.getUsername(),
storage.getPassword());
+ nasFeignClient.updateFile( authHeader, volumeUuid, filePath,
fileInfo);
+ logger.info("updateFile: File updated successfully: {} in volume:
{}", filePath, volumeUuid);
+ return true;
+ } catch (FeignException e) {
+ logger.error("updateFile: Failed to update file: {} in volume:
{}", filePath, volumeUuid, e);
+ return false;
+ } catch (Exception e){
+ logger.error("updateFile: Exception while updating file: {} in
volume: {}", filePath, volumeUuid, e);
+ return false;
+ }
+ }
+
+
Review Comment:
Methods from line 301-367 are not used. Is this expected?
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java:
##########
@@ -180,11 +212,11 @@ public void disableLogicalAccess(Map<String, String>
values) {
@Override
public Map<String, String> getLogicalAccess(Map<String, String> values) {
Review Comment:
This method does not seem to be used outside the tests. Is this expected?
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java:
##########
@@ -36,41 +42,116 @@
public class OntapStorageUtils {
private static final Logger logger =
LogManager.getLogger(OntapStorageUtils.class);
-
private static final String BASIC = "Basic";
private static final String AUTH_HEADER_COLON = ":";
+ /**
+ * Method generates authentication headers using storage backend
credentials passed as normal string
+ *
+ * @param username -->> username of the storage backend
+ * @param password -->> normal decoded password of the storage backend
+ * @return
+ */
public static String generateAuthHeader (String username, String password)
{
byte[] encodedBytes = Base64Utils.encode((username + AUTH_HEADER_COLON
+ password).getBytes(StandardCharsets.UTF_8));
return BASIC + StringUtils.SPACE + new String(encodedBytes);
}
+ public static CloudStackVolume
createCloudStackVolumeRequestByProtocol(StoragePoolVO storagePool, Map<String,
String> details, DataObject volumeObject) {
+ CloudStackVolume cloudStackVolumeRequest = null;
+
+ String protocol = details.get(OntapStorageConstants.PROTOCOL);
+ ProtocolType protocolType = ProtocolType.valueOf(protocol);
+ switch (protocolType) {
+ case NFS3:
+ cloudStackVolumeRequest = new CloudStackVolume();
+
cloudStackVolumeRequest.setDatastoreId(String.valueOf(storagePool.getId()));
+ cloudStackVolumeRequest.setVolumeInfo(volumeObject);
+ break;
+ case ISCSI:
+ Svm svm = new Svm();
+ svm.setName(details.get(OntapStorageConstants.SVM_NAME));
+ cloudStackVolumeRequest = new CloudStackVolume();
+ Lun lunRequest = new Lun();
+ lunRequest.setSvm(svm);
+
+ LunSpace lunSpace = new LunSpace();
+ lunSpace.setSize(volumeObject.getSize());
+ lunRequest.setSpace(lunSpace);
+ //Lun name is full path like in unified
"/vol/VolumeName/LunName"
+ String lunName =
volumeObject.getName().replace(OntapStorageConstants.HYPHEN,
OntapStorageConstants.UNDERSCORE);
+ if(!isValidName(lunName)) {
+ String errMsg = "createAsync: Invalid dataObject name [" +
lunName + "]. It must start with a letter and can only contain letters, digits,
and underscores, and be up to 200 characters long.";
+ throw new InvalidParameterValueException(errMsg);
+ }
+ String lunFullName = getLunName(storagePool.getName(),
lunName);
+ lunRequest.setName(lunFullName);
+
+ String osType =
getOSTypeFromHypervisor(storagePool.getHypervisor().name());
+ lunRequest.setOsType(Lun.OsTypeEnum.valueOf(osType));
+
+ cloudStackVolumeRequest.setLun(lunRequest);
+ break;
+ default:
+ throw new CloudRuntimeException("Unsupported protocol " +
protocol);
+
+ }
+ return cloudStackVolumeRequest;
+ }
+
+ public static boolean isValidName(String name) {
+ // Check for null and length constraint first
+ if (name == null || name.length() > 200) {
+ return false;
+ }
+ // Regex: Starts with a letter, followed by letters, digits, or
underscores
+ return name.matches(OntapStorageConstants.ONTAP_NAME_REGEX);
+ }
+
+ public static String getOSTypeFromHypervisor(String hypervisorType){
Review Comment:
```suggestion
public static String getOSTypeFromHypervisor(String hypervisorType) {
```
--
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]
