"Update code as per coding principle"

Author: Srivastava, Piyush

plugins/storage/volume/ontap/pom.xml

@@ -24,7 +24,7 @@
     <parent>
         <groupId>org.apache.cloudstack</groupId>
         <artifactId>cloudstack-plugins</artifactId>
-        <version>4.22.0.0-SNAPSHOT</version>
+        <version>4.23.0.0-SNAPSHOT</version>
         <relativePath>../../../pom.xml</relativePath>
     </parent>
     <properties>

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

@@ -144,16 +144,9 @@ public Object decode(Response response, Type type) throws IOException, DecodeExc
                 try (InputStream bodyStream = response.body().asInputStream()) {
                     json = new String(bodyStream.readAllBytes(), StandardCharsets.UTF_8);
                     logger.debug("Decoding JSON response: {}", json);
-                    logger.debug("Target type: {}", type);
-                    logger.debug("About to call jsonMapper.readValue()...");
-
                     Object result = null;
                     try {
-                        logger.debug("Calling jsonMapper.constructType()...");
                         var javaType = jsonMapper.getTypeFactory().constructType(type);
-                        logger.debug("constructType() returned: {}", javaType);
-
-                        logger.debug("Calling jsonMapper.readValue() with json and javaType...");
                         result = jsonMapper.readValue(json, javaType);
                         logger.debug("jsonMapper.readValue() completed successfully");
                     } catch (Throwable ex) {

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/FeignConfiguration.java

@@ -192,12 +192,7 @@ public DataStore initialize(Map<String, Object> dsInfos) {
         ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL));
         switch (protocol) {
             case NFS:
-                // Use NetworkFilesystem (not ManagedNFS) with managed=true
-                // This routes to LibvirtStorageAdaptor which has full qemu-img support
-                // Same pattern as CloudByte/Elastistor plugin
                 parameters.setType(Storage.StoragePoolType.NetworkFilesystem);
-                // Path should be just the NFS export path (junction path), NOT host:path
-                // CloudStack will construct the full mount path as: hostAddress + ":" + path
                 path = "/" + storagePoolName;
                 s_logger.info("Setting NFS path for storage pool: " + path);
                 host = "10.193.192.136"; // TODO hardcoded for now
@@ -233,9 +228,7 @@ public DataStore initialize(Map<String, Object> dsInfos) {
                     s_logger.error("createStorageVolume returned null for volume: " + storagePoolName);
                     throw new CloudRuntimeException("Failed to create ONTAP volume: " + storagePoolName);
                 }
-
                 s_logger.info("Volume object retrieved successfully. UUID: " + volume.getUuid() + ", Name: " + volume.getName());
-
                 details.putIfAbsent(Constants.VOLUME_UUID, volume.getUuid());
                 details.putIfAbsent(Constants.VOLUME_NAME, volume.getName());
             } catch (Exception e) {
@@ -246,10 +239,6 @@ public DataStore initialize(Map<String, Object> dsInfos) {
             throw new CloudRuntimeException("ONTAP details validation failed, cannot create primary storage");
         }
 
-        // Add mountpoint detail for ManagedNFS - required by KVM agent's ManagedNfsStorageAdaptor
-        // The 'mountpoint' key is used by connectPhysicalDisk() to mount NFS export
-        details.put("mountpoint", path);
-
         // Set parameters for primary data store
         parameters.setPort(Constants.ONTAP_PORT);
         parameters.setHost(host);
@@ -293,7 +282,6 @@ public boolean attachCluster(DataStore dataStore, ClusterScope scope) {
 
         logger.debug("attachCluster: Attaching the pool to each of the host in the cluster: {}", primaryStore.getClusterId());
         for (HostVO host : hostsToConnect) {
-            // TODO: Fetch the host IQN and add to the initiator group on ONTAP cluster
             try {
                 _storageMgr.connectHostToSharedPool(host, dataStore.getId());
             } catch (Exception e) {
@@ -330,7 +318,6 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper
         strategy.createAccessGroup(accessGroupRequest);
 
         for (HostVO host : hostsToConnect) {
-            // TODO: Fetch the host IQN and add to the initiator group on ONTAP cluster
             try {
                 _storageMgr.connectHostToSharedPool(host, dataStore.getId());
             } catch (Exception e) {

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

@@ -38,10 +38,6 @@
 import org.apache.cloudstack.engine.subsystem.api.storage.HypervisorHostListener;
 import com.cloud.host.dao.HostDao;
 
-/**
- * HypervisorHostListener implementation for ONTAP storage.
- * Handles connecting/disconnecting hosts to/from ONTAP-backed storage pools.
- */
 public class OntapHostListener implements HypervisorHostListener {
     protected Logger logger = LogManager.getLogger(getClass());
 

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/listener/OntapHostListener.java

@@ -150,23 +150,15 @@ public Volume createStorageVolume(String volumeName, Long size) {
         Volume volumeRequest = new Volume();
         Svm svm = new Svm();
         svm.setName(svmName);
-
         Nas nas = new Nas();
-        nas.setPath("/" + volumeName);
+        nas.setPath(Constants.PATH_SEPARATOR + volumeName);
 
         volumeRequest.setName(volumeName);
         volumeRequest.setSvm(svm);
         volumeRequest.setAggregates(aggregates);
         volumeRequest.setSize(size);
-        volumeRequest.setNas(nas); // be default if we don't set path , ONTAP create a volume with mount/junction path // TODO check if we need to append svm name or not
-        // since storage pool also cannot be duplicate so junction path can also be not duplicate so /volumeName will always be unique
-        // Make the POST API call to create the volume
+        volumeRequest.setNas(nas);
         try {
-            /*
-              ONTAP created a default rule of 0.0.0.0 if no export rule are defined while creating volume
-              and since in storage pool creation, cloudstack is not aware of the host , we can either create default or
-              permissive rule and later update it as part of attachCluster or attachZone implementation
-             */
             JobResponse jobResponse = volumeFeignClient.createVolumeWithJob(authHeader, volumeRequest);
             if (jobResponse == null || jobResponse.getJob() == null) {
                 throw new CloudRuntimeException("Failed to initiate volume creation for " + volumeName);
@@ -201,7 +193,6 @@ public Volume createStorageVolume(String volumeName, Long size) {
             throw new CloudRuntimeException("Failed to create volume: " + e.getMessage());
         }
         s_logger.info("Volume created successfully: " + volumeName);
-        // Below code is to update volume uuid to storage pool mapping once and used for all other workflow saving get volume call
         try {
             Map<String, Object> queryParams = Map.of(Constants.NAME, volumeName);
             s_logger.debug("Fetching volume details for: " + volumeName);