diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java index 7b22c123cc78..495ae45a2896 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java @@ -210,7 +210,7 @@ private void rescanIscsiSessions(String iqn, String host, int port) { } private void waitForDiskToBecomeAvailable(String volumeUuid, KVMStoragePool pool) { - int numberOfTries = 30; + int numberOfTries = 10; int timeBetweenTries = 1000; while (getPhysicalDisk(volumeUuid, pool).getSize() == 0 && numberOfTries > 0) { @@ -294,8 +294,9 @@ private long getDeviceSize(String deviceByPath) { logger.debug("Device by-path does not exist yet: " + deviceByPath); return 0L; } - } catch (Exception ignore) { + } catch (Exception ex) { // If FS check fails for any reason, fall back to blockdev call + logger.error("Error fetching device size for {}", deviceByPath, ex); } Script iScsiAdmCmd = new Script(true, "blockdev", 0, logger); diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java index 9034a136b647..04996b74d2a5 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java @@ -70,6 +70,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.jetbrains.annotations.Nullable; import javax.inject.Inject; import java.util.ArrayList; @@ -110,7 +111,7 @@ public DataTO getTO(DataObject data) { @Override public boolean volumesRequireGrantAccessWhenUsed() { - logger.info("OntapPrimaryDatastoreDriver: volumesRequireGrantAccessWhenUsed: Called"); + logger.trace("volumesRequireGrantAccessWhenUsed invoked"); return true; } @@ -389,44 +390,7 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL))) { // Only retrieve LUN name for iSCSI volumes - String cloudStackVolumeName = volumeDetailsDao.findDetail(volumeVO.getId(), OntapStorageConstants.LUN_DOT_NAME).getValue(); - UnifiedSANStrategy sanStrategy = (UnifiedSANStrategy) OntapStorageUtils.getStrategyByStoragePoolDetails(details); - String accessGroupName = OntapStorageUtils.getIgroupName(svmName, host.getName()); - - // Validate if Igroup exist ONTAP for this host as we may be using delete_on_unmap= true and igroup may be deleted by ONTAP automatically - Map getAccessGroupMap = Map.of( - OntapStorageConstants.NAME, accessGroupName, - OntapStorageConstants.SVM_DOT_NAME, svmName - ); - AccessGroup accessGroup = sanStrategy.getAccessGroup(getAccessGroupMap); - if(accessGroup == null || accessGroup.getIgroup() == null) { - logger.info("grantAccess: Igroup {} does not exist for the host {} : Need to create Igroup for the host ", accessGroupName, host.getName()); - // create the igroup for the host and perform lun-mapping - accessGroup = new AccessGroup(); - List hosts = new ArrayList<>(); - hosts.add((HostVO) host); - accessGroup.setHostsToConnect(hosts); - accessGroup.setStoragePoolId(storagePool.getId()); - accessGroup = sanStrategy.createAccessGroup(accessGroup); - }else{ - logger.info("grantAccess: Igroup {} already exist for the host {}: ", accessGroup.getIgroup().getName() ,host.getName()); - /* TODO Below cases will be covered later, for now they will be a pre-requisite on customer side - 1. Igroup exist with the same name but host initiator has been rempved - 2. Igroup exist with the same name but host initiator has been changed may be due to new NIC or new adapter - In both cases we need to verify current host initiator is registered in the igroup before allowing access - Incase it is not , add it and proceed for lun-mapping - */ - } - logger.info("grantAccess: Igroup {} is present now with initiators {} ", accessGroup.getIgroup().getName(), accessGroup.getIgroup().getInitiators()); - // Create or retrieve existing LUN mapping - String lunNumber = sanStrategy.ensureLunMapped(svmName, cloudStackVolumeName, accessGroupName); - - // Update volume path if changed (e.g., after migration or re-mapping) - String iscsiPath = OntapStorageConstants.SLASH + storagePool.getPath() + OntapStorageConstants.SLASH + lunNumber; - if (volumeVO.getPath() == null || !volumeVO.getPath().equals(iscsiPath)) { - volumeVO.set_iScsiName(iscsiPath); - volumeVO.setPath(iscsiPath); - } + grantAccessIscsi(host, volumeVO, details, svmName, storagePool); } else if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL))) { // For NFS, no access grant needed - file is accessible via mount logger.debug("grantAccess: NFS volume [{}], no igroup mapping required", volumeVO.getUuid()); @@ -446,6 +410,47 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore } } + private void grantAccessIscsi(Host host, VolumeVO volumeVO, Map details, String svmName, StoragePoolVO storagePool) { + String cloudStackVolumeName = volumeDetailsDao.findDetail(volumeVO.getId(), OntapStorageConstants.LUN_DOT_NAME).getValue(); + UnifiedSANStrategy sanStrategy = (UnifiedSANStrategy) OntapStorageUtils.getStrategyByStoragePoolDetails(details); + String accessGroupName = OntapStorageUtils.getIgroupName(svmName, host.getName()); + + // Validate if Igroup exist ONTAP for this host as we may be using delete_on_unmap= true and igroup may be deleted by ONTAP automatically + Map getAccessGroupMap = Map.of( + OntapStorageConstants.NAME, accessGroupName, + OntapStorageConstants.SVM_DOT_NAME, svmName + ); + AccessGroup accessGroup = sanStrategy.getAccessGroup(getAccessGroupMap); + if(accessGroup == null || accessGroup.getIgroup() == null) { + logger.info("grantAccess: Igroup {} does not exist for the host {} : Need to create Igroup for the host ", accessGroupName, host.getName()); + // create the igroup for the host and perform lun-mapping + accessGroup = new AccessGroup(); + List hosts = new ArrayList<>(); + hosts.add((HostVO) host); + accessGroup.setHostsToConnect(hosts); + accessGroup.setStoragePoolId(storagePool.getId()); + accessGroup = sanStrategy.createAccessGroup(accessGroup); + }else{ + logger.info("grantAccess: Igroup {} already exist for the host {}: ", accessGroup.getIgroup().getName() , host.getName()); + /* TODO Below cases will be covered later, for now they will be a pre-requisite on customer side + 1. Igroup exist with the same name but host initiator has been removed + 2. Igroup exist with the same name but host initiator has been changed may be due to new NIC or new adapter + In both cases we need to verify current host initiator is registered in the igroup before allowing access + Incase it is not , add it and proceed for lun-mapping + */ + } + logger.info("grantAccess: Igroup {} is present now with initiators {} ", accessGroup.getIgroup().getName(), accessGroup.getIgroup().getInitiators()); + // Create or retrieve existing LUN mapping + String lunNumber = sanStrategy.ensureLunMapped(svmName, cloudStackVolumeName, accessGroupName); + + // Update volume path if changed (e.g., after migration or re-mapping) + String iscsiPath = OntapStorageConstants.SLASH + storagePool.getPath() + OntapStorageConstants.SLASH + lunNumber; + if (volumeVO.getPath() == null || !volumeVO.getPath().equals(iscsiPath)) { + volumeVO.set_iScsiName(iscsiPath); + volumeVO.setPath(iscsiPath); + } + } + /** * Revokes a host's access to a volume. */ @@ -505,42 +510,61 @@ private void revokeAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, // Retrieve LUN name from volume details; if missing, volume may not have been fully created VolumeDetailVO lunDetail = volumeDetailsDao.findDetail(volumeVO.getId(), OntapStorageConstants.LUN_DOT_NAME); - String lunName = lunDetail != null ? lunDetail.getValue() : null; - if (lunName == null) { - logger.warn("revokeAccessForVolume: No LUN name found for volume [{}]; skipping revoke", volumeVO.getId()); - return; - } - - // Verify LUN still exists on ONTAP (may have been manually deleted) - CloudStackVolume cloudStackVolume = getCloudStackVolumeByName(storageStrategy, svmName, lunName); - if (cloudStackVolume == null || cloudStackVolume.getLun() == null || cloudStackVolume.getLun().getUuid() == null) { - logger.warn("revokeAccessForVolume: LUN for volume [{}] not found on ONTAP, skipping revoke", volumeVO.getId()); - return; - } - - // Verify igroup still exists on ONTAP - AccessGroup accessGroup = getAccessGroupByName(storageStrategy, svmName, accessGroupName); - if (accessGroup == null || accessGroup.getIgroup() == null || accessGroup.getIgroup().getUuid() == null) { - logger.warn("revokeAccessForVolume: iGroup [{}] not found on ONTAP, skipping revoke", accessGroupName); - return; - } - - // Verify host initiator is in the igroup before attempting to remove mapping - SANStrategy sanStrategy = (UnifiedSANStrategy) storageStrategy; - if (!sanStrategy.validateInitiatorInAccessGroup(host.getStorageUrl(), svmName, accessGroup.getIgroup())) { - logger.warn("revokeAccessForVolume: Initiator [{}] is not in iGroup [{}], skipping revoke", - host.getStorageUrl(), accessGroupName); - return; - } + ValidateRevoke result = getValidateRevoke(volumeVO, host, lunDetail, storageStrategy, svmName, accessGroupName); + if (result == null) return; // Remove the LUN mapping from the igroup Map disableLogicalAccessMap = new HashMap<>(); - disableLogicalAccessMap.put(OntapStorageConstants.LUN_DOT_UUID, cloudStackVolume.getLun().getUuid()); - disableLogicalAccessMap.put(OntapStorageConstants.IGROUP_DOT_UUID, accessGroup.getIgroup().getUuid()); + disableLogicalAccessMap.put(OntapStorageConstants.LUN_DOT_UUID, result.cloudStackVolume.getLun().getUuid()); + disableLogicalAccessMap.put(OntapStorageConstants.IGROUP_DOT_UUID, result.accessGroup.getIgroup().getUuid()); storageStrategy.disableLogicalAccess(disableLogicalAccessMap); logger.info("revokeAccessForVolume: Successfully revoked access to LUN [{}] for host [{}]", - lunName, host.getName()); + result.lunName, host.getName()); + } + } + + @Nullable + private ValidateRevoke getValidateRevoke(VolumeVO volumeVO, Host host, VolumeDetailVO lunDetail, StorageStrategy storageStrategy, String svmName, String accessGroupName) { + String lunName = lunDetail != null ? lunDetail.getValue() : null; + if (lunName == null) { + logger.warn("revokeAccessForVolume: No LUN name found for volume [{}]; skipping revoke", volumeVO.getId()); + return null; + } + + // Verify LUN still exists on ONTAP (may have been manually deleted) + CloudStackVolume cloudStackVolume = getCloudStackVolumeByName(storageStrategy, svmName, lunName); + if (cloudStackVolume == null || cloudStackVolume.getLun() == null || cloudStackVolume.getLun().getUuid() == null) { + logger.warn("revokeAccessForVolume: LUN for volume [{}] not found on ONTAP, skipping revoke", volumeVO.getId()); + return null; + } + + // Verify igroup still exists on ONTAP + AccessGroup accessGroup = getAccessGroupByName(storageStrategy, svmName, accessGroupName); + if (accessGroup == null || accessGroup.getIgroup() == null || accessGroup.getIgroup().getUuid() == null) { + logger.warn("revokeAccessForVolume: iGroup [{}] not found on ONTAP, skipping revoke", accessGroupName); + return null; + } + + // Verify host initiator is in the igroup before attempting to remove mapping + SANStrategy sanStrategy = (UnifiedSANStrategy) storageStrategy; + if (!sanStrategy.validateInitiatorInAccessGroup(host.getStorageUrl(), svmName, accessGroup.getIgroup())) { + logger.warn("revokeAccessForVolume: Initiator [{}] is not in iGroup [{}], skipping revoke", + host.getStorageUrl(), accessGroupName); + return null; + } + return new ValidateRevoke(lunName, cloudStackVolume, accessGroup); + } + + private static class ValidateRevoke { + public final String lunName; + public final CloudStackVolume cloudStackVolume; + public final AccessGroup accessGroup; + + public ValidateRevoke(String lunName, CloudStackVolume cloudStackVolume, AccessGroup accessGroup) { + this.lunName = lunName; + this.cloudStackVolume = cloudStackVolume; + this.accessGroup = accessGroup; } } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/VolumeConcise.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/VolumeConcise.java index b76fb4f03d6b..82cea1260e10 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/VolumeConcise.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/VolumeConcise.java @@ -30,7 +30,6 @@ public class VolumeConcise { private String uuid; @JsonProperty("name") - private String name; public String getUuid() { @@ -46,5 +45,6 @@ public String getName() { } public void setName(String name) { - this.name = name; } + this.name = name; + } } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java index 1e44ef3aaa83..d7b89de25461 100755 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java @@ -231,7 +231,7 @@ private long validateInitializeInputs(Long capacityBytes, Long podId, Long clust throw new CloudRuntimeException("Storage pool name is null or empty, cannot create primary storage"); } - if (StringUtils.isBlank(providerName )) { + if (StringUtils.isBlank(providerName)) { throw new CloudRuntimeException("Provider name is null or empty, cannot create primary storage"); } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java index 02c201adaa13..8fdde0029127 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java @@ -593,9 +593,9 @@ public abstract JobResponse revertSnapshotForCloudStackVolume(String snapshotNam * //TODO for NFS 3.0 and NFS 4.1 protocols (e.g., export rule management) * //TODO for Nvme/TCP and Nvme/FC protocols * @param values map including SVM name, LUN name, and igroup name (for SAN) or equivalent for NAS - * @return map containing logical unit number for the new/existing mapping (SAN) or relevant info for NAS + * @return String containing logical unit number for the new/existing mapping (SAN) or relevant info for NAS */ - abstract public Map enableLogicalAccess(Map values); + abstract public String enableLogicalAccess(Map values); /** * Method encapsulates the behavior based on the opted protocol in subclasses @@ -608,9 +608,9 @@ public abstract JobResponse revertSnapshotForCloudStackVolume(String snapshotNam * Method encapsulates the behavior based on the opted protocol in subclasses * lunMap lookup for iSCSI/FC protocols (GET-only, no side-effects) * @param values map with SVM name, LUN name, and igroup name (for SAN) or equivalent for NAS - * @return map containing logical unit number if mapping exists; otherwise null + * @return String containing logical unit number if mapping exists; otherwise null */ - abstract public Map getLogicalAccess(Map values); + abstract public String getLogicalAccess(Map values); // ── FlexVolume Snapshot accessors ──────────────────────────────────────── diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java index f305dd6d070d..477e92630387 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java @@ -202,7 +202,7 @@ public AccessGroup getAccessGroup(Map values) { } @Override - public Map enableLogicalAccess(Map values) { + public String enableLogicalAccess(Map values) { return null; } @@ -211,8 +211,8 @@ public void disableLogicalAccess(Map values) { } @Override - public Map getLogicalAccess(Map values) { - return Map.of(); + public String getLogicalAccess(Map values) { + return null; } private ExportPolicy createExportPolicy(String svmName, ExportPolicy policy) { diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java index 02661909d7e6..5f1ac265fc50 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java @@ -62,7 +62,7 @@ public void setOntapStorage(OntapStorage ontapStorage) { @Override public CloudStackVolume createCloudStackVolume(CloudStackVolume cloudstackVolume) { - logger.info("createCloudStackVolume : Creating Lun with cloudstackVolume request {} ", cloudstackVolume); + logger.trace("createCloudStackVolume : Creating Lun with cloudstackVolume request {} ", cloudstackVolume); if (cloudstackVolume == null || cloudstackVolume.getLun() == null) { logger.error("createCloudStackVolume: LUN creation failed. Invalid request: {}", cloudstackVolume); throw new CloudRuntimeException(" Failed to create Lun, invalid request"); @@ -79,7 +79,6 @@ public CloudStackVolume createCloudStackVolume(CloudStackVolume cloudstackVolume } Lun lun = createdLun.getRecords().get(0); logger.debug("createCloudStackVolume: LUN created successfully. Lun: {}", lun); - logger.info("createCloudStackVolume: LUN created successfully. LunName: {}", lun.getName()); CloudStackVolume createdCloudStackVolume = new CloudStackVolume(); createdCloudStackVolume.setLun(lun); @@ -105,7 +104,7 @@ public void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) { logger.error("deleteCloudStackVolume: Lun deletion failed. Invalid request: {}", cloudstackVolume); throw new CloudRuntimeException(" Failed to delete Lun, invalid request"); } - logger.info("deleteCloudStackVolume : Deleting Lun: {}", cloudstackVolume.getLun().getName()); + logger.trace("deleteCloudStackVolume : Deleting Lun: {}", cloudstackVolume.getLun().getName()); try { String authHeader = OntapStorageUtils.generateAuthHeader(storage.getUsername(), storage.getPassword()); Map queryParams = Map.of("allow_delete_while_mapped", "true"); @@ -118,7 +117,7 @@ public void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) { } throw feignEx; } - logger.info("deleteCloudStackVolume: Lun deleted successfully. LunName: {}", cloudstackVolume.getLun().getName()); + logger.trace("deleteCloudStackVolume: Lun deleted successfully. LunName: {}", cloudstackVolume.getLun().getName()); } catch (Exception e) { logger.error("Exception occurred while deleting Lun: {}, Exception: {}", cloudstackVolume.getLun().getName(), e.getMessage()); throw new CloudRuntimeException("Failed to delete Lun: " + e.getMessage()); @@ -130,8 +129,7 @@ public void copyCloudStackVolume(CloudStackVolume cloudstackVolume) {} @Override public CloudStackVolume getCloudStackVolume(Map values) { - logger.info("getCloudStackVolume : fetching Lun"); - logger.debug("getCloudStackVolume : fetching Lun with params {} ", values); + logger.trace("getCloudStackVolume : fetching Lun with params {} ", values); if (values == null || values.isEmpty()) { logger.error("getCloudStackVolume: get Lun failed. Invalid request: {}", values); throw new CloudRuntimeException(" get Lun Failed, invalid request"); @@ -152,7 +150,6 @@ public CloudStackVolume getCloudStackVolume(Map values) { } Lun lun = lunResponse.getRecords().get(0); logger.debug("getCloudStackVolume: Lun Details : {}", lun); - logger.info("getCloudStackVolume: Fetched the Lun successfully. LunName: {}", lun.getName()); CloudStackVolume cloudStackVolume = new CloudStackVolume(); cloudStackVolume.setLun(lun); @@ -172,7 +169,7 @@ public CloudStackVolume getCloudStackVolume(Map values) { @Override public AccessGroup createAccessGroup(AccessGroup accessGroup) { - logger.debug("createAccessGroup : Creating Igroup with access group request {} ", accessGroup); + logger.trace("createAccessGroup : Creating Igroup with access group request {} ", accessGroup); if (accessGroup == null) { logger.error("createAccessGroup: Igroup creation failed. Invalid request: {}", accessGroup); throw new CloudRuntimeException("Failed to create Igroup, invalid request"); @@ -188,7 +185,7 @@ public AccessGroup createAccessGroup(AccessGroup accessGroup) { String igroupName = null; try { Map dataStoreDetails = storagePoolDetailsDao.listDetailsKeyPairs(accessGroup.getStoragePoolId()); - logger.debug("createAccessGroup: Successfully fetched datastore details."); + logger.trace("createAccessGroup: Successfully fetched datastore details."); // Generate Igroup request Igroup igroupRequest = new Igroup(); @@ -239,7 +236,6 @@ public AccessGroup createAccessGroup(AccessGroup accessGroup) { throw feignEx; } - logger.info("createAccessGroup: createdIgroup: {}", createdIgroup); logger.debug("createAccessGroup: createdIgroup Records: {}", createdIgroup.getRecords()); if (createdIgroup.getRecords() == null || createdIgroup.getRecords().isEmpty()) { logger.error("createAccessGroup: Igroup creation failed for Igroup Name {}", igroupName); @@ -247,10 +243,9 @@ public AccessGroup createAccessGroup(AccessGroup accessGroup) { } Igroup igroup = createdIgroup.getRecords().get(0); logger.debug("createAccessGroup: Successfully extracted igroup from response: {}", igroup); - logger.info("createAccessGroup: Igroup created successfully. IgroupName: {}", igroup.getName()); createdAccessGroup.setIgroup(igroup); - logger.debug("createAccessGroup: Returning createdAccessGroup"); + logger.trace("createAccessGroup: Returning createdAccessGroup"); return createdAccessGroup; } catch (Exception e) { logger.error("Exception occurred while creating Igroup: {}, Exception: {}", igroupName, e.getMessage(), e); @@ -260,7 +255,7 @@ public AccessGroup createAccessGroup(AccessGroup accessGroup) { @Override public void deleteAccessGroup(AccessGroup accessGroup) { - logger.info("deleteAccessGroup: Deleting iGroup"); + logger.trace("deleteAccessGroup: Deleting iGroup"); if (accessGroup == null) { logger.error("deleteAccessGroup: Igroup deletion failed. Invalid request: {}", accessGroup); @@ -335,7 +330,7 @@ private boolean validateProtocolSupport(List hosts, ProtocolType protoco return false; } } - logger.info("validateProtocolSupportAndFetchHostsIdentifier: All hosts support the protocol: " + protocolType.name()); + logger.trace("validateProtocolSupportAndFetchHostsIdentifier: All hosts support the protocol: " + protocolType.name()); return true; } @@ -346,8 +341,7 @@ public AccessGroup updateAccessGroup(AccessGroup accessGroup) { @Override public AccessGroup getAccessGroup(Map values) { - logger.info("getAccessGroup : fetch Igroup"); - logger.debug("getAccessGroup : fetching Igroup with params {} ", values); + logger.trace("getAccessGroup : fetching Igroup with params {} ", values); if (values == null || values.isEmpty()) { logger.error("getAccessGroup: get Igroup failed. Invalid request: {}", values); throw new CloudRuntimeException("get Igroup Failed, invalid request"); @@ -383,10 +377,10 @@ public AccessGroup getAccessGroup(Map values) { } } - public Map enableLogicalAccess(Map values) { - logger.info("enableLogicalAccess : Create LunMap"); - logger.debug("enableLogicalAccess : Creating LunMap with values {} ", values); - Map response = null; + @Override + public String enableLogicalAccess(Map values) { + logger.trace("enableLogicalAccess : Creating LunMap with values {} ", values); + String lunNumber = null; if (values == null) { logger.error("enableLogicalAccess: LunMap creation failed. Invalid request values: null"); throw new CloudRuntimeException("Failed to create LunMap, invalid request"); @@ -435,25 +429,28 @@ public Map enableLogicalAccess(Map values) { OntapStorageConstants.IGROUP_DOT_NAME, igroupName, OntapStorageConstants.FIELDS, OntapStorageConstants.LOGICAL_UNIT_NUMBER )); - response = Map.of( - OntapStorageConstants.LOGICAL_UNIT_NUMBER, lunMapResponse.getRecords().get(0).getLogicalUnitNumber().toString() - ); + if(lunMapResponse != null && lunMapResponse.getRecords() != null && !lunMapResponse.getRecords().isEmpty() + && lunMapResponse.getRecords().get(0).getLogicalUnitNumber() != null) { + lunNumber = lunMapResponse.getRecords().get(0).getLogicalUnitNumber().toString(); + + } else { + logger.error("enableLogicalAccess: Failed to fetch LunMap details for Lun: {} and igroup: {}. LunMap response is null or empty.", lunName, igroupName); + throw new CloudRuntimeException("Failed to fetch LunMap details for Lun: " + lunName + " and igroup: " + igroupName); + } } catch (Exception e) { logger.error("enableLogicalAccess: Failed to fetch LunMap details for Lun: {} and igroup: {}, Exception: {}", lunName, igroupName, e); throw new CloudRuntimeException("Failed to fetch LunMap details for Lun: " + lunName + " and igroup: " + igroupName); } - logger.debug("enableLogicalAccess: LunMap created successfully, LunMap: {}", lunMapResponse.getRecords().get(0)); - logger.info("enableLogicalAccess: LunMap created successfully."); + logger.trace("enableLogicalAccess: LunMap created successfully, LunMap: {}", lunMapResponse.getRecords().get(0)); } catch (Exception e) { logger.error("Exception occurred while creating LunMap", e); throw new CloudRuntimeException("Failed to create LunMap: " + e.getMessage()); } - return response; + return lunNumber; } public void disableLogicalAccess(Map values) { - logger.info("disableLogicalAccess : Delete LunMap"); - logger.debug("disableLogicalAccess : Deleting LunMap with values {} ", values); + logger.trace("disableLogicalAccess : Deleting LunMap with values {} ", values); if (values == null) { logger.error("disableLogicalAccess: LunMap deletion failed. Invalid request values: null"); throw new CloudRuntimeException(" Failed to delete LunMap, invalid request"); @@ -482,9 +479,10 @@ public void disableLogicalAccess(Map values) { } // GET-only helper: fetch LUN-map and return logical unit number if it exists; otherwise return null - public Map getLogicalAccess(Map values) { - logger.info("getLogicalAccess : Fetch LunMap"); - logger.debug("getLogicalAccess : Fetching LunMap with values {} ", values); + @Override + public String getLogicalAccess(Map values) { + String lunNumber = null; + logger.trace("getLogicalAccess : Fetching LunMap with values {} ", values); if (values == null) { logger.error("getLogicalAccess: Invalid request values: null"); throw new CloudRuntimeException(" Invalid request"); @@ -507,18 +505,17 @@ public Map getLogicalAccess(Map values) { )); if (lunMapResponse != null && lunMapResponse.getRecords() != null && !lunMapResponse.getRecords().isEmpty()) { Integer lunLogicalUnitNum = lunMapResponse.getRecords().get(0).getLogicalUnitNumber(); - String lunNumber = lunLogicalUnitNum != null ? lunLogicalUnitNum.toString() : null; - return lunNumber != null ? Map.of(OntapStorageConstants.LOGICAL_UNIT_NUMBER, lunNumber) : null; + lunNumber = lunLogicalUnitNum != null ? lunLogicalUnitNum.toString() : null; } } catch (Exception e) { logger.warn("getLogicalAccess: LunMap not found for Lun: {} and igroup: {} ({}).", lunName, igroupName, e.getMessage()); } - return null; + return lunNumber; } @Override public String ensureLunMapped(String svmName, String lunName, String accessGroupName) { - logger.info("ensureLunMapped: Ensuring LUN [{}] is mapped to igroup [{}] on SVM [{}]", lunName, accessGroupName, svmName); + logger.trace("ensureLunMapped: Ensuring LUN [{}] is mapped to igroup [{}] on SVM [{}]", lunName, accessGroupName, svmName); // Check existing map first Map getMap = Map.of( @@ -526,9 +523,8 @@ public String ensureLunMapped(String svmName, String lunName, String accessGroup OntapStorageConstants.SVM_DOT_NAME, svmName, OntapStorageConstants.IGROUP_DOT_NAME, accessGroupName ); - Map mapResp = getLogicalAccess(getMap); - if (mapResp != null && mapResp.containsKey(OntapStorageConstants.LOGICAL_UNIT_NUMBER)) { - String lunNumber = mapResp.get(OntapStorageConstants.LOGICAL_UNIT_NUMBER); + String lunNumber = getLogicalAccess(getMap); + if (lunNumber != null) { logger.info("ensureLunMapped: Existing LunMap found for LUN [{}] in igroup [{}] with LUN number [{}]", lunName, accessGroupName, lunNumber); return lunNumber; } @@ -539,12 +535,12 @@ public String ensureLunMapped(String svmName, String lunName, String accessGroup OntapStorageConstants.SVM_DOT_NAME, svmName, OntapStorageConstants.IGROUP_DOT_NAME, accessGroupName ); - Map response = enableLogicalAccess(enableMap); - if (response == null || !response.containsKey(OntapStorageConstants.LOGICAL_UNIT_NUMBER)) { + String response = enableLogicalAccess(enableMap); + if (response == null ) { throw new CloudRuntimeException("Failed to map LUN [" + lunName + "] to iGroup [" + accessGroupName + "]"); } - logger.info("ensureLunMapped: Successfully mapped LUN [{}] to igroup [{}] with LUN number [{}]", lunName, accessGroupName, response.get(OntapStorageConstants.LOGICAL_UNIT_NUMBER)); - return response.get(OntapStorageConstants.LOGICAL_UNIT_NUMBER); + logger.trace("ensureLunMapped: Successfully mapped LUN [{}] to igroup [{}] with LUN number [{}]", lunName, accessGroupName, response); + return response; } /** * Reverts a LUN to a snapshot using the ONTAP CLI-based snapshot file restore API. @@ -567,7 +563,7 @@ public String ensureLunMapped(String svmName, String lunName, String accessGroup public JobResponse revertSnapshotForCloudStackVolume(String snapshotName, String flexVolUuid, String snapshotUuid, String volumePath, String lunUuid, String flexVolName) { - logger.info("revertSnapshotForCloudStackVolume [iSCSI]: Restoring LUN [{}] from snapshot [{}] on FlexVol [{}]", + logger.trace("revertSnapshotForCloudStackVolume [iSCSI]: Restoring LUN [{}] from snapshot [{}] on FlexVol [{}]", volumePath, snapshotName, flexVolName); if (snapshotName == null || snapshotName.isEmpty()) { @@ -590,7 +586,7 @@ public JobResponse revertSnapshotForCloudStackVolume(String snapshotName, String CliSnapshotRestoreRequest restoreRequest = new CliSnapshotRestoreRequest( svmName, flexVolName, snapshotName, ontapLunPath); - logger.info("revertSnapshotForCloudStackVolume: Calling CLI file restore API with vserver={}, volume={}, snapshot={}, path={}", + logger.trace("revertSnapshotForCloudStackVolume: Calling CLI file restore API with vserver={}, volume={}, snapshot={}, path={}", svmName, flexVolName, snapshotName, ontapLunPath); return getSnapshotFeignClient().restoreFileFromSnapshotCli(authHeader, restoreRequest); diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java index b859f57b37b1..41b5aac40321 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java @@ -169,7 +169,7 @@ public AccessGroup getAccessGroup(Map values) { } @Override - public Map enableLogicalAccess(Map values) { + public String enableLogicalAccess(Map values) { return null; } @@ -178,7 +178,7 @@ public void disableLogicalAccess(Map values) { } @Override - public Map getLogicalAccess(Map values) { + public String getLogicalAccess(Map values) { return null; } } diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java index b3f2364656a7..1c0c84ef91dd 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java @@ -516,13 +516,11 @@ void testEnableLogicalAccess_Success() { when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())).thenReturn(response); // Execute - Map result = unifiedSANStrategy.enableLogicalAccess(values); + String result = unifiedSANStrategy.enableLogicalAccess(values); // Verify assertNotNull(result); - assertTrue(result.containsKey(OntapStorageConstants.LOGICAL_UNIT_NUMBER)); - assertEquals("0", result.get(OntapStorageConstants.LOGICAL_UNIT_NUMBER)); - + assertEquals("0", result); verify(sanFeignClient).createLunMap(eq(authHeader), eq(true), any(LunMap.class)); } } @@ -550,11 +548,11 @@ void testEnableLogicalAccess_AlreadyMapped_ReturnsLunNumber() { when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())).thenReturn(response); // Execute - Map result = unifiedSANStrategy.enableLogicalAccess(values); + String result = unifiedSANStrategy.enableLogicalAccess(values); // Verify assertNotNull(result); - assertEquals("5", result.get(OntapStorageConstants.LOGICAL_UNIT_NUMBER)); + assertEquals("5", result); } } @@ -621,11 +619,11 @@ void testGetLogicalAccess_Success() { when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())).thenReturn(response); // Execute - Map result = unifiedSANStrategy.getLogicalAccess(values); + String result = unifiedSANStrategy.getLogicalAccess(values); // Verify assertNotNull(result); - assertEquals("3", result.get(OntapStorageConstants.LOGICAL_UNIT_NUMBER)); + assertEquals("3", result); } } @@ -645,7 +643,7 @@ void testGetLogicalAccess_NotFound_ReturnsNull() { .thenThrow(new RuntimeException("Not found")); // Execute - Map result = unifiedSANStrategy.getLogicalAccess(values); + String result = unifiedSANStrategy.getLogicalAccess(values); // Verify assertNull(result); @@ -1671,7 +1669,7 @@ void testGetLogicalAccess_EmptyResponse_ReturnsNull() { when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())).thenReturn(emptyResponse); - Map result = unifiedSANStrategy.getLogicalAccess(values); + String result = unifiedSANStrategy.getLogicalAccess(values); assertNull(result); } @@ -1691,7 +1689,7 @@ void testGetLogicalAccess_ExceptionThrown_ReturnsNull() { when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())) .thenThrow(new RuntimeException("Connection failed")); - Map result = unifiedSANStrategy.getLogicalAccess(values); + String result = unifiedSANStrategy.getLogicalAccess(values); assertNull(result); }