CloudStack Volume support with ONTAP storage

[rajiv-jain-netapp]

Description

Co-authored-by: Sandeep Locharla sandeep.locharla@netapp.com
Co-authored-by: Piyush Srivastava piyush5@netapp.com
Co-authored-by: Surya Gupta suryag@netapp.com

This PR...

1. Supports cloudstack-volume usecases support for NFS3 and iSCSI protocols.
2. Support for cloudStack-volume create/delete/attach/detach/snapshot-create.
3. Instance create/delete/snapshot-create support
4. Snapshot-create would not support
   1. memory snapshot
   2. user input for quicing option during snapshot creation

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?

Testing Done:

1. VM create

2. Validate the respective CloudStack volume

3. Storage view for the cloudStack volumes

4. VM snapshot support

5. Storage view, post snapshot create

6. CloudStack UI list view for snapshot

7. CloudStack volume snapshot support

8. Storage view, post snapshot create

9. CloudStack UI list view for snapshot

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

[Copilot]

Pull request overview

This PR adds NetApp ONTAP as a managed primary storage provider, including UI wiring for pool creation and backend support for CloudStack volume workflows over NFS3 and iSCSI, plus VM/volume snapshot integration for KVM.

Changes:

• UI: adds NetApp ONTAP provider inputs (IP/username/password/SVM/protocol) and provider-specific parameter mapping.
• Backend: introduces ONTAP storage strategies (NAS/SAN), lifecycle attach/delete behavior, and Feign clients/models for snapshot + file/LUN restore workflows.
• KVM/engine: adjusts iSCSI handling and VM snapshot behavior to integrate ONTAP-specific snapshot strategy/limitations.

Reviewed changes

Copilot reviewed 42 out of 42 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
ui/src/views/infra/zone/ZoneWizardLaunchZone.vue	Adds NetApp ONTAP provider handling + details mapping during zone wizard primary storage creation
ui/src/views/infra/zone/ZoneWizardAddResources.vue	Adds ONTAP-specific form fields and protocol choices in zone wizard
ui/src/views/infra/zone/StaticInputsForm.vue	Adds numeric-format validation helper used by static forms
ui/src/views/infra/AddPrimaryStorage.vue	Adds ONTAP provider form fields, validation rules, and API param mapping
ui/public/locales/en.json	Adds English i18n strings for ONTAP fields/tooltips
server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java	Adds ONTAP-specific error message when memory snapshots requested without a suitable strategy
plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java	Adds unit tests for UnifiedNASStrategy behaviors
plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java	Adds unit tests for StorageStrategy connect/volume/network methods
plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java	Extends lifecycle tests for init/attach cluster/zone flows
plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java	Adds driver tests for create/delete/grant/revoke access flows
plugins/storage/volume/ontap/src/main/resources/META-INF/cloudstack/storage-volume-ontap/spring-storage-volume-ontap-context.xml	Registers ONTAP VM snapshot strategy bean
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java	Adds ONTAP helpers for auth headers, naming, and protocol-specific volume request building
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java	Updates provider naming and adds constants for snapshot/mount options
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/CloudStackVolume.java	Extends request/response model to support snapshot workflows (flexvol UUID, destination path)
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/AccessGroup.java	Refactors access-group request to use storagePoolId instead of PrimaryDataStoreInfo
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java	Implements iSCSI LUN CRUD, igroup management, LUN mapping helpers, and snapshot revert via CLI API
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java	Implements NFS volume handling via host commands + export policy management + snapshot revert via CLI API
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java	Centralizes Feign client creation and adds snapshot job polling + snapshot feign accessors
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/SANStrategy.java	Adds SAN helper contract for ensuring LUN mappings and initiator validation
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/provider/StorageProviderFactory.java	Selects unified NAS/SAN strategy by protocol and injects components
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/listener/OntapHostListener.java	Passes pool details to agent on host connect and updates pool host refs/capacity
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java	Refactors initialization inputs, forces NFSv3 mount options, and adds attach/delete logic updates
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/VolumeConcise.java	Adds concise Volume model used by snapshot responses
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/SnapshotFileRestoreRequest.java	Adds request model for snapshot file restore API
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/OntapStorage.java	Refactors ONTAP connection model to use storageIP and removes disaggregated flag
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/LunRestoreRequest.java	Adds request model for LUN restore API
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/FlexVolSnapshot.java	Adds model for FlexVolume-level snapshots
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/FileInfo.java	Adjusts file model fields for NAS operations
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/FileClone.java	Adds request model for ONTAP file clone
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/CliSnapshotRestoreRequest.java	Adds CLI passthrough restore-file request model
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SnapshotFeignClient.java	Adds Feign client for FlexVol snapshot and restore APIs (including CLI passthrough)
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java	Adds Feign method for LUN restore
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/NASFeignClient.java	Updates file GET to include metadata and adds file clone endpoint
plugins/storage/volume/ontap/pom.xml	Adds dependencies and surefire argLine for tests
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java	Makes iSCSI login/create idempotent, rescans sessions, avoids logging out when other LUNs exist, adds flush/sync
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCreateDiskOnlyVMSnapshotCommandWrapper.java	Improves error handling and consolidates cleanup logic for snapshot deltas
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java	Updates managed template copy flow to use iSCSI/LUN info available post-grantAccess for ONTAP
engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java	Excludes ONTAP managed pools from the generic KVM file-based VM snapshot strategy and improves failure messages

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

deleteDataStore calls storageStrategy.deleteAccessGroup(accessGroup) but the AccessGroup instance only has storagePoolId set; hostsToConnect is never populated. The SAN implementation (UnifiedSANStrategy.deleteAccessGroup) iterates accessGroup.getHostsToConnect() and will throw at runtime for iSCSI pools. Either populate hostsToConnect (e.g., from the cluster/zone scope or storage_pool_host_ref) before calling deleteAccessGroup, or make SAN deleteAccessGroup handle a null/empty host list safely (and/or skip SAN access-group deletion here if it cannot be determined).

Suggested change

	// Delete access groups associated with this storage pool
	storageStrategy.deleteAccessGroup(accessGroup);
	logger.info("deleteDataStore: Successfully deleted access groups for storage pool '{}'", storagePool.getName());
	// Delete access groups associated with this storage pool only when host information is available.
	// Some SAN strategy implementations iterate hostsToConnect and can fail at runtime if it is null.
	if (accessGroup.getHostsToConnect() == null || accessGroup.getHostsToConnect().isEmpty()) {
	logger.warn("deleteDataStore: Skipping access-group deletion for storage pool '{}' because hostsToConnect is not populated",
	storagePool.getName());
	} else {
	storageStrategy.deleteAccessGroup(accessGroup);
	logger.info("deleteDataStore: Successfully deleted access groups for storage pool '{}'", storagePool.getName());
	}

[Copilot]

generateAuthHeader uses String.getBytes() and new String(byte[]) without an explicit charset. This makes auth header generation platform-default dependent and can break for non-ASCII credentials on systems with non-UTF-8 defaults. Please use StandardCharsets.UTF_8 explicitly for both conversions.

[Copilot]

StorageProviderFactory.getStrategy unconditionally Base64-decodes ontapStorage.getPassword(). This will throw an IllegalArgumentException if the password is provided in plain text (e.g., via API clients or existing DB data), making pool creation/attachment fail. Consider either storing a clear contract for password encoding (and enforcing it at input), or decoding conditionally (try/catch with fallback to the original string).

[Copilot]

VolumeConcise.setName is a no-op, so JSON deserialization will never populate name. This can lead to null names in FlexVolSnapshot.volume and any logic relying on it. Implement the setter (or remove it and make the field immutable with constructor-based binding).

Suggested change

	public void setName(String name) {}
	public void setName(String name) {
	this.name = name;
	}

[Copilot]

hostConnect casts answer to ModifyStoragePoolAnswer without checking instanceof. If the agent returns a different Answer type, this will throw ClassCastException and be caught by the broad catch, losing the real failure reason. Please restore an explicit type check (and return a clear error) before casting.

Suggested change

	// Get the mount path from the answer
	// Get the mount path from the answer
	if (!(answer instanceof ModifyStoragePoolAnswer)) {
	throw new CloudRuntimeException(String.format(
	"Unexpected answer type %s returned for modify storage pool command for pool %s on host %d",
	answer.getClass().getName(), pool, hostId));
	}

[Copilot]

The dependency on cloud-engine-storage-snapshot is pinned to 4.23.0.0-SNAPSHOT while the rest of the module uses ${project.version}. This makes backports/version bumps harder and can break builds if the parent version changes. Prefer ${project.version} (or rely on dependencyManagement) for intra-repo artifacts.

Suggested change

	<version>4.23.0.0-SNAPSHOT</version>
	<version>${project.version}</version>

[Copilot]

The provider-name check uses the literal string "ONTAP", but elsewhere in this PR the provider is identified as "NetApp ONTAP" (UI, KvmFileBasedStorageVmSnapshotStrategy, OntapStorageConstants). As-is, this branch will never trigger and the error message for unsupported memory snapshots on ONTAP managed storage will be missed. Consider comparing against a shared constant (e.g., OntapStorageConstants.ONTAP_PLUGIN_NAME) or updating the literal to the actual provider name used by storage pools.

[Copilot]

In NFS3 host identifier validation, the condition if (ip.isEmpty() && host.getPrivateIpAddress() != null || host.getPrivateIpAddress().trim().isEmpty()) has incorrect operator precedence and can throw a NullPointerException when privateIpAddress is null. It also allows hostIdentifiers.add(ip) to reuse the previous host's IP when host is null. Please restructure this block to (a) short-circuit correctly when private IP is null/blank, and (b) only add an IP for the current host (skip null hosts and reset ip per iteration).

[codecov]

Codecov Report

❌ Patch coverage is 0% with 1612 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.96%. Comparing base (ed575cc) to head (a20b6cc).
⚠️ Report is 92 commits behind head on main.

Files with missing lines	Patch %	Lines
...ck/storage/driver/OntapPrimaryDatastoreDriver.java	0.00%	421 Missing ⚠️
...ck/storage/vmsnapshot/OntapVMSnapshotStrategy.java	0.00%	395 Missing ⚠️
...cloudstack/storage/service/UnifiedSANStrategy.java	0.00%	258 Missing ⚠️
...cloudstack/storage/service/UnifiedNASStrategy.java	0.00%	179 Missing ⚠️
...hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java	0.00%	67 Missing ⚠️
...rage/lifecycle/OntapPrimaryDatastoreLifecycle.java	0.00%	53 Missing ⚠️
...he/cloudstack/storage/utils/OntapStorageUtils.java	0.00%	45 Missing ⚠️
...udstack/storage/feign/model/LunRestoreRequest.java	0.00%	28 Missing ⚠️
...loudstack/storage/feign/model/FlexVolSnapshot.java	0.00%	25 Missing ⚠️
...LibvirtCreateDiskOnlyVMSnapshotCommandWrapper.java	0.00%	24 Missing ⚠️
... and 14 more

Additional details and impacted files

@@             Coverage Diff             @@
##               main   #13053     +/-   ##
===========================================
  Coverage     17.95%   17.96%             
- Complexity    16503    16579     +76     
===========================================
  Files          6019     6036     +17     
  Lines        540748   542698   +1950     
  Branches      66255    66638    +383     
===========================================
+ Hits          97084    97486    +402     
- Misses       432723   434213   +1490     
- Partials      10941    10999     +58     

Flag	Coverage Δ
uitests	3.52% <ø> (-0.02%)	⬇️
unittests	19.11% <0.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.