CSTACKEX-7: ONTAP Primary storage pool by sandeeplocharla · Pull Request #9 · NetApp/cloudstack · GitHub

Description

This PR...

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)
Build/CI
Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

BLOCKER
Critical
Major
Minor
Trivial

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

we kept this naming convention for other feign client and this can return only one record also

Reverted it back

svm is an object which can not be string use svmName instead

sure, will rename it

Can we have type as enum with supported protocols?

Done

we can combine this check into one check bcz both are required, also clusterId can be empty

Yeah, in my previous review to this change i nanother PR, I gave comment to use trim command and then empty check. Please add them for each mandatory parameter

Taken care of it, done

why this check is required again? it is already checked above

Customer has an option to not send both cluster and pod ids, in case of creating a zone level primary storage pool.

add _ for ISDISAGGREGATED constant like IS_DISAGGREGATED

done

add a check for svms null check object

done

add _ where ever possible

done

add null checks

done, thanks

instead of checking for all state of job, check for not success

this seems to be lost during rebase, thanks, done

Instead of getting all svm and picking based on name, better to call getSVMByName. We may need to add this method in svmFeignClient

Yes, this approach is not optimized. fetch the SVM using name as query parameter instead fetch all then find for desired name.

Made the necessary changes

with this change, why do we need to add url parameter at class level annotation?

If we don't pass it explicitly, we need to set a property statically in application.properties or somewhere like that, which in our case is not feasible. Also, user might ask us to deal with multiple filers, so, the IP wouldn't be static

I am not seeing any significant changes here, better remove this class from the PR

This came in as part of rebase. It just has some space changes. Sure, will try to revert it

Yeah, in my previous review to this change i nanother PR, I gave comment to use trim command and then empty check. Please add them for each mandatory parameter

Yes, this approach is not optimized. fetch the SVM using name as query parameter instead fetch all then find for desired name.

change the message

done

Follow naming convention for all fields

…iable names

why you added only inports in this class ?

Lets use camel casing for the variable name

empty check on the sting parameters must use trim method as well

One use case to validate here is that if the passed credentials belong to the cluster, then by default, all the aggregates are mapped to the underlying SVMs. However, if credentials belong to an SVM user, then aggregates must be assigned to the SVM to access them.

@suryag1201 can you help @sandeeplocharla to close on this use case implementation.

there are dependencies on these changes for other engineers, approving this PR for now. Ensure you are picking up the given comments in subsequent PR

# This is the 1st commit message: NFS Cloudstack volume and export policy utils # This is the commit message #2: Licencse add in files # This is the commit message #3: accessgroup create recode # This is the commit message #4: creatacessgroup for NFS impl # This is the commit message #5: storage pool mounting on host # This is the commit message #6: storage pool mounting on host 1 # This is the commit message #7: vm restart issue # This is the commit message #8: vm restart issue 1 # This is the commit message #9: vm restart issue 2 # This is the commit message #10: vm instance creation test1 # This is the commit message #11: vm instance creation test4

* CSTACKEX-7: ONTAP Primary storage pool --------- Co-authored-by: Locharla, Sandeep <Sandeep.Locharla@netapp.com>

sandeeplocharla self-assigned this Oct 21, 2025

sandeeplocharla requested review from piyush5netapp, rajiv-jain-netapp and suryag1201 October 21, 2025 05:53

suryag1201 reviewed Oct 22, 2025

View reviewed changes

rajiv-jain-netapp reviewed Oct 22, 2025

View reviewed changes

Locharla, Sandeep added 2 commits October 22, 2025 20:08

CSTACKEX-7: ONTAP Primary storage pool

9547f02

CSTACKEX-7: Addressed review comments

a9c7f65

sandeeplocharla force-pushed the feature/CSTACKEX-7 branch from 1573d0c to a9c7f65 Compare October 22, 2025 14:38

sandeeplocharla commented Oct 23, 2025

View reviewed changes

CSTACKEX-7: Addressed few more comments

24d3275

suryag1201 reviewed Oct 23, 2025

View reviewed changes

CSTACKEX-7: Made changes to correct an exception message and some var…

2274e60

…iable names

rajiv-jain-netapp reviewed Oct 24, 2025

View reviewed changes

sandeeplocharla requested review from rajiv-jain-netapp and suryag1201 October 24, 2025 08:09

piyush5netapp approved these changes Oct 24, 2025

View reviewed changes

rajiv-jain-netapp approved these changes Oct 24, 2025

View reviewed changes

sandeeplocharla merged commit 25353c2 into main Oct 24, 2025
13 of 16 checks passed

sandeeplocharla added a commit that referenced this pull request Jan 30, 2026

CSTACKEX-7: ONTAP Primary storage pool (#9 )

fc84a4f

* CSTACKEX-7: ONTAP Primary storage pool --------- Co-authored-by: Locharla, Sandeep <Sandeep.Locharla@netapp.com>

rajiv-jain-netapp pushed a commit that referenced this pull request Apr 16, 2026

CSTACKEX-7: ONTAP Primary storage pool (#9 )

74b1962

* CSTACKEX-7: ONTAP Primary storage pool --------- Co-authored-by: Locharla, Sandeep <Sandeep.Locharla@netapp.com>

                   //this method to get all svms and also filtered svms based on query params as a part of URL
                   @RequestMapping(method = RequestMethod.GET)
-                  OntapResponse<Svm> getSvmResponse(URI baseURL, @RequestHeader("Authorization") String header);
+                  OntapResponse<Svm> getSvms(URI baseURL, @RequestHeader("Authorization") String header);

+                  public static String Username;
+                  public static String Password;
+                  public static String ManagementLIF;
+                  public static String Svm;

+                  public static String Password;
+                  public static String ManagementLIF;
+                  public static String Svm;
+                  public static String Protocol;

+                      @SuppressWarnings("unchecked")
+                      Map<String, String> details = (Map<String, String>)dsInfos.get("details");
+                      // Validations
+                      if (podId != null && clusterId == null) {

+                          // Call the SVM API to check if the SVM exists
+                          Svm svm = null;
+                          URI url = URI.create(Constants.HTTPS + storage.getManagementLIF() + Constants.GETSVMs);
+                          OntapResponse<Svm> svms = svmFeignClient.getSvms(url, authHeader);

+                      }
+                      PrimaryDataStoreParameters parameters = new PrimaryDataStoreParameters();
+                      if (clusterId != null) {

+                      details.put(Constants.MANAGEMENTLIF, url);
+                      // Validate the ONTAP details
+                      if(details.get(Constants.ISDISAGGREGATED) == null || details.get(Constants.ISDISAGGREGATED).isEmpty()) {

+              public class Constants {
+                  public static final String NFS = "nfs";
+                  public static final String ISCSI = "iscsi";
+                  public static final String PROTOCOL = "protocol";

+                          // Create URI for POST CreateVolume API
+                          URI url = utils.generateURI(Constants.CREATEVOLUME);
+                          // Call the VolumeFeignClient to create the volume
+                          JobResponse jobResponse = volumeFeignClient.createVolumeWithJob(url, authHeader, volumeRequest);

+                          url = utils.generateURI(Constants.GETJOBBYUUID);
+                          int jobRetryCount = 0, maxJobRetries = Constants.JOBMAXRETRIES;
+                          Job createVolumeJob = null;
+                          while(createVolumeJob == null || createVolumeJob.getState().equals(Constants.JOBRUNNING) || createVolumeJob.getState().equals(Constants.JOBQUEUE) || createVolumeJob.getState().equals(Constants.JOBPAUSED)) {

                   @RequestMapping(method = RequestMethod.DELETE, value="/{uuid}")
-                  void deleteVolume(@RequestHeader("Authorization") String authHeader, @PathVariable("uuid") String uuid);
+                  void deleteVolume(URI baseURL, @RequestHeader("Authorization") String authHeader, @PathVariable("uuid") String uuid);

+                          ClusterVO clusterVO = _clusterDao.findById(clusterId);
+                          Preconditions.checkNotNull(clusterVO, "Unable to locate the specified cluster");
+                          if (clusterVO.getHypervisorType() != Hypervisor.HypervisorType.KVM) {
+                              throw new CloudRuntimeException("ONTAP primary storage is not supported for KVM hypervisor");

+              import org.apache.cloudstack.storage.utils.Constants.ProtocolType;
+              public class OntapStorage {
+                  public static String Username;

+              public class OntapStorage {
+                  public static String _username;
+                  public static String _password;
+                  public static String _managementLIF;

+                          }
+                      }
+                      if (storagePoolName == null || storagePoolName.isEmpty()) {

+                              throw new CloudRuntimeException("iSCSI protocol is not enabled on SVM " + svmName);
+                          }
+                          List<Aggregate> aggrs = svm.getAggregates();
+                          if (aggrs == null || aggrs.isEmpty()) {

CSTACKEX-7: ONTAP Primary storage pool by sandeeplocharla · Pull Request #9 · NetApp/cloudstack · GitHub

Uh oh!

CSTACKEX-7: ONTAP Primary storage pool - #9

CSTACKEX-7: ONTAP Primary storage pool#9sandeeplocharla merged 4 commits intomainNetApp/cloudstack:mainfrom feature/CSTACKEX-7NetApp/cloudstack:feature/CSTACKEX-7Copy head branch name to clipboard

Conversation

sandeeplocharla commented Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

suryag1201 Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

suryag1201 Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

suryag1201 Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CSTACKEX-7: ONTAP Primary storage pool#9
sandeeplocharla merged 4 commits into
mainfrom
feature/CSTACKEX-7

sandeeplocharla commented Oct 21, 2025 •
edited
Loading

suryag1201 Oct 22, 2025 •
edited
Loading

suryag1201 Oct 22, 2025 •
edited
Loading

suryag1201 Oct 22, 2025 •
edited
Loading

suryag1201 Oct 23, 2025 •
edited
Loading