Skip to content

Create & Delete, Enable & Disable, Enter & Cancel maintenance of Prim…#12563

Open
sandeeplocharla wants to merge 1 commit intoapache:mainfrom
NetApp:netapp-ontap-primary-storage-pool
Open

Create & Delete, Enable & Disable, Enter & Cancel maintenance of Prim…#12563
sandeeplocharla wants to merge 1 commit intoapache:mainfrom
NetApp:netapp-ontap-primary-storage-pool

Conversation

@sandeeplocharla
Copy link

@sandeeplocharla sandeeplocharla commented Feb 1, 2026

Create & Delete, Enable & Disable, Enter & Cancel maintenance of Primary StoragePool with ONTAP storage

Co-authored-by: Rajiv Jain rajiv1@netapp.com
Co-authored-by: Piyush Srivastava piyush5@netapp.com
Co-authored-by: Surya Gupta suryag@netapp.com

Description

NetApp ONTAP Storage Plugin

Introduction

This document describes in brief the design and implementation of the NetApp ONTAP storage plugin for Apache CloudStack. The plugin enables CloudStack to use NetApp ONTAP as a primary storage provider.

ONTAP Terminology

Term Description
SVM Storage Virtual Machine - a logical storage container that provides data access to clients
FlexVolume A logical volume that can grow or shrink within an aggregate
Aggregate A collection of physical disks that provides storage for volumes
LIF Logical Interface - a network interface for client access (Management LIF for admin, Data LIF for storage traffic)
iGroup Initiator Group - a collection of host initiator IQNs used for iSCSI access control
Export Policy A set of rules that define NFS client access to volumes

Scope

Current Implementation

The following operations are currently implemented:

Operation Description
Create Primary Storage Pool Creates a FlexVolume on ONTAP and registers it as a primary storage pool in CloudStack
Delete Primary Storage Pool Removes the storage pool from CloudStack and deletes the FlexVolume from ONTAP
Attach Cluster/Zone Creates access groups (iGroups for iSCSI, export policies for NFS) and connects hosts
Enable/Disable Storage Pool Enables or disables the storage pool for use
Enter/Cancel Maintenance Places storage pool in maintenance mode or brings it back online

Supported Configurations

Configuration Supported Values
Hypervisor KVM
ONTAP Platform Unified
Protocols NFS 3.0, iSCSI
Storage Pool Scope Cluster, Zone

Storage Pool Lifecycle

Create Storage Pool

  1. Validate input parameters (SVM, protocol, credentials)
  2. Connect to ONTAP and verify SVM state and protocol enablement
  3. Create FlexVolume on ONTAP
  4. Register storage pool in CloudStack

Attach to Cluster/Zone

  1. Identify eligible hosts in the scope
  2. Create access group (iGroup or export policy)
  3. Connect each host to the storage pool

Delete Storage Pool

  1. Delete access groups from ONTAP
  2. Delete FlexVolume from ONTAP
  3. Remove storage pool from CloudStack

Configuration Parameters

Parameters are passed via URL in semicolon-separated key=value format:

Parameter Description
username ONTAP admin username
password ONTAP admin password
managementLIF ONTAP cluster management IP
svmName Storage Virtual Machine name
protocol NFS3 or ISCSI

There are few files under feign folder which aren't currently being consumed by the workflows being proposed for review. But, they are definitely required for upcoming workflows.

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

How Has This Been Tested?

  1. Management Server Operations:

    • Added a new zone using the Management server UI.
    • Verified functionality of core and advanced zone types.
  2. Primary Storage Pool Operations:

    • Create: Verified creation of primary storage pool.
    • Delete: Verified deletion of primary storage pool.
    • Enable/Disable: Tested enabling and disabling the storage pool.
    • Maintenance Mode: Verified entering and exiting maintenance mode.
    • Cancel Maintenance: Ensured maintenance cancellation works.

Testing Environment:

  • The test setup was conducted on an Ubuntu VM with the following components installed and configured:
    • Management server
    • Cloudstack agent and KVM
    • NFS server
    • iSCSI initiator
  • Testing has been done currently with a single host.
  • Following are snapshots from the test done:
    NFS3 Primary Storage Pool - Create
Screenshot 2026-02-02 at 4 40 22 PM Screenshot 2026-02-02 at 4 40 40 PM

NFS3 type ONTAP Volume created

Screenshot 2026-02-02 at 4 41 33 PM

Export Policy created on ONTAP

Screenshot 2026-02-02 at 4 43 18 PM

Disabled Primary Storage Pool - NFS3

Screenshot 2026-02-02 at 4 43 49 PM

Re-Enabled Primary Storage Pool - NFS3

Screenshot 2026-02-02 at 4 43 57 PM

Enter maintenance mode - NFS3

Screenshot 2026-02-02 at 4 44 23 PM

Cancel maintenance mode - NFS3

Screenshot 2026-02-02 at 4 44 38 PM

Delete Primary StoragePool - NFS3

Screenshot 2026-02-02 at 4 45 13 PM

ONTAP Volume deleted

Screenshot 2026-02-02 at 4 45 35 PM

ONTAP ExportPolicy deleted

Screenshot 2026-02-02 at 4 45 45 PM

iSCSI Primary Storage Pool - Delete

Screenshot 2026-02-02 at 4 47 05 PM Screenshot 2026-02-02 at 4 47 18 PM

iSCSI type ONTAP Volume created

Screenshot 2026-02-02 at 4 47 42 PM

iGroup created

Screenshot 2026-02-02 at 4 48 18 PM

Disabled Primary Storage Pool - iSCSI

Screenshot 2026-02-02 at 4 48 47 PM

Re-Enabled Primary Storage Pool - iSCSI

Screenshot 2026-02-02 at 4 48 58 PM

Enter maintenance mode - iSCSI

Screenshot 2026-02-02 at 4 49 09 PM

Cancel maintenance mode - iSCSI

Screenshot 2026-02-02 at 4 49 23 PM

Delete Primary StoragePool - iSCSI

Screenshot 2026-02-02 at 4 49 48 PM

ONTAP iGroup deleted

Screenshot 2026-02-02 at 4 50 52 PM

ONTAP Volume deleted

Screenshot 2026-02-02 at 4 51 10 PM

@codecov
Copy link

codecov bot commented Feb 1, 2026

Codecov Report

❌ Patch coverage is 5.59222% with 2718 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.83%. Comparing base (7b94ccc) to head (b8e5cd6).
⚠️ Report is 24 commits behind head on main.

Files with missing lines Patch % Lines
...he/cloudstack/storage/service/StorageStrategy.java 0.35% 280 Missing ⚠️
...rage/lifecycle/OntapPrimaryDatastoreLifecycle.java 30.88% 209 Missing and 17 partials ⚠️
...cloudstack/storage/service/UnifiedNASStrategy.java 0.00% 200 Missing ⚠️
...org/apache/cloudstack/storage/feign/model/Lun.java 0.00% 196 Missing ⚠️
...cloudstack/storage/service/UnifiedSANStrategy.java 0.00% 186 Missing ⚠️
...pache/cloudstack/storage/feign/model/FileInfo.java 0.00% 175 Missing ⚠️
.../apache/cloudstack/storage/feign/model/Igroup.java 0.00% 155 Missing ⚠️
...che/cloudstack/storage/feign/model/ExportRule.java 0.00% 108 Missing ⚠️
...e/cloudstack/storage/feign/FeignConfiguration.java 0.00% 89 Missing ⚠️
...cloudstack/storage/listener/OntapHostListener.java 0.00% 85 Missing ⚠️
... and 32 more
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #12563      +/-   ##
============================================
- Coverage     17.89%   17.83%   -0.06%     
- Complexity    16084    16118      +34     
============================================
  Files          5936     5979      +43     
  Lines        532635   535613    +2978     
  Branches      65148    65370     +222     
============================================
+ Hits          95298    95538     +240     
- Misses       426667   429379    +2712     
- Partials      10670    10696      +26     
Flag Coverage Δ
uitests 3.68% <ø> (+<0.01%) ⬆️
unittests 18.92% <5.59%> (-0.07%) ⬇️

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.

@sandeeplocharla sandeeplocharla force-pushed the netapp-ontap-primary-storage-pool branch 2 times, most recently from c495c7c to 20d4e97 Compare February 2, 2026 12:55
@sandeeplocharla sandeeplocharla marked this pull request as ready for review February 2, 2026 14:05
@sandeeplocharla sandeeplocharla force-pushed the netapp-ontap-primary-storage-pool branch from 20d4e97 to c98cf8c Compare February 2, 2026 14:12
@kiranchavala
Copy link
Member

@blueorangutan package

@blueorangutan
Copy link

@kiranchavala a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✖️ debian ✔️ suse15. SL-JID 16683

@kiranchavala
Copy link
Member

@blueorangutan test

@blueorangutan
Copy link

@kiranchavala a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

@DaanHoogland DaanHoogland added this to the 4.23.0 milestone Feb 3, 2026
@DaanHoogland
Copy link
Contributor

Hey @sandeeplocharla et al,

Great work, some issues came up in the github actions. Can you first look at the license check and the pre-commit actions?
thanks for the extensive test description as well. We cannot guarantee 3rd party testing as I hope you understand. I’ll be sure to review your work and thanks again.

@sandeeplocharla
Copy link
Author

Hi @DaanHoogland
Thank you! I'm trying to understand the errors. For us to have more clarity, could you please help us understand in brief on what is being checked in the tests that failed?

@rajiv-jain-netapp
Copy link

Hey @sandeeplocharla et al,

Great work, some issues came up in the github actions. Can you first look at the license check and the pre-commit actions? thanks for the extensive test description as well. We cannot guarantee 3rd party testing as I hope you understand. I’ll be sure to review your work and thanks again.

Thank you, @DaanHoogland. We look forward to receiving your review comments and will address them as soon as possible.

@sandeeplocharla sandeeplocharla force-pushed the netapp-ontap-primary-storage-pool branch 2 times, most recently from 08b53f0 to b1792b4 Compare February 3, 2026 11:58
@DaanHoogland DaanHoogland self-requested a review February 3, 2026 13:41
@sandeeplocharla
Copy link
Author

sandeeplocharla commented Feb 3, 2026

Hi, the project level Code coverage seems to be failing, due to mutliple files that this code is indirectly touching, doesn't have coverage. What could be done in such a case?
Also, please help us with the smoke and component tests that are failing? I don't think we have made changes that are relevant to those tests.

@DaanHoogland
Copy link
Contributor

Hi, the project level Code coverage seems to be failing, due to mutliple files that this code is indirectly touching, doesn't have coverage. What could be done in such a case? Also, please help us with the smoke and component tests that are failing? I don't think we have made changes that are relevant to those tests.

@sandeeplocharla , we have ambitions on coverage that we don’t meet historically. Please trty to meet them on your part of the code, more we cannot ask. As for the failing tests, these seem te be the same that are failing on main as well. If you can give us any hints on them that will be very much appreciated ;)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you do license checks on the extra dependencies added in this file?
see https://apache.org/legal/resolved.html for explanation.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will check the licenses and get back to you.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @DaanHoogland , I've cross-checked the licenses of the dependencies added in our pom.xml. I had to remove one which wasn't conforming with ACF guidelines that you've shared.
Currently, all licenses in the ONTAP plugin are compliant with Apache CloudStack norms. All runtime dependencies use Apache License 2.0 (Category A). Test dependencies use MIT and EPL 2.0 licenses, which are also Category A compatible. No Category X licenses are present.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks so much @sandeeplocharla 👍

@sandeeplocharla
Copy link
Author

sandeeplocharla commented Feb 3, 2026

Hi, the project level Code coverage seems to be failing, due to mutliple files that this code is indirectly touching, doesn't have coverage. What could be done in such a case? Also, please help us with the smoke and component tests that are failing? I don't think we have made changes that are relevant to those tests.

@sandeeplocharla , we have ambitions on coverage that we don’t meet historically. Please trty to meet them on your part of the code, more we cannot ask. As for the failing tests, these seem te be the same that are failing on main as well. If you can give us any hints on them that will be very much appreciated ;)

This PR does not have the benchmark coverage that we usually aim for within our team. But, we plan on having maximum code coverage possible with the upcoming PRs. As for this PR, there are around 5400 indirect files because of which the coverage dropped. I'm not sure, how we would able to handle them all.
Will try to see if there's anything that we could do in RCA for the failing tests, but, in the meanwhile, please let us know how we could proceed from here?

@blueorangutan
Copy link

[SF] Trillian test result (tid-15359)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 54505 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr12563-t15359-kvm-ol8.zip
Smoke tests completed. 146 look OK, 4 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_provision_certificate Error 121.48 test_certauthority_root.py
ContextSuite context=TestListIdsParams>:teardown Error 1.11 test_list_ids_parameter.py
test_01_snapshot_root_disk Error 5.22 test_snapshots.py
test_02_list_snapshots_with_removed_data_store Error 45.47 test_snapshots.py
test_02_list_snapshots_with_removed_data_store Error 45.48 test_snapshots.py
ContextSuite context=TestSnapshotStandaloneBackup>:teardown Error 28.38 test_snapshots.py
test_01_snapshot_usage Error 25.62 test_usage.py
test_01_vpn_usage Error 1.07 test_usage.py

@DaanHoogland
Copy link
Contributor

Will try to see if there's anything that we could do in RCA for the failing tests, but, in the meanwhile, please let us know how we could proceed from here?

this is good enough until we encounter tangible issues. Please continue with the comment outstanding now. review effort will have to be made and I (or we) will hopefully get to this soon. Please be prepared for some rebasing and conflict resolving over the coming weeks 🥺 … unfortunately I cannot promise when further review will happen, but this PR is on my list.

…ary StoragePool with ONTAP storage

Co-authored-by: Rajiv Jain <Rajiv.Jain@netapp.com>

Create & Delete, Enable & Disable, Enter & Cancel maintenance of Primary StoragePool with ONTAP storage
Co-authored-by: Rajiv Jain<rajiv1@netapp.com>

Edited readme file

Fixed license check issue

Removed dependency that's not conforming with ACF guidelines
@sandeeplocharla sandeeplocharla force-pushed the netapp-ontap-primary-storage-pool branch from b1792b4 to b8e5cd6 Compare February 4, 2026 09:37
@DaanHoogland DaanHoogland self-assigned this Feb 4, 2026
@DaanHoogland
Copy link
Contributor

@blueorangutan package

@blueorangutan
Copy link

@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 16871

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✖️ debian ✔️ suse15. SL-JID 16874

@DaanHoogland
Copy link
Contributor

@blueorangutan test

@blueorangutan
Copy link

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian test result (tid-15500)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 63136 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr12563-t15500-kvm-ol8.zip
Smoke tests completed. 147 look OK, 3 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
ContextSuite context=TestListIdsParams>:teardown Error 1.12 test_list_ids_parameter.py
test_01_snapshot_root_disk Error 3.85 test_snapshots.py
test_02_list_snapshots_with_removed_data_store Error 46.56 test_snapshots.py
test_02_list_snapshots_with_removed_data_store Error 46.57 test_snapshots.py
ContextSuite context=TestSnapshotStandaloneBackup>:teardown Error 27.38 test_snapshots.py
test_01_snapshot_usage Error 22.65 test_usage.py
test_01_vpn_usage Error 1.08 test_usage.py

@sandeeplocharla
Copy link
Author

Will try to see if there's anything that we could do in RCA for the failing tests, but, in the meanwhile, please let us know how we could proceed from here?

this is good enough until we encounter tangible issues. Please continue with the comment outstanding now. review effort will have to be made and I (or we) will hopefully get to this soon. Please be prepared for some rebasing and conflict resolving over the coming weeks 🥺 … unfortunately I cannot promise when further review will happen, but this PR is on my list.

Hello @DaanHoogland, let us know if this PR has to be rebased or if anything else should be done for this PR?

@nvazquez
Copy link
Contributor

Thanks @sandeeplocharla great work! An additional question about the functionalities, would SnapMirror be supported for volume replication? In case it is not, are there any plans to add support for it?

@DaanHoogland
Copy link
Contributor

Hello @DaanHoogland, let us know if this PR has to be rebased or if anything else should be done for this PR?

No, @harikrishna-patnala @sureshanaparti @weizhouapache , can you review please?

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new NetApp ONTAP primary storage plugin module to CloudStack, implementing primary storage pool lifecycle operations (create/delete, enable/disable, maintenance) and ONTAP REST integrations for NFS3 and iSCSI.

Changes:

  • Introduces the new storage/volume/ontap plugin module (provider, lifecycle, driver, host listener, strategy layer).
  • Adds Feign REST clients + ONTAP model classes used by the strategies.
  • Adds initial unit tests and wires the plugin into the build (plugins/pom.xml, client/pom.xml).

Reviewed changes

Copilot reviewed 60 out of 60 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/provider/OntapPrimaryDatastoreProviderTest.java Adds unit tests for provider configuration and getters.
plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java Adds unit tests for lifecycle initialization validation paths.
plugins/storage/volume/ontap/src/main/resources/META-INF/cloudstack/storage-volume-ontap/spring-storage-volume-ontap-context.xml Registers the ONTAP primary datastore provider bean for the plugin.
plugins/storage/volume/ontap/src/main/resources/META-INF/cloudstack/storage-volume-ontap/module.properties Declares the plugin module name and parent module.
plugins/storage/volume/ontap/src/main/resources/META-INF/cloudstack/storage-volume-ontap/logback-spring.xml Adds a Logback configuration intended for Feign logging.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java Adds helper methods (auth header generation, strategy lookup, naming helpers).
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Constants.java Centralizes ONTAP constants (keys, ports, REST params, naming tokens).
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/ProtocolType.java Defines supported protocol enum values (NFS3, ISCSI).
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/CloudStackVolume.java Adds a container model for CloudStack<->ONTAP volume/LUN/file data.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/AccessGroup.java Adds a model representing an iGroup/export-policy plus target hosts/scope.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java Implements SAN-side access group creation/deletion (iGroup) scaffolding.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java Implements NAS-side access group creation/deletion (export policy) logic.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java Implements ONTAP connectivity validation, volume create/delete, and helpers.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/SANStrategy.java Adds SAN strategy base class.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/NASStrategy.java Adds NAS strategy base class.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/provider/StorageProviderFactory.java Selects the correct strategy (Unified NAS/SAN) based on protocol/config.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/provider/OntapPrimaryDatastoreProvider.java Implements the primary datastore provider for ONTAP.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/listener/OntapHostListener.java Handles host connect/disconnect events to mount/unmount the primary pool.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java Implements pool lifecycle: initialize/create, attach cluster/zone, maintain, delete.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/response/OntapResponse.java Adds generic ONTAP list response wrapper.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/response/JobResponse.java Adds ONTAP job wrapper for async operations.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/VolumeSpaceLogicalSpace.java Adds volume logical-space model.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/VolumeSpace.java Adds volume space model.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/VolumeQosPolicy.java Adds QoS policy model.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Volume.java Adds ONTAP volume model.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Version.java Adds cluster version model.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Svm.java Adds SVM model (protocol enabled flags, aggregates, etc.).
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Qos.java Adds QoS wrapper model.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Policy.java Adds QoS policy model.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/OntapStorage.java Adds configuration model for ONTAP connectivity and protocol selection.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Nas.java Adds NAS settings model (path, export policy).
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/LunSpace.java Adds LUN space model.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/LunMap.java Adds LUN mapping model.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Lun.java Adds LUN model.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Job.java Adds job/status model.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/IscsiService.java Adds iSCSI service/target model.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/IpInterface.java Adds network interface (LIF) model.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Initiator.java Adds initiator model for iGroups.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Igroup.java Adds iGroup model.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/FileInfo.java Adds NAS file model.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/ExportRule.java Adds export rule model.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/ExportPolicy.java Adds export policy model.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Cluster.java Adds cluster model.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/AntiRansomware.java Adds anti-ransomware model field wrapper.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Aggregate.java Adds aggregate model with space/state helpers.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/VolumeFeignClient.java Adds volume REST API client interface.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SvmFeignClient.java Adds SVM REST API client interface.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java Adds SAN REST API client interface (iSCSI, iGroups, LUNs, maps).
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/NetworkFeignClient.java Adds network LIF REST API client interface.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/NASFeignClient.java Adds NAS REST API client interface (files, export policies).
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/JobFeignClient.java Adds job polling REST API client interface.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/ClusterFeignClient.java Adds cluster REST API client interface.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/AggregateFeignClient.java Adds aggregate REST API client interface.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/FeignConfiguration.java Adds Feign client wiring (HTTP client, encoder/decoder, retry, interceptor).
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/FeignClientFactory.java Central factory for building Feign clients with the shared configuration.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java Adds a primary datastore driver stub with basic capabilities.
plugins/storage/volume/ontap/pom.xml Adds the new module Maven build config and dependencies (Feign, Jackson, tests).
plugins/storage/volume/ontap/README.md Documents plugin features, requirements, configuration and limitations.
plugins/pom.xml Registers storage/volume/ontap as a plugins reactor module.
client/pom.xml Adds the ONTAP plugin artifact to the client module dependencies.

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

Comment on lines +140 to +147
String authHeader = Utility.generateAuthHeader(storage.getUsername(), storage.getPassword());
String svmName = storage.getSvmName();
String exportPolicyName = primaryDataStoreInfo.getDetails().get(Constants.EXPORT_POLICY_NAME);
String exportPolicyId = primaryDataStoreInfo.getDetails().get(Constants.EXPORT_POLICY_ID);

try {
nasFeignClient.deleteExportPolicyById(authHeader,exportPolicyId);
s_logger.info("deleteAccessGroup: Successfully deleted export policy '{}'", exportPolicyName);
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exportPolicyId is read from primaryDataStoreInfo.getDetails() and used directly in deleteExportPolicyById without validating it. If the detail is missing/empty (or details weren’t loaded), this will result in an invalid ONTAP call and a hard failure. Please validate exportPolicyId (and/or fall back to lookup by exportPolicyName) before attempting deletion, and handle the "already deleted" case gracefully.

Copilot uses AI. Check for mistakes.
Comment on lines +42 to +45
public static String generateAuthHeader (String username, String password) {
byte[] encodedBytes = Base64Utils.encode((username + AUTH_HEADER_COLON + password).getBytes());
return BASIC + StringUtils.SPACE + new String(encodedBytes);
}
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generateAuthHeader uses platform-default encoding for both getBytes() and new String(encodedBytes), which can lead to incorrect auth headers on non-UTF-8 platforms. Use an explicit charset (e.g., UTF-8) for both steps to make the header generation deterministic.

Copilot uses AI. Check for mistakes.
Comment on lines +401 to +410
case NFS3:
String ip = "";
for (HostVO host : hosts) {
if (host != null) {
ip = host.getStorageIpAddress() != null ? host.getStorageIpAddress().trim() : "";
if (ip.isEmpty() && host.getPrivateIpAddress() != null || host.getPrivateIpAddress().trim().isEmpty()) {
return false;
} else {
ip = ip.isEmpty() ? host.getPrivateIpAddress().trim() : ip;
}
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the NFS3 branch, the condition if (ip.isEmpty() && host.getPrivateIpAddress() != null || host.getPrivateIpAddress().trim().isEmpty()) has incorrect operator precedence and can throw an NPE when getPrivateIpAddress() is null. It also returns false when ip is empty but privateIpAddress is present (the opposite of what you want). Please rewrite this logic with proper null/empty checks and parentheses so it correctly falls back to privateIpAddress when storageIpAddress is missing.

Copilot uses AI. Check for mistakes.
Comment on lines +92 to +98
private SSLConnectionSocketFactory getSSLSocketFactory() {
try {
SSLContext sslContext = SSLContexts.custom().loadTrustMaterial(null, new TrustAllStrategy()).build();
return new SSLConnectionSocketFactory(sslContext, new NoopHostnameVerifier());
} catch (Exception ex) {
throw new RuntimeException(ex);
}
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Feign HTTP client is configured to trust all TLS certificates and disable hostname verification unconditionally (TrustAllStrategy + NoopHostnameVerifier). This undermines HTTPS security for ONTAP credentials and traffic. Please make TLS verification the default and only allow skipping verification via an explicit, opt-in configuration setting.

Copilot uses AI. Check for mistakes.
Comment on lines +101 to +109
public RequestInterceptor createRequestInterceptor() {
return template -> {
logger.info("Feign Request URL: {}", template.url());
logger.info("HTTP Method: {}", template.method());
logger.info("Headers: {}", template.headers());
if (template.body() != null) {
logger.info("Body: {}", new String(template.body(), StandardCharsets.UTF_8));
}
};
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The request interceptor logs full headers and request bodies at INFO. This will include the Authorization header (Basic auth) and may log sensitive payload fields, leaking credentials into logs. Please redact sensitive headers (at least Authorization) and consider lowering to DEBUG/TRACE or making request logging opt-in.

Copilot uses AI. Check for mistakes.
Comment on lines +105 to +110
if (Objects.equals(storage.getProtocol(), Constants.NFS) && !svm.getNfsEnabled()) {
s_logger.error("NFS protocol is not enabled on SVM " + svmName);
return false;
} else if (Objects.equals(storage.getProtocol(), Constants.ISCSI) && !svm.getIscsiEnabled()) {
s_logger.error("iSCSI protocol is not enabled on SVM " + svmName);
return false;
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

storage.getProtocol() is a ProtocolType, but the checks compare it to Constants.NFS/Constants.ISCSI (strings). This condition will never be true, so protocol enablement on the SVM is effectively not validated. Compare against ProtocolType.NFS3 / ProtocolType.ISCSI (or convert consistently) so an SVM with the protocol disabled fails validation as intended.

Copilot uses AI. Check for mistakes.
Comment on lines +128 to +138
s_logger.info("Selected aggregate: " + aggr.getName() + " for volume operations.");
this.aggregates = List.of(aggr);
break;
}
if (this.aggregates == null || this.aggregates.isEmpty()) {
s_logger.error("No suitable aggregates found on SVM " + svmName + " for volume creation.");
return false;
}

this.aggregates = aggrs;
s_logger.info("Successfully connected to ONTAP cluster and validated ONTAP details provided");
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The aggregate selection logic sets this.aggregates = List.of(aggr) when it finds a suitable aggregate, but then immediately overwrites it with this.aggregates = aggrs. This makes the earlier validation/selection misleading and can allow later operations to consider aggregates that were just deemed unsuitable. Either keep only the filtered/selected aggregates, or remove the initial selection loop and rely on createStorageVolume to choose the aggregate.

Copilot uses AI. Check for mistakes.
Comment on lines +144 to +149
} catch (Exception feignEx) {
String errMsg = feignEx.getMessage();
if (errMsg != null && errMsg.contains(("5374023"))) {
s_logger.warn("createAccessGroup: Igroup with name {} already exists. Fetching existing Igroup.", igroupName);
return createdAccessGroup;
}
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When ONTAP returns error code 5374023 (iGroup already exists), the code logs that it will fetch the existing iGroup but then returns an empty AccessGroup without setting the iGroup. This can hide real issues and makes the return value misleading. Either fetch and set the existing iGroup (e.g., via getIgroupResponse) or clearly document/return null and let callers handle the "already exists" case.

Copilot uses AI. Check for mistakes.
@DaanHoogland
Copy link
Contributor

@sandeeplocharla , can you review co-pilot’s sugestions and close/refuse or implement them?

@rajiv-jain-netapp
Copy link

Thanks @sandeeplocharla great work! An additional question about the functionalities, would SnapMirror be supported for volume replication? In case it is not, are there any plans to add support for it?

Hi @nvazquez , we have yet to plan the replication features for the upcoming PRs. However, SnapMirror support will be one of the key ONTAP capabilities that we intend to leverage for the plugin‑side workflow.

@nvazquez
Copy link
Contributor

Many thanks @rajiv-jain-netapp @sandeeplocharla

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants