CSTACKEX-191: addressing copilot comments

Author: rajiv-jain-netapp

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

@@ -433,7 +433,7 @@ private void grantAccessIscsi(Host host, VolumeVO volumeVO, Map<String, String>
         }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
+              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

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

@@ -593,7 +593,7 @@ 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 String enableLogicalAccess(Map<String,String> values);
 
@@ -608,7 +608,7 @@ 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 String getLogicalAccess(Map<String, String> values);
 

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

@@ -377,6 +377,7 @@ public AccessGroup getAccessGroup(Map<String, String> values) {
         }
     }
 
+    @Override
     public String enableLogicalAccess(Map<String, String> values) {
         logger.trace("enableLogicalAccess : Creating LunMap with values {} ", values);
         String lunNumber = null;
@@ -478,6 +479,7 @@ public void disableLogicalAccess(Map<String, String> values) {
     }
 
     // GET-only helper: fetch LUN-map and return logical unit number if it exists; otherwise return null
+    @Override
     public String getLogicalAccess(Map<String, String> values) {
         String lunNumber = null;
         logger.trace("getLogicalAccess : Fetching LunMap with values {} ", values);

plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java

@@ -520,7 +520,7 @@ void testEnableLogicalAccess_Success() {
 
             // Verify
             assertNotNull(result);
-
+            assertEquals("0", result);
             verify(sanFeignClient).createLunMap(eq(authHeader), eq(true), any(LunMap.class));
         }
     }
@@ -552,6 +552,7 @@ void testEnableLogicalAccess_AlreadyMapped_ReturnsLunNumber() {
 
             // Verify
             assertNotNull(result);
+            assertEquals("5", result);
         }
     }