Skip to content

Commit d0beb93

Browse files
suryag1201Gupta, Surya
andauthored
Bugfix/CSTACKEX-129 Primary storage-pool is not failing even if NFS3/ISCSI protocol is not enabled at the SVM(#61)
### Description This PR... Primary storage-pool is not failing even if NFS3/ISCSI protocol is not enabled at the storage VM <!-- For new features, provide link to FS, dev ML discussion etc. --> <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. --> <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged --> <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" --> <!-- Fixes: # --> <!--- ******************************************************************************* --> <!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. --> <!--- PLEASE PUT AN 'X' in only **ONE** box --> <!--- ******************************************************************************* --> ### Types of changes - [ ] 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) ### Feature/Enhancement Scale or Bug Severity #### Feature/Enhancement Scale - [ ] Major - [ ] Minor #### Bug Severity - [ ] BLOCKER - [ ] Critical - [ ] Major - [x] Minor - [ ] Trivial ### Screenshots (if appropriate): <img width="1707" height="848" alt="Screenshot 2026-05-24 at 8 45 02 PM" src="https://github.com/user-attachments/assets/9dbc8386-29b4-4ce2-b863-6933c1c0a8d6" /> <img width="1697" height="937" alt="Screenshot 2026-05-24 at 8 44 46 PM" src="https://github.com/user-attachments/assets/5c8abce4-0343-453e-a980-bd7124034083" /> ### How Has This Been Tested? <!-- Please describe in detail how you tested your changes. --> <!-- Include details of your testing environment, and the tests you ran to --> #### How did you try to break this feature and the system with this change? <!-- see how your change affects other areas of the code, etc. --> <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document --> --------- Co-authored-by: Gupta, Surya <Surya.Gupta@netapp.com>
1 parent 8ae97bf commit d0beb93

4 files changed

Lines changed: 57 additions & 48 deletions

File tree

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

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@
2323
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
2424
import com.fasterxml.jackson.annotation.JsonInclude;
2525
import com.fasterxml.jackson.annotation.JsonProperty;
26+
import com.fasterxml.jackson.annotation.JsonSetter;
2627

2728
import java.util.List;
29+
import java.util.Map;
2830
import java.util.Objects;
2931

3032
@JsonIgnoreProperties(ignoreUnknown = true)
@@ -36,13 +38,10 @@ public class Svm {
3638
@JsonProperty("name")
3739
private String name = null;
3840

39-
@JsonProperty("iscsi.enabled")
4041
private Boolean iscsiEnabled = null;
4142

42-
@JsonProperty("fcp.enabled")
4343
private Boolean fcpEnabled = null;
4444

45-
@JsonProperty("nfs.enabled")
4645
private Boolean nfsEnabled = null;
4746

4847
@JsonProperty("aggregates")
@@ -73,28 +72,43 @@ public void setName(String name) {
7372
this.name = name;
7473
}
7574

75+
public Boolean getNfsEnabled() {
76+
return Boolean.TRUE.equals(nfsEnabled);
77+
}
78+
79+
public void setNfsEnabled(Boolean nfsEnabled) {
80+
this.nfsEnabled = nfsEnabled;
81+
}
82+
83+
@JsonSetter("nfs")
84+
public void setNfs(Map<String, Object> nfs) {
85+
this.nfsEnabled = nfs != null ? Boolean.TRUE.equals(nfs.get("enabled")) : false;
86+
}
87+
7688
public Boolean getIscsiEnabled() {
77-
return iscsiEnabled;
89+
return Boolean.TRUE.equals(iscsiEnabled);
7890
}
7991

8092
public void setIscsiEnabled(Boolean iscsiEnabled) {
8193
this.iscsiEnabled = iscsiEnabled;
8294
}
8395

96+
@JsonSetter("iscsi")
97+
public void setIscsi(Map<String, Object> iscsi) {
98+
this.iscsiEnabled = iscsi != null ? Boolean.TRUE.equals(iscsi.get("enabled")) : false;
99+
}
100+
84101
public Boolean getFcpEnabled() {
85-
return fcpEnabled;
102+
return Boolean.TRUE.equals(fcpEnabled);
86103
}
87104

88105
public void setFcpEnabled(Boolean fcpEnabled) {
89106
this.fcpEnabled = fcpEnabled;
90107
}
91108

92-
public Boolean getNfsEnabled() {
93-
return nfsEnabled;
94-
}
95-
96-
public void setNfsEnabled(Boolean nfsEnabled) {
97-
this.nfsEnabled = nfsEnabled;
109+
@JsonSetter("fcp")
110+
public void setFcp(Map<String, Object> fcp) {
111+
this.fcpEnabled = fcp != null ? Boolean.TRUE.equals(fcp.get("enabled")) : false;
98112
}
99113

100114
public List<Aggregate> getAggregates() {

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

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -108,31 +108,32 @@ public boolean connect() {
108108
Svm svm = new Svm();
109109
logger.info("Fetching the SVM details...");
110110
Map<String, Object> queryParams = Map.of(OntapStorageConstants.NAME, svmName, OntapStorageConstants.FIELDS, OntapStorageConstants.AGGREGATES +
111-
OntapStorageConstants.COMMA + OntapStorageConstants.STATE);
111+
OntapStorageConstants.COMMA + OntapStorageConstants.STATE +
112+
OntapStorageConstants.COMMA + OntapStorageConstants.NFS_ENABLED + OntapStorageConstants.COMMA + OntapStorageConstants.ISCSI_ENABLED);
112113
OntapResponse<Svm> svms = svmFeignClient.getSvmResponse(queryParams, authHeader);
113114
if (svms != null && svms.getRecords() != null && !svms.getRecords().isEmpty()) {
114115
svm = svms.getRecords().get(0);
115116
} else {
116-
logger.error("No SVM found on the ONTAP cluster by the name" + svmName + ".");
117-
return false;
117+
logger.error("No SVM found on the ONTAP cluster by the name " + svmName + ".");
118+
throw new CloudRuntimeException("No SVM found on the ONTAP cluster by the name " + svmName + ".");
118119
}
119120

120121
logger.info("Validating SVM state and protocol settings...");
121122
if (!Objects.equals(svm.getState(), OntapStorageConstants.RUNNING)) {
122123
logger.error("SVM " + svmName + " is not in running state.");
123-
return false;
124+
throw new CloudRuntimeException("SVM " + svmName + " is not in running state.");
124125
}
125-
if (Objects.equals(storage.getProtocol(), OntapStorageConstants.NFS) && !svm.getNfsEnabled()) {
126+
if (Objects.equals(storage.getProtocol(), ProtocolType.NFS3) && !svm.getNfsEnabled()) {
126127
logger.error("NFS protocol is not enabled on SVM " + svmName);
127-
return false;
128-
} else if (Objects.equals(storage.getProtocol(), OntapStorageConstants.ISCSI) && !svm.getIscsiEnabled()) {
129-
logger.error("iSCSI protocol is not enabled on SVM " + svmName);
130-
return false;
128+
throw new CloudRuntimeException("NFS protocol is not enabled on SVM " + svmName);
129+
} else if (Objects.equals(storage.getProtocol(), ProtocolType.ISCSI) && !svm.getIscsiEnabled()) {
130+
logger.error("ISCSI protocol is not enabled on SVM " + svmName);
131+
throw new CloudRuntimeException("ISCSI protocol is not enabled on SVM " + svmName);
131132
}
132133
List<Aggregate> aggrs = svm.getAggregates();
133134
if (aggrs == null || aggrs.isEmpty()) {
134135
logger.error("No aggregates are assigned to SVM " + svmName);
135-
return false;
136+
throw new CloudRuntimeException("No aggregates are assigned to SVM " + svmName);
136137
}
137138
for (Aggregate aggr : aggrs) {
138139
logger.debug("Found aggregate: " + aggr.getName() + " with UUID: " + aggr.getUuid());
@@ -155,13 +156,13 @@ public boolean connect() {
155156
}
156157
if (this.aggregates == null || this.aggregates.isEmpty()) {
157158
logger.error("No suitable aggregates found on SVM " + svmName + " for volume creation.");
158-
return false;
159+
throw new CloudRuntimeException("No suitable aggregates found on SVM " + svmName + " for volume creation.");
159160
}
160161

161162
logger.info("Successfully connected to ONTAP cluster and validated ONTAP details provided");
162163
} catch (Exception e) {
163164
logger.error("Failed to connect to ONTAP cluster: " + e.getMessage(), e);
164-
return false;
165+
throw new CloudRuntimeException("Failed to connect to ONTAP cluster: " + e.getMessage(), e);
165166
}
166167
return true;
167168
}

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ public class OntapStorageConstants {
3030

3131
public static final String NFS = "nfs";
3232
public static final String ISCSI = "iscsi";
33+
34+
public static final String NFS_ENABLED = "nfs.enabled";
35+
public static final String ISCSI_ENABLED = "iscsi.enabled";
3336
public static final String SIZE = "size";
3437
public static final String PROTOCOL = "protocol";
3538
public static final String SVM_NAME = "svmName";

plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@
5353
import java.util.Map;
5454

5555
import static org.junit.jupiter.api.Assertions.assertEquals;
56-
import static org.junit.jupiter.api.Assertions.assertFalse;
5756
import static org.junit.jupiter.api.Assertions.assertNotNull;
5857
import static org.junit.jupiter.api.Assertions.assertThrows;
5958
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -239,11 +238,9 @@ public void testConnect_svmNotFound() {
239238

240239
when(svmFeignClient.getSvmResponse(anyMap(), anyString())).thenReturn(svmResponse);
241240

242-
// Execute
243-
boolean result = storageStrategy.connect();
244-
245-
// Verify
246-
assertFalse(result, "connect() should return false when SVM is not found");
241+
// Execute & Verify
242+
CloudRuntimeException ex = assertThrows(CloudRuntimeException.class, () -> storageStrategy.connect());
243+
assertTrue(ex.getMessage().contains("No SVM found"));
247244
}
248245

249246
@Test
@@ -259,11 +256,9 @@ public void testConnect_svmNotRunning() {
259256

260257
when(svmFeignClient.getSvmResponse(anyMap(), anyString())).thenReturn(svmResponse);
261258

262-
// Execute
263-
boolean result = storageStrategy.connect();
264-
265-
// Verify
266-
assertFalse(result, "connect() should return false when SVM is not running");
259+
// Execute & Verify
260+
CloudRuntimeException ex = assertThrows(CloudRuntimeException.class, () -> storageStrategy.connect());
261+
assertTrue(ex.getMessage().contains("not in running state"));
267262
}
268263

269264
@Test
@@ -285,8 +280,8 @@ public void testConnect_nfsNotEnabled() {
285280
when(svmFeignClient.getSvmResponse(anyMap(), anyString())).thenReturn(svmResponse);
286281

287282
// Execute & Verify
288-
boolean result = storageStrategy.connect();
289-
assertFalse(result, "connect() should fail when NFS is disabled");
283+
CloudRuntimeException ex = assertThrows(CloudRuntimeException.class, () -> storageStrategy.connect());
284+
assertTrue(ex.getMessage().contains("NFS protocol is not enabled"));
290285
}
291286

292287
@Test
@@ -314,8 +309,8 @@ public void testConnect_iscsiNotEnabled() {
314309
when(svmFeignClient.getSvmResponse(anyMap(), anyString())).thenReturn(svmResponse);
315310

316311
// Execute & Verify
317-
boolean result = storageStrategy.connect();
318-
assertFalse(result, "connect() should fail when iSCSI is disabled");
312+
CloudRuntimeException ex = assertThrows(CloudRuntimeException.class, () -> storageStrategy.connect());
313+
assertTrue(ex.getMessage().contains("ISCSI protocol is not enabled"));
319314
}
320315

321316
@Test
@@ -332,23 +327,19 @@ public void testConnect_noAggregates() {
332327

333328
when(svmFeignClient.getSvmResponse(anyMap(), anyString())).thenReturn(svmResponse);
334329

335-
// Execute
336-
boolean result = storageStrategy.connect();
337-
338-
// Verify
339-
assertFalse(result, "connect() should return false when no aggregates are assigned");
330+
// Execute & Verify
331+
CloudRuntimeException ex = assertThrows(CloudRuntimeException.class, () -> storageStrategy.connect());
332+
assertTrue(ex.getMessage().contains("No aggregates"));
340333
}
341334

342335
@Test
343336
public void testConnect_nullSvmResponse() {
344337
// Setup
345338
when(svmFeignClient.getSvmResponse(anyMap(), anyString())).thenReturn(null);
346339

347-
// Execute
348-
boolean result = storageStrategy.connect();
349-
350-
// Verify
351-
assertFalse(result, "connect() should return false when SVM response is null");
340+
// Execute & Verify
341+
CloudRuntimeException ex = assertThrows(CloudRuntimeException.class, () -> storageStrategy.connect());
342+
assertTrue(ex.getMessage().contains("No SVM found"));
352343
}
353344

354345
// ========== createStorageVolume() Tests ==========

0 commit comments

Comments
 (0)