Skip to content

Commit 31e1a64

Browse files
CSTACKEX-127: Primary storage-pool is getting created even if desired data LIFs are not reachable (#76)
Choosing IpInterface based on its status and affinity to the chosen aggregate This PR... 1. Fixed an issue in picking an unavailable IP while creating the storage pool. 2. 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 <!--- ******************************************************************************* --> <!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. --> <!--- PLEASE PUT AN 'X' in only **ONE** box --> <!--- ******************************************************************************* --> - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [X] 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) - [X] Major - [ ] Minor - [ ] BLOCKER - [ ] Critical - [X] Major - [ ] Minor - [ ] Trivial Note: The following images have been captured for NFS3, the same would be the case for iSCSI. <img width="487" height="661" alt="Screenshot 2026-07-08 at 6 55 32 AM" src="https://github.com/user-attachments/assets/e48a42c7-9914-4563-bc27-f1ad6501629c" /> 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.** <img width="1122" height="742" alt="Screenshot 2026-07-07 at 10 26 34 PM" src="https://github.com/user-attachments/assets/8b748425-c548-4d03-8265-2b86a568f45b" /> <img width="1122" height="419" alt="Screenshot 2026-07-07 at 10 26 49 PM" src="https://github.com/user-attachments/assets/45177fc2-4e87-4ead-928e-659e50504d9d" /> <img width="1032" height="432" alt="Screenshot 2026-07-07 at 10 27 48 PM" src="https://github.com/user-attachments/assets/6bbe3533-2081-429d-ba1f-fa1a8731dd3f" /> 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`** <img width="1032" height="765" alt="Screenshot 2026-07-07 at 10 28 38 PM" src="https://github.com/user-attachments/assets/adb8e4ff-670b-49d2-b8ca-1d7558f34643" /> <img width="1032" height="415" alt="Screenshot 2026-07-07 at 10 28 50 PM" src="https://github.com/user-attachments/assets/6d982076-adba-4458-960b-dc0066d1c5a6" /> <img width="1032" height="380" alt="Screenshot 2026-07-07 at 11 05 22 PM" src="https://github.com/user-attachments/assets/c73957ec-a82b-4093-bf88-43da515d8663" /> <img width="1032" height="220" alt="Screenshot 2026-07-07 at 11 05 43 PM" src="https://github.com/user-attachments/assets/f179e0b7-808f-4643-9d89-7ebb52fa1d33" /> **Scenario-3 [pool_P3]: None of the `040d` node LIFs are UP. First best available LIF is picked from `040c`.** <img width="867" height="764" alt="Screenshot 2026-07-07 at 11 07 00 PM" src="https://github.com/user-attachments/assets/59a05c54-e4f7-468a-9682-9fc392ba45c9" /> <img width="867" height="420" alt="Screenshot 2026-07-07 at 11 07 22 PM" src="https://github.com/user-attachments/assets/87c4a008-17d8-4bff-9335-14e0707f9ddd" /> <img width="1029" height="425" alt="Screenshot 2026-07-07 at 11 08 30 PM" src="https://github.com/user-attachments/assets/2f2888d0-0a7a-4e29-91d4-d03f1bf4440e" /> <img width="1029" height="228" alt="Screenshot 2026-07-07 at 11 08 54 PM" src="https://github.com/user-attachments/assets/5cf00e5f-0a46-4010-899d-14c605b676d2" />
1 parent c027e2d commit 31e1a64

10 files changed

Lines changed: 899 additions & 142 deletions

File tree

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/AggregateFeignClient.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,14 @@
1919

2020
package org.apache.cloudstack.storage.feign.client;
2121

22+
import java.util.Map;
23+
2224
import org.apache.cloudstack.storage.feign.model.Aggregate;
2325
import org.apache.cloudstack.storage.feign.model.response.OntapResponse;
26+
2427
import feign.Headers;
2528
import feign.Param;
29+
import feign.QueryMap;
2630
import feign.RequestLine;
2731

2832
public interface AggregateFeignClient {
@@ -33,5 +37,6 @@ public interface AggregateFeignClient {
3337

3438
@RequestLine("GET /api/storage/aggregates/{uuid}")
3539
@Headers({"Authorization: {authHeader}"})
36-
Aggregate getAggregateByUUID(@Param("authHeader") String authHeader, @Param("uuid") String uuid);
40+
Aggregate getAggregateByUUID(@Param("authHeader") String authHeader, @Param("uuid") String uuid,
41+
@QueryMap Map<String, Object> queryParams);
3742
}

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

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,14 @@
1919

2020
package org.apache.cloudstack.storage.feign.model;
2121

22+
import java.util.Objects;
23+
24+
import com.fasterxml.jackson.annotation.JsonCreator;
2225
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
2326
import com.fasterxml.jackson.annotation.JsonInclude;
2427
import com.fasterxml.jackson.annotation.JsonProperty;
25-
import com.fasterxml.jackson.annotation.JsonCreator;
2628
import com.fasterxml.jackson.annotation.JsonValue;
2729

28-
import java.util.Objects;
29-
3030
@JsonIgnoreProperties(ignoreUnknown = true)
3131
@JsonInclude(JsonInclude.Include.NON_NULL)
3232
public class Aggregate {
@@ -77,6 +77,17 @@ public int hashCode() {
7777
@JsonProperty("space")
7878
private AggregateSpace space = null;
7979

80+
@JsonProperty("node")
81+
private Node node = null;
82+
83+
84+
public Node getNode() {
85+
return node;
86+
}
87+
88+
public void setNode(Node node) {
89+
this.node = node;
90+
}
8091

8192
public Aggregate name(String name) {
8293
this.name = name;
@@ -107,10 +118,18 @@ public StateEnum getState() {
107118
return state;
108119
}
109120

121+
public void setState(StateEnum state) {
122+
this.state = state;
123+
}
124+
110125
public AggregateSpace getSpace() {
111126
return space;
112127
}
113128

129+
public void setSpace(AggregateSpace space) {
130+
this.space = space;
131+
}
132+
114133
public Double getAvailableBlockStorageSpace() {
115134
if (space != null && space.blockStorage != null) {
116135
return space.blockStorage.available;
@@ -148,9 +167,32 @@ public String toString() {
148167
return "DiskAggregates [name=" + name + ", uuid=" + uuid + "]";
149168
}
150169

170+
@JsonIgnoreProperties(ignoreUnknown = true)
171+
@JsonInclude(JsonInclude.Include.NON_NULL)
172+
public static class Node {
173+
@JsonProperty("name")
174+
private String name;
175+
176+
public String getName() {
177+
return name;
178+
}
179+
180+
public void setName(String name) {
181+
this.name = name;
182+
}
183+
}
184+
151185
public static class AggregateSpace {
152186
@JsonProperty("block_storage")
153187
private AggregateSpaceBlockStorage blockStorage = null;
188+
189+
public AggregateSpaceBlockStorage getBlockStorage() {
190+
return blockStorage;
191+
}
192+
193+
public void setBlockStorage(AggregateSpaceBlockStorage blockStorage) {
194+
this.blockStorage = blockStorage;
195+
}
154196
}
155197

156198
public static class AggregateSpaceBlockStorage {
@@ -160,6 +202,14 @@ public static class AggregateSpaceBlockStorage {
160202
private Double size = null;
161203
@JsonProperty("used")
162204
private Double used = null;
205+
206+
public Double getAvailable() {
207+
return available;
208+
}
209+
210+
public void setAvailable(Double available) {
211+
this.available = available;
212+
}
163213
}
164214

165215
}

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

Lines changed: 83 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,13 @@
1919

2020
package org.apache.cloudstack.storage.feign.model;
2121

22+
import java.util.List;
23+
import java.util.Objects;
24+
2225
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
2326
import com.fasterxml.jackson.annotation.JsonInclude;
2427
import com.fasterxml.jackson.annotation.JsonProperty;
2528

26-
import java.util.List;
27-
import java.util.Objects;
28-
2929
@JsonIgnoreProperties(ignoreUnknown = true)
3030
@JsonInclude(JsonInclude.Include.NON_NULL)
3131
public class IpInterface {
@@ -44,6 +44,15 @@ public class IpInterface {
4444
@JsonProperty("services")
4545
private List<String> services;
4646

47+
@JsonProperty("state")
48+
private String state;
49+
50+
@JsonProperty("enabled")
51+
private Boolean enabled;
52+
53+
@JsonProperty("location")
54+
private Location location;
55+
4756
// Getters and setters
4857
public String getUuid() {
4958
return uuid;
@@ -85,6 +94,30 @@ public void setServices(List<String> services) {
8594
this.services = services;
8695
}
8796

97+
public String getState() {
98+
return state;
99+
}
100+
101+
public void setState(String state) {
102+
this.state = state;
103+
}
104+
105+
public Boolean getEnabled() {
106+
return enabled;
107+
}
108+
109+
public void setEnabled(Boolean enabled) {
110+
this.enabled = enabled;
111+
}
112+
113+
public Location getLocation() {
114+
return location;
115+
}
116+
117+
public void setLocation(Location location) {
118+
this.location = location;
119+
}
120+
88121
@Override
89122
public boolean equals(Object o) {
90123
if (this == o) {
@@ -98,12 +131,14 @@ public boolean equals(Object o) {
98131
Objects.equals(name, that.name) &&
99132
Objects.equals(ip, that.ip) &&
100133
Objects.equals(svm, that.svm) &&
101-
Objects.equals(services, that.services);
134+
Objects.equals(services, that.services) &&
135+
Objects.equals(state, that.state) &&
136+
Objects.equals(enabled, that.enabled);
102137
}
103138

104139
@Override
105140
public int hashCode() {
106-
return Objects.hash(uuid, name, ip, svm, services);
141+
return Objects.hash(uuid, name, ip, svm, services, state, enabled);
107142
}
108143

109144
@Override
@@ -114,9 +149,52 @@ public String toString() {
114149
", ip=" + ip +
115150
", svm=" + svm +
116151
", services=" + services +
152+
", state='" + state + '\'' +
153+
", enabled=" + enabled +
117154
'}';
118155
}
119156

157+
@JsonIgnoreProperties(ignoreUnknown = true)
158+
@JsonInclude(JsonInclude.Include.NON_NULL)
159+
public static class Node {
160+
@JsonProperty("name")
161+
private String name;
162+
163+
public String getName() {
164+
return name;
165+
}
166+
167+
public void setName(String name) {
168+
this.name = name;
169+
}
170+
}
171+
172+
@JsonIgnoreProperties(ignoreUnknown = true)
173+
@JsonInclude(JsonInclude.Include.NON_NULL)
174+
public static class Location {
175+
@JsonProperty("home_node")
176+
private Node homeNode;
177+
178+
@JsonProperty("node")
179+
private Node node;
180+
181+
public Node getHomeNode() {
182+
return homeNode;
183+
}
184+
185+
public void setHomeNode(Node homeNode) {
186+
this.homeNode = homeNode;
187+
}
188+
189+
public Node getNode() {
190+
return node;
191+
}
192+
193+
public void setNode(Node node) {
194+
this.node = node;
195+
}
196+
}
197+
120198
// Nested class for IP information
121199
@JsonIgnoreProperties(ignoreUnknown = true)
122200
@JsonInclude(JsonInclude.Include.NON_NULL)

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java‎

Lines changed: 56 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -19,31 +19,27 @@
1919

2020
package org.apache.cloudstack.storage.lifecycle;
2121

22-
import org.apache.cloudstack.engine.subsystem.api.storage.Scope;
23-
import com.cloud.agent.api.StoragePoolInfo;
24-
import com.cloud.dc.ClusterVO;
25-
import com.cloud.dc.dao.ClusterDao;
26-
import com.cloud.exception.InvalidParameterValueException;
27-
import com.cloud.host.HostVO;
28-
import com.cloud.hypervisor.Hypervisor;
29-
import com.cloud.resource.ResourceManager;
30-
import com.cloud.storage.Storage;
31-
import com.cloud.storage.StorageManager;
32-
import com.cloud.storage.StoragePool;
33-
import com.cloud.storage.StoragePoolAutomation;
34-
import com.cloud.utils.exception.CloudRuntimeException;
35-
import com.google.common.base.Preconditions;
22+
import java.util.ArrayList;
23+
import java.util.HashSet;
24+
import java.util.List;
25+
import java.util.Map;
26+
import java.util.Set;
27+
import java.util.UUID;
28+
29+
import javax.inject.Inject;
30+
3631
import org.apache.cloudstack.api.ApiConstants;
3732
import org.apache.cloudstack.engine.subsystem.api.storage.ClusterScope;
3833
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
3934
import org.apache.cloudstack.engine.subsystem.api.storage.HostScope;
4035
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreInfo;
4136
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreLifeCycle;
4237
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreParameters;
38+
import org.apache.cloudstack.engine.subsystem.api.storage.Scope;
4339
import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope;
4440
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
45-
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
4641
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDetailsDao;
42+
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
4743
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
4844
import org.apache.cloudstack.storage.datastore.lifecycle.BasePrimaryDataStoreLifeCycleImpl;
4945
import org.apache.cloudstack.storage.feign.model.OntapStorage;
@@ -59,13 +55,21 @@
5955
import org.apache.logging.log4j.LogManager;
6056
import org.apache.logging.log4j.Logger;
6157

62-
import javax.inject.Inject;
63-
import java.util.ArrayList;
64-
import java.util.HashSet;
65-
import java.util.List;
66-
import java.util.Map;
67-
import java.util.Set;
68-
import java.util.UUID;
58+
import com.cloud.agent.api.StoragePoolInfo;
59+
import com.cloud.alert.AlertManager;
60+
import com.cloud.dc.ClusterVO;
61+
import com.cloud.dc.dao.ClusterDao;
62+
import com.cloud.exception.InvalidParameterValueException;
63+
import com.cloud.host.HostVO;
64+
import com.cloud.hypervisor.Hypervisor;
65+
import com.cloud.resource.ResourceManager;
66+
import com.cloud.storage.Storage;
67+
import com.cloud.storage.StorageManager;
68+
import com.cloud.storage.StoragePool;
69+
import com.cloud.storage.StoragePoolAutomation;
70+
import com.cloud.utils.Pair;
71+
import com.cloud.utils.exception.CloudRuntimeException;
72+
import com.google.common.base.Preconditions;
6973

7074
public class OntapPrimaryDatastoreLifecycle extends BasePrimaryDataStoreLifeCycleImpl implements PrimaryDataStoreLifeCycle {
7175
@Inject private ClusterDao _clusterDao;
@@ -76,6 +80,7 @@ public class OntapPrimaryDatastoreLifecycle extends BasePrimaryDataStoreLifeCycl
7680
@Inject private StoragePoolAutomation _storagePoolAutomation;
7781
@Inject private PrimaryDataStoreDao storagePoolDao;
7882
@Inject private StoragePoolDetailsDao storagePoolDetailsDao;
83+
@Inject private AlertManager _alertMgr;
7984
private static final Logger logger = LogManager.getLogger(OntapPrimaryDatastoreLifecycle.class);
8085

8186
private static final long ONTAP_MIN_VOLUME_SIZE_IN_BYTES = 1677721600L;
@@ -135,13 +140,6 @@ public DataStore initialize(Map<String, Object> dsInfos) {
135140
StorageStrategy storageStrategy = StorageProviderFactory.getStrategy(ontapStorage);
136141
boolean isValid = storageStrategy.connect();
137142
if (isValid) {
138-
// Get the DataLIF for data access
139-
String dataLIF = storageStrategy.getNetworkInterface();
140-
if (dataLIF == null || dataLIF.isEmpty()) {
141-
throw new CloudRuntimeException("Failed to retrieve Data LIF from ONTAP, cannot create primary storage");
142-
}
143-
logger.info("Using Data LIF for storage access: " + dataLIF);
144-
details.put(OntapStorageConstants.DATA_LIF, dataLIF);
145143
if (storageStrategy.getResolvedSvmUuid() != null && !storageStrategy.getResolvedSvmUuid().isEmpty()) {
146144
details.put(OntapStorageConstants.SVM_UUID, storageStrategy.getResolvedSvmUuid());
147145
}
@@ -160,6 +158,15 @@ public DataStore initialize(Map<String, Object> dsInfos) {
160158
logger.error("Exception occurred while creating ONTAP volume: " + storagePoolName, e);
161159
throw new CloudRuntimeException("Failed to create ONTAP volume: " + storagePoolName + ". Error: " + e.getMessage(), e);
162160
}
161+
162+
Pair<String, String> lifResult;
163+
try {
164+
lifResult = storageStrategy.getNetworkInterface();
165+
} catch (Exception e) {
166+
logger.error("Exception occurred while retrieving network interface for pool: " + storagePoolName, e);
167+
throw new CloudRuntimeException("Failed to retrieve Data LIF from ONTAP: " + e.getMessage(), e);
168+
}
169+
processDataLifSelection(lifResult, details, storagePoolName, zoneId, podId);
163170
} else {
164171
throw new CloudRuntimeException("ONTAP details validation failed, cannot create primary storage");
165172
}
@@ -275,6 +282,26 @@ private long validateInitializeInputs(Long capacityBytes, Long podId, Long clust
275282
return capacityBytes;
276283
}
277284

285+
private void processDataLifSelection(Pair<String, String> lifResult, Map<String, String> details,
286+
String storagePoolName, Long zoneId, Long podId) {
287+
String dataLIF = lifResult.first();
288+
if (dataLIF == null || dataLIF.isEmpty()) {
289+
throw new CloudRuntimeException("Failed to retrieve Data LIF from ONTAP, cannot create primary storage");
290+
}
291+
logger.info("Using Data LIF for storage access: " + dataLIF);
292+
details.put(OntapStorageConstants.DATA_LIF, dataLIF);
293+
294+
// Persist LIF warning as a pool detail and fire a storage alert so the user is informed
295+
if (lifResult.second() != null) {
296+
String lifWarning = lifResult.second();
297+
details.put(OntapStorageConstants.LIF_WARNING, lifWarning);
298+
logger.warn("LIF selection warning for pool '" + storagePoolName + "': " + lifWarning);
299+
String alertSubject = "ONTAP Storage Pool '" + storagePoolName + "': "
300+
+ lifWarning.split(OntapStorageConstants.SEMICOLON)[0].trim();
301+
OntapStorageUtils.sendStorageAlert(_alertMgr, zoneId, podId, alertSubject, lifWarning);
302+
}
303+
}
304+
278305
@Override
279306
public boolean attachCluster(DataStore dataStore, ClusterScope scope) {
280307
logger.debug("In attachCluster for ONTAP primary storage");

0 commit comments

Comments
 (0)