CSTACKEX-127: Primary storage-pool is getting created even if desired data LIFs are not reachable by sandeeplocharla · Pull Request #76 · NetApp/cloudstack

sandeeplocharla · 2026-07-08T01:21:09Z

Choosing IpInterface based on its status and affinity to the chosen aggregate

Description

This PR...

Fixed an issue in picking an unavailable IP while creating the storage pool.
When CloudStack creates an ONTAP primary storage pool, it now picks the best available network interface (LIF) using a priority-based selection:
- Best case: Uses a LIF homed on the same node as the storage aggregate — optimal I/O, no warning
- Degraded case: All home-node LIFs are down but a failover LIF is running on that node — pool is created, admin is warned
- Fallback case: No LIF at all on the aggregate's node, pool is created using a LIF from a different node, admin is warned with a latency note
- Failure case: No usable LIF anywhere, pool creation fails with a clear error

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?

Note: The following images have been captured for NFS3, the same would be the case for iSCSI.

Clearly, by the virtue of free space available, the plugin would choose sti246_vsim_ocvs040d_aggr1 by default.

Scenario-1 [pool_P1]: Happy path; No LIFs were down.

The first best available LIF with current node and home node matching with the chosen node has been picked.

Scenario-2 [pool_P2_1]: LIFs on 040d node were down; with one LIF whose current node: 040d, while its home node: 040c

Scenario-3 [pool_P3]: None of the 040d node LIFs are UP. First best available LIF is picked from 040c.

…ggregate

Copilot

Pull request overview

This PR updates the ONTAP primary storage workflow to select a reachable/usable data LIF based on operational status and node affinity (aggregate home node vs. failover vs. cross-node fallback), and surfaces warnings via pool details/alerts when selection is degraded.

Changes:

Adds node-aware, status-aware LIF selection in StorageStrategy.getNetworkInterface() and returns both the chosen LIF IP + an optional warning.
Captures the chosen aggregate’s node during volume creation to bias LIF selection toward optimal locality.
Updates lifecycle + tests to handle the new (lifIp, warning) result and emits storage alerts when degraded selection occurs.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java	Expands unit tests for LIF status and node-affinity tiers; avoids mocking issues on newer JDKs.
plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java	Updates mocks for new `Pair<String,String>` LIF return type and scope behavior.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java	Adds helper to send storage alerts for degraded LIF selection.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java	Adds constants for aggregate node/space fields and LIF state/location/warning keys.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java	Implements tiered LIF selection and tracks `chosenAggregateNode` from aggregate selection.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java	Persists selected LIF + warning and emits an alert during pool initialization.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/IpInterface.java	Extends model with `state`, `enabled`, and `location` to support selection logic.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Aggregate.java	Extends model with `node` and setters needed for reading/constructing detailed aggregate info.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/AggregateFeignClient.java	Adds `@QueryMap` to request specific aggregate fields (node/space/state).

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

rajiv-jain-netapp · 2026-07-10T13:11:09Z

+            Pair<String, String> lifResult;
+            try {
+                lifResult = storageStrategy.getNetworkInterface();
+            } catch (Exception e) {
+                logger.error("Exception occurred while retrieving network interface for pool: " + storagePoolName, e);
+                throw new CloudRuntimeException("Failed to retrieve Data LIF from ONTAP: " + e.getMessage(), e);
+            }


this comment has value, lets fix this

+        if (aggrChosen.getNode() != null) {
+            this.chosenAggregateNode = aggrChosen.getNode().getName();
+        }


rajiv-jain-netapp · 2026-07-10T13:27:21Z

+            for (IpInterface iface : response.getRecords()) {
+                if (!Boolean.TRUE.equals(iface.getEnabled()) || !OntapStorageConstants.LIF_STATE_UP.equals(iface.getState())) {
+                    continue;
+                }
+                if (!isIPv4Address(iface.getIp().getAddress())) {
+                    continue;
+                }


address this comment as well

+        } catch (Exception e) {
            logger.error("Exception while retrieving network interfaces: ", e);
            throw new CloudRuntimeException("Failed to retrieve network interfaces: " + e.getMessage());
        }


github-actions · 2026-07-10T04:59:09Z

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

rajiv-jain-netapp · 2026-07-10T13:11:09Z

+            Pair<String, String> lifResult;
+            try {
+                lifResult = storageStrategy.getNetworkInterface();
+            } catch (Exception e) {
+                logger.error("Exception occurred while retrieving network interface for pool: " + storagePoolName, e);
+                throw new CloudRuntimeException("Failed to retrieve Data LIF from ONTAP: " + e.getMessage(), e);
+            }


this comment has value, lets fix this

rajiv-jain-netapp · 2026-07-10T13:23:55Z

                continue;
            }

            if (aggrResp.getSpace() == null || aggrResp.getAvailableBlockStorageSpace() == null) {


you are not fetcing availableblockStorageSpace now? would this work?

rajiv-jain-netapp · 2026-07-10T13:26:00Z

+                    OntapStorageConstants.IP_ADDRESS + OntapStorageConstants.COMMA
+                    + OntapStorageConstants.STATE + OntapStorageConstants.COMMA
+                    + OntapStorageConstants.LIF_ENABLED + OntapStorageConstants.COMMA
+                    + OntapStorageConstants.LIF_LOCATION_HOME_NODE + OntapStorageConstants.COMMA


what is the diff between home_node and node values?

rajiv-jain-netapp · 2026-07-10T13:27:21Z

+            for (IpInterface iface : response.getRecords()) {
+                if (!Boolean.TRUE.equals(iface.getEnabled()) || !OntapStorageConstants.LIF_STATE_UP.equals(iface.getState())) {
+                    continue;
+                }
+                if (!isIPv4Address(iface.getIp().getAddress())) {
+                    continue;
+                }


address this comment as well

rajiv-jain-netapp · 2026-07-10T13:29:12Z

+                if (!isIPv4Address(iface.getIp().getAddress())) {
+                    continue;
+                }
+                if (chosenAggregateNode != null) {


add code comment for the behaviour, you are trying to implement here

Choosing IpInterface based on its status and affinity to the chosen a…

2b19269

…ggregate

sandeeplocharla self-assigned this Jul 8, 2026

Copilot AI review requested due to automatic review settings July 8, 2026 01:21

sandeeplocharla requested review from piyush5netapp, rajiv-jain-netapp and suryag1201 as code owners July 8, 2026 01:21

Copilot started reviewing on behalf of sandeeplocharla July 8, 2026 01:21 View session

Copilot AI reviewed Jul 8, 2026

View reviewed changes

github-actions Bot added the status:has-conflicts label Jul 10, 2026

rajiv-jain-netapp reviewed Jul 10, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

CSTACKEX-127: Primary storage-pool is getting created even if desired data LIFs are not reachable#76

CSTACKEX-127: Primary storage-pool is getting created even if desired data LIFs are not reachable#76
sandeeplocharla wants to merge 1 commit into
mainfrom
feature/CSTACKEX-127

sandeeplocharla commented Jul 8, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

rajiv-jain-netapp Jul 10, 2026

Uh oh!

rajiv-jain-netapp Jul 10, 2026

Uh oh!

github-actions Bot commented Jul 10, 2026

Uh oh!

rajiv-jain-netapp Jul 10, 2026

Uh oh!

rajiv-jain-netapp Jul 10, 2026

Uh oh!

rajiv-jain-netapp Jul 10, 2026

Uh oh!

rajiv-jain-netapp Jul 10, 2026

Uh oh!

rajiv-jain-netapp Jul 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

sandeeplocharla commented Jul 8, 2026 • 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?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

rajiv-jain-netapp Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

rajiv-jain-netapp Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 10, 2026

Uh oh!

rajiv-jain-netapp Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

rajiv-jain-netapp Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

rajiv-jain-netapp Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

rajiv-jain-netapp Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

rajiv-jain-netapp Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sandeeplocharla commented Jul 8, 2026 •

edited

Loading