CSTACKEX-34: further review comments incorporations

rajiv-jain-netapp · rajiv-jain-netapp · commit 465fffedcf18 · 2025-10-28T15:18:02.000+05:30
diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/OntapStorage.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/OntapStorage.java
@@ -19,14 +19,15 @@
 
 package org.apache.cloudstack.storage.feign.model;
 
-import org.apache.cloudstack.storage.utils.Constants.ProtocolType;
+
+import org.apache.cloudstack.storage.service.model.ProtocolType;
 
 public class OntapStorage {
     private  String username;
     private  String password;
     private  String managementLIF;
     private  String svmName;
-    private  ProtocolType protocolType;
+    private ProtocolType protocolType;
     private  Boolean isDisaggregated;
 
     public OntapStorage(String username, String password, String managementLIF, String svmName, ProtocolType protocolType, Boolean isDisaggregated) {
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
@@ -42,8 +42,8 @@
 import org.apache.cloudstack.storage.feign.model.OntapStorage;
 import org.apache.cloudstack.storage.provider.StorageProviderFactory;
 import org.apache.cloudstack.storage.service.StorageStrategy;
+import org.apache.cloudstack.storage.service.model.ProtocolType;
 import org.apache.cloudstack.storage.utils.Constants;
-import org.apache.cloudstack.storage.utils.Constants.ProtocolType;
 import org.apache.cloudstack.storage.volume.datastore.PrimaryDataStoreHelper;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/provider/StorageProviderFactory.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/provider/StorageProviderFactory.java
@@ -24,7 +24,7 @@
 import org.apache.cloudstack.storage.service.StorageStrategy;
 import org.apache.cloudstack.storage.service.UnifiedNASStrategy;
 import org.apache.cloudstack.storage.service.UnifiedSANStrategy;
-import org.apache.cloudstack.storage.utils.Constants.ProtocolType;
+import org.apache.cloudstack.storage.service.model.ProtocolType;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.springframework.stereotype.Component;
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
@@ -73,14 +73,6 @@ public abstract class StorageStrategy {
 
     private static final Logger s_logger = (Logger) LogManager.getLogger(StorageStrategy.class);
 
-    protected enum PROTOCOLS
-          { NFS30,
-            NFS41,
-            FC,
-            iSCSI,
-            NvmeTCP,
-            NvmeFC };
-
     public StorageStrategy(OntapStorage ontapStorage) {
         storage = ontapStorage;
 
@@ -209,7 +201,7 @@ public Volume createStorageVolume(String volumeName, Long size) {
      *
      * @param values
      */
-    public Volume updateStorageVolume(Map<String,String> values)
+    public Volume updateStorageVolume(Volume volume)
     {
         //TODO
         return null;
@@ -222,7 +214,7 @@ public Volume updateStorageVolume(Map<String,String> values)
      *
      * @param values
      */
-    public void deleteStorageVolume(Map<String,String> values)
+    public void deleteStorageVolume(Volume volume)
     {
         //TODO
     }
@@ -234,7 +226,7 @@ public void deleteStorageVolume(Map<String,String> values)
      *
      * @param values
      */
-    public Volume getStorageVolume(Map<String,String> values)
+    public Volume getStorageVolume(Volume volume)
     {
         //TODO
         return null;
@@ -248,7 +240,7 @@ public Volume getStorageVolume(Map<String,String> values)
      *     createNameSpace for Nvme/TCP and Nvme/FC protocol
      * @param values
      */
-    abstract public CloudStackVolume createCloudStackVolume(Map<String,String> values);
+    abstract public CloudStackVolume createCloudStackVolume(CloudStackVolume cloudstackVolume);
 
     /**
      * Method encapsulates the behavior based on the opted protocol in subclasses.
@@ -258,7 +250,7 @@ public Volume getStorageVolume(Map<String,String> values)
      *     updateNameSpace for Nvme/TCP and Nvme/FC protocol
      * @param values
      */
-    abstract CloudStackVolume updateCloudStackVolume(Map<String,String> values);
+    abstract CloudStackVolume updateCloudStackVolume(CloudStackVolume cloudstackVolume);
 
     /**
      * Method encapsulates the behavior based on the opted protocol in subclasses.
@@ -268,7 +260,7 @@ public Volume getStorageVolume(Map<String,String> values)
      *     deleteNameSpace for Nvme/TCP and Nvme/FC protocol
      * @param values
      */
-    abstract void deleteCloudStackVolume(Map<String,String> values);
+    abstract void deleteCloudStackVolume(CloudStackVolume cloudstackVolume);
 
     /**
      * Method encapsulates the behavior based on the opted protocol in subclasses.
@@ -278,7 +270,7 @@ public Volume getStorageVolume(Map<String,String> values)
      *     getNameSpace for Nvme/TCP and Nvme/FC protocol
      * @param values
      */
-    abstract CloudStackVolume getCloudStackVolume(Map<String,String> values);
+    abstract CloudStackVolume getCloudStackVolume(CloudStackVolume cloudstackVolume);
 
     /**
      * Method encapsulates the behavior based on the opted protocol in subclasses
@@ -287,7 +279,7 @@ public Volume getStorageVolume(Map<String,String> values)
      *     createSubsystem    for Nvme/TCP and Nvme/FC protocols
      * @param values
      */
-    abstract AccessGroup createAccessGroup(Map<String,String> values);
+    abstract AccessGroup createAccessGroup(AccessGroup accessGroup);
 
     /**
      * Method encapsulates the behavior based on the opted protocol in subclasses
@@ -296,7 +288,7 @@ public Volume getStorageVolume(Map<String,String> values)
      *     deleteSubsystem    for Nvme/TCP and Nvme/FC protocols
      * @param values
      */
-    abstract void deleteAccessGroup(Map<String,String> values);
+    abstract void deleteAccessGroup(AccessGroup accessGroup);
 
     /**
      * Method encapsulates the behavior based on the opted protocol in subclasses
@@ -305,7 +297,7 @@ public Volume getStorageVolume(Map<String,String> values)
      *     //TODO  for Nvme/TCP and Nvme/FC protocols
      * @param values
      */
-    abstract AccessGroup updateAccessGroup(Map<String,String> values);
+    abstract AccessGroup updateAccessGroup(AccessGroup accessGroup);
 
     /**
      * Method encapsulates the behavior based on the opted protocol in subclasses
@@ -314,7 +306,7 @@ public Volume getStorageVolume(Map<String,String> values)
      *     getNameSpace    for Nvme/TCP and Nvme/FC protocols
      * @param values
      */
-    abstract AccessGroup getAccessGroup(Map<String,String> values);
+    abstract AccessGroup getAccessGroup(AccessGroup accessGroup);
 
     /**
      * Method encapsulates the behavior based on the opted protocol in subclasses
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
@@ -31,47 +31,47 @@ public UnifiedNASStrategy(OntapStorage ontapStorage) {
     }
 
     @Override
-    public CloudStackVolume createCloudStackVolume(Map<String, String> values) {
+    public CloudStackVolume createCloudStackVolume(CloudStackVolume cloudstackVolume) {
         //TODO
         return null;
     }
 
     @Override
-    public CloudStackVolume updateCloudStackVolume(Map<String, String> values) {
+    public CloudStackVolume updateCloudStackVolume(CloudStackVolume cloudstackVolume) {
         //TODO
         return null;
     }
 
     @Override
-    public void deleteCloudStackVolume(Map<String, String> values) {
+    public void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) {
 
     }
 
     @Override
-    public CloudStackVolume getCloudStackVolume(Map<String, String> values) {
+    public CloudStackVolume getCloudStackVolume(CloudStackVolume cloudstackVolume) {
         //TODO
         return null;
     }
 
     @Override
-    public AccessGroup createAccessGroup(Map<String, String> values) {
+    public AccessGroup createAccessGroup(AccessGroup accessGroup) {
         //TODO
         return null;
     }
 
     @Override
-    public void deleteAccessGroup(Map<String, String> values) {
+    public void deleteAccessGroup(AccessGroup accessGroup) {
 
     }
 
     @Override
-    public AccessGroup updateAccessGroup(Map<String, String> values) {
+    public AccessGroup updateAccessGroup(AccessGroup accessGroup) {
         //TODO
         return null;
     }
 
     @Override
-    public AccessGroup getAccessGroup(Map<String, String> values) {
+    public AccessGroup getAccessGroup(AccessGroup accessGroup) {
         //TODO
         return null;
     }
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
@@ -31,47 +31,47 @@ public UnifiedSANStrategy(OntapStorage ontapStorage) {
     }
 
     @Override
-    public CloudStackVolume createCloudStackVolume(Map<String, String> values) {
+    public CloudStackVolume createCloudStackVolume(CloudStackVolume cloudstackVolume) {
         //TODO
         return null;
     }
 
     @Override
-    CloudStackVolume updateCloudStackVolume(Map<String, String> values) {
+    CloudStackVolume updateCloudStackVolume(CloudStackVolume cloudstackVolume) {
         //TODO
         return null;
     }
 
     @Override
-    void deleteCloudStackVolume(Map<String, String> values) {
+    void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) {
 
     }
 
     @Override
-    CloudStackVolume getCloudStackVolume(Map<String, String> values) {
+    CloudStackVolume getCloudStackVolume(CloudStackVolume cloudstackVolume) {
         //TODO
         return null;
     }
 
     @Override
-    public AccessGroup createAccessGroup(Map<String, String> values) {
+    public AccessGroup createAccessGroup(AccessGroup accessGroup) {
         //TODO
         return null;
     }
 
     @Override
-    public void deleteAccessGroup(Map<String, String> values) {
+    public void deleteAccessGroup(AccessGroup accessGroup) {
 
     }
 
     @Override
-    public AccessGroup updateAccessGroup(Map<String, String> values) {
+    public AccessGroup updateAccessGroup(AccessGroup accessGroup) {
         //TODO
         return null;
     }
 
     @Override
-    public AccessGroup getAccessGroup(Map<String, String> values) {
+    public AccessGroup getAccessGroup(AccessGroup accessGroup) {
         //TODO
         return null;
     }
diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/AccessGroup.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/AccessGroup.java
@@ -1,12 +1,12 @@
 package org.apache.cloudstack.storage.service.model;
 
+import org.apache.cloudstack.storage.feign.model.ExportPolicy;
 import org.apache.cloudstack.storage.feign.model.Igroup;
-import org.apache.cloudstack.storage.feign.model.Policy;
 
 public class AccessGroup {
 
-    Igroup igroup;
-    Policy policy;
+    private Igroup igroup;
+    private ExportPolicy exportPolicy;
 
     public Igroup getIgroup() {
         return igroup;
@@ -16,11 +16,11 @@ public void setIgroup(Igroup igroup) {
         this.igroup = igroup;
     }
 
-    public Policy getPolicy() {
-        return policy;
+    public ExportPolicy getPolicy() {
+        return exportPolicy;
     }
 
-    public void setPolicy(Policy policy) {
-        this.policy = policy;
+    public void setPolicy(ExportPolicy policy) {
+        this.exportPolicy = policy;
     }
 }
diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/CloudStackVolume.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/CloudStackVolume.java
@@ -5,8 +5,8 @@
 
 public class CloudStackVolume {
 
-    FileInfo file;
-    Lun lun;
+    private FileInfo file;
+    private Lun lun;
 
     public FileInfo getFile() {
         return file;
diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/ProtocolType.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/ProtocolType.java
@@ -0,0 +1,6 @@
+package org.apache.cloudstack.storage.service.model;
+
+public enum ProtocolType {
+        NFS,
+        ISCSI
+}
diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Constants.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Constants.java
@@ -20,10 +20,6 @@
 package org.apache.cloudstack.storage.utils;
 
 public class Constants {
-    public enum ProtocolType {
-        NFS,
-        ISCSI
-    }
 
     public static final String NFS = "nfs";
     public static final String ISCSI = "iscsi";

Original file line number	Diff line number	Diff line change
`@@ -31,47 +31,47 @@ public UnifiedNASStrategy(OntapStorage ontapStorage) {`
`31`	`31`	`}`
`32`	`32`
`33`	`33`	`@Override`
`34`		`- public CloudStackVolume createCloudStackVolume(Map<String, String> values) {`
	`34`	`+ public CloudStackVolume createCloudStackVolume(CloudStackVolume cloudstackVolume) {`
`35`	`35`	`//TODO`
`36`	`36`	`return null;`
`37`	`37`	`}`
`38`	`38`
`39`	`39`	`@Override`
`40`		`- public CloudStackVolume updateCloudStackVolume(Map<String, String> values) {`
	`40`	`+ public CloudStackVolume updateCloudStackVolume(CloudStackVolume cloudstackVolume) {`
`41`	`41`	`//TODO`
`42`	`42`	`return null;`
`43`	`43`	`}`
`44`	`44`
`45`	`45`	`@Override`
`46`		`- public void deleteCloudStackVolume(Map<String, String> values) {`
	`46`	`+ public void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) {`
`47`	`47`
`48`	`48`	`}`
`49`	`49`
`50`	`50`	`@Override`
`51`		`- public CloudStackVolume getCloudStackVolume(Map<String, String> values) {`
	`51`	`+ public CloudStackVolume getCloudStackVolume(CloudStackVolume cloudstackVolume) {`
`52`	`52`	`//TODO`
`53`	`53`	`return null;`
`54`	`54`	`}`
`55`	`55`
`56`	`56`	`@Override`
`57`		`- public AccessGroup createAccessGroup(Map<String, String> values) {`
	`57`	`+ public AccessGroup createAccessGroup(AccessGroup accessGroup) {`
`58`	`58`	`//TODO`
`59`	`59`	`return null;`
`60`	`60`	`}`
`61`	`61`
`62`	`62`	`@Override`
`63`		`- public void deleteAccessGroup(Map<String, String> values) {`
	`63`	`+ public void deleteAccessGroup(AccessGroup accessGroup) {`
`64`	`64`
`65`	`65`	`}`
`66`	`66`
`67`	`67`	`@Override`
`68`		`- public AccessGroup updateAccessGroup(Map<String, String> values) {`
	`68`	`+ public AccessGroup updateAccessGroup(AccessGroup accessGroup) {`
`69`	`69`	`//TODO`
`70`	`70`	`return null;`
`71`	`71`	`}`
`72`	`72`
`73`	`73`	`@Override`
`74`		`- public AccessGroup getAccessGroup(Map<String, String> values) {`
	`74`	`+ public AccessGroup getAccessGroup(AccessGroup accessGroup) {`
`75`	`75`	`//TODO`
`76`	`76`	`return null;`
`77`	`77`	`}`
Original file line number	Diff line number	Diff line change
`@@ -31,47 +31,47 @@ public UnifiedSANStrategy(OntapStorage ontapStorage) {`
`31`	`31`	`}`
`32`	`32`
`33`	`33`	`@Override`
`34`		`- public CloudStackVolume createCloudStackVolume(Map<String, String> values) {`
	`34`	`+ public CloudStackVolume createCloudStackVolume(CloudStackVolume cloudstackVolume) {`
`35`	`35`	`//TODO`
`36`	`36`	`return null;`
`37`	`37`	`}`
`38`	`38`
`39`	`39`	`@Override`
`40`		`- CloudStackVolume updateCloudStackVolume(Map<String, String> values) {`
	`40`	`+ CloudStackVolume updateCloudStackVolume(CloudStackVolume cloudstackVolume) {`
`41`	`41`	`//TODO`
`42`	`42`	`return null;`
`43`	`43`	`}`
`44`	`44`
`45`	`45`	`@Override`
`46`		`- void deleteCloudStackVolume(Map<String, String> values) {`
	`46`	`+ void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) {`
`47`	`47`
`48`	`48`	`}`
`49`	`49`
`50`	`50`	`@Override`
`51`		`- CloudStackVolume getCloudStackVolume(Map<String, String> values) {`
	`51`	`+ CloudStackVolume getCloudStackVolume(CloudStackVolume cloudstackVolume) {`
`52`	`52`	`//TODO`
`53`	`53`	`return null;`
`54`	`54`	`}`
`55`	`55`
`56`	`56`	`@Override`
`57`		`- public AccessGroup createAccessGroup(Map<String, String> values) {`
	`57`	`+ public AccessGroup createAccessGroup(AccessGroup accessGroup) {`
`58`	`58`	`//TODO`
`59`	`59`	`return null;`
`60`	`60`	`}`
`61`	`61`
`62`	`62`	`@Override`
`63`		`- public void deleteAccessGroup(Map<String, String> values) {`
	`63`	`+ public void deleteAccessGroup(AccessGroup accessGroup) {`
`64`	`64`
`65`	`65`	`}`
`66`	`66`
`67`	`67`	`@Override`
`68`		`- public AccessGroup updateAccessGroup(Map<String, String> values) {`
	`68`	`+ public AccessGroup updateAccessGroup(AccessGroup accessGroup) {`
`69`	`69`	`//TODO`
`70`	`70`	`return null;`
`71`	`71`	`}`
`72`	`72`
`73`	`73`	`@Override`
`74`		`- public AccessGroup getAccessGroup(Map<String, String> values) {`
	`74`	`+ public AccessGroup getAccessGroup(AccessGroup accessGroup) {`
`75`	`75`	`//TODO`
`76`	`76`	`return null;`
`77`	`77`	`}`
-Original file line number
+Diff line change
@@ @@ -0,0 +1,6 @@ @@
 +package org.apache.cloudstack.storage.service.model;
++
 +public enum ProtocolType {
 +        NFS,
 +        ISCSI
 +}