feature/CSTACKEX-46: Create, Delete iSCSI type Cloudstack volumes, Enter, Cancel Maintenance mode by sandeeplocharla · Pull Request #27 · NetApp/cloudstack

sandeeplocharla · 2026-01-21T16:42:27Z

Description

This PR has the following changes:

Create iSCSI type cloudstack volume
Delete iSCSI type cloudstack volume
Enter and Cancel Maintenance when storagepool has CS volumes

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

Screenshots (if appropriate):

How Has This Been Tested?

https://netapp.zoom.us/rec/share/r0rh_HhXD_lZNuppLeGGMk4Y4jg8-p9DXwIB_GD2e-uEslEH5geYzchrYcBDY7PJ.WjzaTxXoGt0B8nqe?startTime=1768919430000
Passcode: n=AE=.C9

Create ISCSI type Primary storage pool
Create Compute & Disk offering.
Create an instance with compute & disk offering to create both ROOT & DATA disks via ontap plugin.
Add additional CS Volume with ontap plugin and allocate it to the instance created.
Disable, Re-Enable Storage Pool (with CS Volumes)
Enter, Cancel Maintenance of Storage Pool (with CS Volumes)
Delete Instance + Expunge Volumes
Disable, Re-Enable Storage Pool (without CS Volumes)
Enter, Cancel Maintenance of Storage Pool (without CS Volumes)
Delete Storage Pool

https://netapp.zoom.us/rec/share/Ax74XcCVkQtXglaNwNb9rTcn_1k1TvxplvaLmXTdFxjz7DeQixmiqj-uwX0QXjz0.EwnlsurMGr3jLku3?startTime=1768920957000
Passcode: $%j3+K4B

Create an instance with compute & disk offering to create both ROOT & DATA disks via Default Primary & ontap plugins.
Add additional CS Volume with ontap plugin and allocate it to the instance created.
Disable, Re-Enable Storage Pool (with CS Volumes)
Enter, Cancel Maintenance of Storage Pool (with CS Volumes)
Delete Instance + Expunge Volumes
Disable, Re-Enable Storage Pool (without CS Volumes)
Enter, Cancel Maintenance of Storage Pool (without CS Volumes)
Delete Storage Pool

Observations:

Though we have increased retries to wait for discovery of luns, the additional volume attachment still times out, so, it needs a manual retry.
When placed into Maintenance mode, Instance is getting Stopped in the first case while it remains in Running state in the second case. We currently don't have any code attributing to this behaviour in our plugin code. Need to understand why cloudstack is behaving this way.

I've also tested force delete of storage pool when CS volumes were present, but, haven't covered it in these recordings.

…base in agent code

…ragePool create and delete

sandeeplocharla · 2026-01-27T09:45:23Z

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

+                if (volumeVO != null) {
+                    volumeVO.setPath(null);
+                    if (cloudStackVolume.getLun().getUuid() != null) {
+                        volumeVO.setFolder(cloudStackVolume.getLun().getUuid());


Add a comment on the need for this

sandeeplocharla · 2026-01-27T10:01:46Z

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

+        if (mapResp != null && mapResp.containsKey(Constants.LOGICAL_UNIT_NUMBER)) {
+            String lunNumber = mapResp.get(Constants.LOGICAL_UNIT_NUMBER);
+            s_logger.info("ensureLunMapped: Existing LunMap found for LUN [{}] in igroup [{}] with LUN number [{}]", lunName, accessGroupName, lunNumber);
+            return lunNumber;


Add a TODO comment to keep an eye on possibility of duplicate LUN for luns

sandeeplocharla · 2026-01-27T10:28:37Z

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

+                    VolumeVO volumeVO = volumeDao.findById(volInfo.getId());
+                    if (volumeVO != null) {
+                        String iscsiPath = Constants.SLASH + storagePool.getPath() + Constants.SLASH + lunNumber;
+                        volumeVO.set_iScsiName(iscsiPath);


Remove any redundant code

sandeeplocharla · 2026-01-27T10:46:41Z

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

+                return false;
+            }
+            // Set the aggregates which are according to the storage requirements
+            for (Aggregate aggr : aggrs) {


Check for rebase issue here

sandeeplocharla · 2026-01-27T10:56:47Z

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

+            Map<String, Object> queryParams = Map.of("allow_delete_while_mapped", "true");
+            try {
+                sanFeignClient.deleteLun(authHeader, cloudstackVolume.getLun().getUuid(), queryParams);
+            } catch (Exception ex) {


Check if feignException should be added

sandeeplocharla · 2026-01-27T11:05:07Z

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

+            String authHeader = Utility.generateAuthHeader(storage.getUsername(), storage.getPassword());
+            sanFeignClient.deleteLunMap(authHeader, lunUUID, igroupUUID);
+            s_logger.info("disableLogicalAccess: LunMap deleted successfully.");
+        } catch (Exception e) {


Use feignException

piyush5netapp · 2026-01-28T15:39:28Z

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

+        return null;
+    }

+    /**


I think its would be better to skip these comments on func as some of the information are related to our plugin internal design.

piyush5netapp · 2026-01-28T15:41:59Z

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

+                VolumeInfo volInfo = (VolumeInfo) dataObject;
+
+                // Create the backend storage object (LUN for iSCSI, no-op for NFS)
+                CloudStackVolume created = createCloudStackVolume(dataStore, volInfo, details);


a better name would be more meaningful.

piyush5netapp · 2026-01-28T15:47:05Z

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

+                        }
+
+                        // Determine scope ID based on storage pool scope (cluster or zone level igroup)
+                        long scopeId = (storagePool.getScope() == ScopeType.CLUSTER)


@rajiv-jain-netapp Can we discuss on including scope id and scope both for igroup name. It would be better to have only storage pool name reference in igroup name . Having scope or any other entity creates a dependency of their lifecycle on igroup.

@piyush5netapp lets discuss in tomorrow scrum

rajiv-jain-netapp · 2026-01-29T13:45:15Z

...s/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java

+                return 0L;
+            }
+        } catch (Exception ignore) {
+            // If FS check fails for any reason, fall back to blockdev call


lets add a logger here

rajiv-jain-netapp · 2026-01-29T13:57:41Z

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

+                        }
+
+                        // Determine scope ID based on storage pool scope (cluster or zone level igroup)
+                        long scopeId = (storagePool.getScope() == ScopeType.CLUSTER)


@piyush5netapp lets discuss in tomorrow scrum

rajiv-jain-netapp · 2026-01-29T14:05:22Z

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

+                        volumeVO.setPath(iscsiPath);
+                        s_logger.info("createAsync: Volume [{}] iSCSI path set to {}", volumeVO.getId(), iscsiPath);
+
+                    } else if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) {


I am not seeing anything significant for NFS block. I would recommend to not have if-else for protocol atleast for this situation, we should handle it via strategy concrete implementation.

...s/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java

rajiv-jain-netapp · 2026-01-29T14:10:55Z

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

-                    // ManagedNFS qcow2 backing file deletion handled by KVM host/libvirt; nothing to do via ONTAP REST.
-                    s_logger.info("deleteAsync: ManagedNFS volume {} no-op ONTAP deletion", data.getId());
+                    // NFS file deletion is handled by the hypervisor; no ONTAP REST call needed
+                    s_logger.info("deleteAsync: ManagedNFS volume {} - file deletion handled by hypervisor", data.getId());


same comment here as well

rajiv-jain-netapp · 2026-01-29T14:20:15Z

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

+                            VirtualMachine.State.Destroyed,
+                            VirtualMachine.State.Expunging,
+                            VirtualMachine.State.Error).contains(vm.getState())) {
+                        s_logger.debug("revokeAccess: Volume [{}] is still attached to VM [{}] in state [{}], skipping revokeAccess",


I guess, this logger can be made error instead debug, to highlight in the logs

This can be a warning at max, as this is cloudstack behaviour to make the revokeAccess call but its us who don't want to perform it, when the VMs state is one if these.

rajiv-jain-netapp · 2026-01-29T14:24:08Z

...ins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Igroup.java


    @JsonProperty("protocol")
-    private ProtocolEnum protocol = ProtocolEnum.mixed;
+    private ProtocolEnum protocol = null;


this is good catch, since we have 1-1 mapping between igroup and storage-pool, we should not keep protocol type as mixed, lets keep as per the storage-pool protocol.

rajiv-jain-netapp · 2026-01-29T14:25:04Z

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


    @JsonInclude(JsonInclude.Include.NON_NULL)
    public static class Links { }
-


There is no code change in this file. Can you exclude this file?

rajiv-jain-netapp · 2026-01-29T14:32:08Z

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

-    CloudStackVolume getCloudStackVolume(CloudStackVolume cloudstackVolume) {
-        //TODO
-        return null;
+    public void copyCloudStackVolume(CloudStackVolume cloudstackVolume) {


this code is added for which workflow?

This has lun clone, could be used for snapshot

suryag1201 · 2026-01-29T17:16:17Z

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

        if (callback == null) {
            throw new InvalidParameterValueException("createAsync: callback should not be null");
        }
+


add dataObject null condition check, also add error logger

suryag1201 · 2026-01-29T17:18:40Z

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

+                VolumeInfo volInfo = (VolumeInfo) dataObject;
+
+                // Create the backend storage object (LUN for iSCSI, no-op for NFS)
+                CloudStackVolume created = createCloudStackVolume(dataStore, volInfo, details);


it can also come for snapshot ?

suryag1201 · 2026-01-29T17:21:16Z

...ntap/src/main/java/org/apache/cloudstack/storage/provider/OntapPrimaryDatastoreProvider.java

    public String getName() {
        s_logger.trace("OntapPrimaryDatastoreProvider: getName: Called");
-        return "ONTAP Primary Datastore Provider";
+        return "ONTAP";


please add this string to some constant and use it here

…ryStoragePool

Co-authored-by: Srivastava, Piyush <Piyush.Srivastava@netapp.com>

…ce recreate option (apache#12004)

…t settings (apache#11740)

…che#11847)

…pache#11845)

…che#10705)

Co-authored-by: Vishesh <8760112+vishesh92@users.noreply.github.com>

* api,server: allow configuring repetitive alerts Fixes apache#6613 Introduces support for configuring additional alert types that can be published repeatedly, beyond the default set. Operators can now use the dynamic configuration `alert.allowed.repetitive.types` to specify a comma-separated list of alert type names that should be allowed for repetitive publication. Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com> * add tests Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com> * fix Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com> * test fix Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com> * allow repetition for custom alerts Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com> * remove refactoring Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com> --------- Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

…hey already existed. (apache#12059)

…pache#12497) Bumps [org.apache.maven.plugins:maven-war-plugin](https://github.com/apache/maven-war-plugin) from 3.4.0 to 3.5.1. - [Release notes](https://github.com/apache/maven-war-plugin/releases) - [Commits](apache/maven-war-plugin@maven-war-plugin-3.4.0...maven-war-plugin-3.5.1) --- updated-dependencies: - dependency-name: org.apache.maven.plugins:maven-war-plugin dependency-version: 3.5.1 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Replace maven-jgit-buildnumber-plugin with thread safe buildnumber-maven-plugin * Fix mysql-connector-java warning * Fix thread safe warning for properties-maven-plugin * Fix mvn build - marvin warnings * Update tools/marvin/README.md * Update tools/marvin/README.md Co-authored-by: dahn <daan.hoogland@gmail.com> --------- Co-authored-by: dahn <daan.hoogland@gmail.com>

Signed-off-by: Viddya K <viddya.k@ibm.com> Signed-off-by: Niyam Siwach <niyam@ibm.com> Co-authored-by: root <root@c63716v1.fyre.ibm.com>

… registration (apache#12165) * engine/schema: prepend algorithm to checksum during systemvm template registration * Update utils/src/main/java/org/apache/cloudstack/utils/security/DigestHelper.java

* extension/proxmox: improve host vm power reporting Add `statuses` action in extensions to report VM power states This PR introduces support for retrieving the power state of all VMs on a host directly from an extension using the new `statuses` action. When available, this provides a single aggregated response, reducing the need for multiple calls. If the extension does not implement `statuses`, the server will gracefully fall back to querying individual VMs using the existing `status` action. This helps with updating the host in CloudStack after out-of-band migrations for the VM. Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com> * address review Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com> --------- Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

…#12422)

… storage (apache#12465)

…job polling method

github-actions · 2026-02-06T07:12:49Z

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

…job polling method

…ure/CSTACKEX-46-new

…-new' into feature/CSTACKEX-46-new

sandeeplocharla requested review from piyush5netapp, rajiv-jain-netapp and suryag1201 January 21, 2026 16:42

sandeeplocharla self-assigned this Jan 21, 2026

Locharla, Sandeep added 2 commits January 22, 2026 15:04

CSTACKEX-46: Fixed a check style issue which got introduced during re…

3eea5c4

…base in agent code

CSTACKEX-46: Fixed a couple of issues observed while testing NFS3 sto…

63addcd

…ragePool create and delete

sandeeplocharla commented Jan 27, 2026

View reviewed changes

CSTACKEX-46: Refactored driver class

dfc1ee9

piyush5netapp reviewed Jan 28, 2026

View reviewed changes

rajiv-jain-netapp reviewed Jan 29, 2026

View reviewed changes

suryag1201 reviewed Jan 29, 2026

View reviewed changes

Locharla, Sandeep added 2 commits February 5, 2026 11:35

CSTACKEX-46: Fixed code as per comments received

5895ae3

CSTACKEX-46: Added some code that was added to community PR for Prima…

887e589

…ryStoragePool

sandeeplocharla requested review from piyush5netapp, rajiv-jain-netapp and suryag1201 February 5, 2026 10:58

piyush5netapp and others added 10 commits February 6, 2026 12:35

UTs for NFS storage pool creation code (#29)

1d1f596

Co-authored-by: Srivastava, Piyush <Piyush.Srivastava@netapp.com>

uefi property typo (apache#11929)

0e3a51e

DB setup: support db schema creation (with --schema-only) without for…

5ef9a22

…ce recreate option (apache#12004)

Enable UEFI on KVM hosts (by default), and configure with some defaul…

bb71248

…t settings (apache#11740)

CKS: update cloud.kubernetes.cluster.network.offering to dynamic (apa…

ce8b1e4

…che#11847)

consider Instance in Starting state for listPodsByUserConcentration (a…

c09aebd

…pache#11845)

Veeam: get templateId from vm instance if vm is created from ISO (apa…

6848e30

…che#10705)

Veeam: use pre-defined object mapper (apache#10715)

8fc0c93

UI: Update and reset domain level configuration (apache#11571)

da1e48b

Track volume usage data at a vm granularity as well (apache#11531)

cea35ea

Co-authored-by: Vishesh <8760112+vishesh92@users.noreply.github.com>

shwstppr and others added 19 commits February 6, 2026 12:40

kvm: honour disk controller for vm during attach volume (apache#12452)

5fc8c11

Fixed: unnecessary regeneration of SSH keys in developer mode when t…

7224af5

…hey already existed. (apache#12059)

test: increase timeout in test_human_readable_logs.py (apache#11972)

1a28863

api,server: allow cleaning up vm extraconfig (apache#11974)

45dd567

Add s390x Support for Cloudstack Simulator (apache#12337)

3223ada

Signed-off-by: Viddya K <viddya.k@ibm.com> Signed-off-by: Niyam Siwach <niyam@ibm.com> Co-authored-by: root <root@c63716v1.fyre.ibm.com>

eofFix

8b43a9b

engine/schema: prepend algorithm to checksum during systemvm template…

2be8758

… registration (apache#12165) * engine/schema: prepend algorithm to checksum during systemvm template registration * Update utils/src/main/java/org/apache/cloudstack/utils/security/DigestHelper.java

Set CORS headers for both http/https in SSVM apache config (apache#9389)

7f5444e

Restart xl2tpd service when the VPN configuration is modified (apache…

6aa69ca

…#12422)

Update search filter size dynamically (apache#12552)

b5cba90

UI: Prevent extra API calls in search filter on scrolling (apache#12553)

077c70f

Update physical size for the snapshots of the volumes on ceph primary…

81e945c

… storage (apache#12465)

Update storage usage / size in backup usage response (apache#12539)

892706a

CSTACKEX-46: Added UTs and variables to pass maxRetries and delay to …

b88a6cb

…job polling method

CSTACKEX-46: Removed the unused json dependency from pom.xml

2e8c287

github-actions bot added the status:has-conflicts label Feb 6, 2026

Locharla, Sandeep and others added 3 commits February 6, 2026 12:55

CSTACKEX-46: Added UTs and variables to pass maxRetries and delay to …

459a5a8

…job polling method

CSTACKEX-46: Removed the unused json dependency from pom.xml

835a740

Merge branch 'main' of https://github.com/NetApp/cloudstack into feat…

698b4ce

…ure/CSTACKEX-46-new

github-actions bot removed the status:has-conflicts label Feb 6, 2026

Srivastava, Piyush and others added 4 commits February 6, 2026 14:37

CSTACKEX-46:removed unrelated changes

a6aed97

CSTACKEX-46: Rebase and resolving conflicts

5e28662

Merge remote-tracking branch 'refs/remotes/origin/feature/CSTACKEX-46…

0b4a5d6

…-new' into feature/CSTACKEX-46-new

CSTACKEX-46: Rebase changes

e35f0a1

sandeeplocharla merged commit 9bf90ea into main Feb 6, 2026
13 of 16 checks passed


		@JsonInclude(JsonInclude.Include.NON_NULL)
		public static class Links { }

Conversation

sandeeplocharla commented Jan 21, 2026

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

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!

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!

suryag1201 Jan 29, 2026 • 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!

github-actions bot commented Feb 6, 2026

Uh oh!

Uh oh!

suryag1201 Jan 29, 2026 •

edited

Loading