CSTACKEX-46 Resolve bot review comments

Author: Gupta, Surya

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

@@ -166,17 +166,7 @@ private CloudStackVolume createCloudStackVolumeRequestByProtocol(StoragePoolVO s
             String lunFullName = Utility.getLunName(storagePool.getName(), volumeInfo.getName());
             lunRequest.setName(lunFullName);
 
-            String hypervisorType = storagePool.getHypervisor().name();
-            String osType = null;
-            switch (hypervisorType) {
-                case Constants.KVM:
-                    osType = Lun.OsTypeEnum.LINUX.getValue();
-                    break;
-                default:
-                    String errMsg = "createCloudStackVolume : Unsupported hypervisor type " + hypervisorType + " for ONTAP storage";
-                    s_logger.error(errMsg);
-                    throw new CloudRuntimeException(errMsg);
-            }
+            String osType = Utility.getOSTypeFromHypervisor(storagePool.getHypervisor().name());
             lunRequest.setOsType(Lun.OsTypeEnum.valueOf(osType));
 
             cloudStackVolumeRequest.setLun(lunRequest);
@@ -270,7 +260,7 @@ private void grantAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO,
             AccessGroup accessGroup = getAccessGroupByName(storageStrategy, svmName, accessGroupName);
             if(!hostInitiatorFoundInIgroup(host.getStorageUrl(), accessGroup.getIgroup())) {
                 s_logger.error("grantAccess: initiator [{}] is not present in iGroup [{}]", host.getStorageUrl(), accessGroupName);
-                throw new CloudRuntimeException("grantAccess: initiator [" + host.getStorageUrl() + "] is not present in iGroup [" + accessGroupName);
+                throw new CloudRuntimeException("grantAccess: initiator [" + host.getStorageUrl() + "] is not present in iGroup [" + accessGroupName + "]");
             }
 
             Map<String, String> enableLogicalAccessMap = new HashMap<>();

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java

@@ -275,8 +275,9 @@ public boolean attachCluster(DataStore dataStore, ClusterScope scope) {
         ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL));
         //TODO- Check if we have to handle heterogeneous host within the cluster
         if (!validateProtocolSupportAndFetchHostsIdentifier(hostsToConnect, protocol, hostsIdentifier)) {
-            s_logger.error("attachCluster: Not all hosts in the cluster support the protocol: " + protocol.name());
-            throw new CloudRuntimeException("attachCluster: Not all hosts in the cluster support the protocol: " + protocol.name());
+            String errMsg = "attachCluster: Not all hosts in the cluster support the protocol: " + protocol.name();
+            s_logger.error(errMsg);
+            throw new CloudRuntimeException(errMsg);
         }
         //TODO - check if no host to connect then also need to create access group without initiators
         if (hostsIdentifier != null && hostsIdentifier.size() > 0) {
@@ -329,8 +330,9 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper
         ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL));
         //TODO- Check if we have to handle heterogeneous host within the zone
         if (!validateProtocolSupportAndFetchHostsIdentifier(hostsToConnect, protocol, hostsIdentifier)) {
-            s_logger.error("attachZone: Not all hosts in the zone support the protocol: " + protocol.name());
-            throw new CloudRuntimeException("attachZone: Not all hosts in the zone support the protocol: " + protocol.name());
+            String errMsg = "attachZone: Not all hosts in the zone support the protocol: " + protocol.name();
+            s_logger.error(errMsg);
+            throw new CloudRuntimeException(errMsg);
         }
         if (hostsIdentifier != null && !hostsIdentifier.isEmpty()) {
             AccessGroup accessGroupRequest = createAccessGroupRequestByProtocol(storagePool, scope.getScopeId(), details, hostsIdentifier);

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

@@ -310,9 +310,10 @@ public Volume getStorageVolume(Volume volume)
 
     /**
      * Method encapsulates the behavior based on the opted protocol in subclasses
-     @@ -306,22 +306,22 @@ public AccessGroup getAccessGroup(Map<String, String> values)
-      *     getNameSpace    for Nvme/TCP and Nvme/FC protocols
-      * @param values
+     *     getIGroup       example getIgroup   for iSCSI and FC protocols
+     *     getExportPolicy example getExportPolicy for NFS 3.0 and NFS 4.1 protocols
+     *     //TODO  for Nvme/TCP and Nvme/FC protocols
+      * @param values map to get access group values like name, svm name etc.
      */
     abstract public AccessGroup getAccessGroup(Map<String, String> values);
 

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

@@ -197,9 +197,6 @@ public AccessGroup getAccessGroup(Map<String, String> values) {
                 throw new CloudRuntimeException("Failed to fetch Igroup");
             }
             Igroup igroup = igroupResponse.getRecords().get(0);
-            s_logger.debug("getAccessGroup: Igroup Details : {}", igroup);
-            s_logger.info("getAccessGroup: Fetched the Igroup successfully. IgroupName: {}", igroup.getName());
-
             AccessGroup accessGroup = new AccessGroup();
             accessGroup.setIgroup(igroup);
             return accessGroup;
@@ -226,7 +223,7 @@ public void enableLogicalAccess(Map<String, String> values) {
             OntapResponse<LunMap> createdLunMap = sanFeignClient.createLunMap(authHeader, true, lunMapRequest);
             if (createdLunMap == null || createdLunMap.getRecords() == null || createdLunMap.getRecords().size() == 0) {
                 s_logger.error("enableLogicalAccess: LunMap failed for Lun: {} and igroup: {}", lunName, igroupName);
-                throw new CloudRuntimeException("Failed to perform LunMap for Lun: " +lunName+ " and igroup: " + igroupName);
+                throw new CloudRuntimeException("Failed to perform LunMap for Lun: " + lunName + " and igroup: " + igroupName);
             }
             LunMap lunMap = createdLunMap.getRecords().get(0);
             s_logger.debug("enableLogicalAccess: LunMap created successfully, LunMap: {}", lunMap);