Skip to content

Commit 695dac6

Browse files
sandeeplocharlasandeeplocharla
committed
Bugfix/CSTACKEX-254: Change the minimum storagepool size to 20MB and throw an exception in case of invalid capacity bytes instead of adjusting to minimum requirement (#94)
This PR corrects the minimum allowed storage pool size from 1.56GB to 20MB as per ONTAP guidelines for FlexVol. Also, this removes the behaviour to silently correct the invalid capacity bytes to min. requirement, instead it'll throw an exception to let the user decide on the corrective action. <!--- ******************************************************************************* --> <!--- 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) - [ ] Major - [ ] Minor - [ ] BLOCKER - [ ] Critical - [X] Major - [ ] Minor - [ ] Trivial `Case-1: Storage Pool Capacity Bytes = 214B, less than the min. req of 20MB` <img width="1047" height="735" alt="Screenshot 2026-08-20 at 7 08 03 AM" src="https://github.com/user-attachments/assets/f09d23d8-4c74-4222-8b2e-e4b37b3de1b3" /> `Case-2: Storage Pool Capacity Bytes = 20971520B or 20MB` <img width="1047" height="735" alt="Screenshot 2026-08-20 at 7 15 58 AM" src="https://github.com/user-attachments/assets/8b4a47af-0d90-4636-bb48-5d41260a0fad" /> <img width="1047" height="735" alt="Screenshot 2026-08-20 at 7 16 27 AM" src="https://github.com/user-attachments/assets/f0033b69-0390-4fc6-8394-0e67247274be" /> <img width="1103" height="437" alt="Screenshot 2026-08-20 at 7 17 16 AM" src="https://github.com/user-attachments/assets/1bf51bd5-d746-43b6-b99e-fd87515db411" />
1 parent c22db54 commit 695dac6

3 files changed

Lines changed: 59 additions & 28 deletions

File tree

‎plugins/storage/volume/ontap/README.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ The NetApp ONTAP Storage Plugin provides integration between Apache CloudStack a
6565

6666
### Minimum Volume Size
6767

68-
ONTAP requires a minimum volume size of **1.56 GB** (1,677,721,600 bytes). The plugin will automatically adjust requested sizes below this threshold.
68+
ONTAP requires a minimum volume size of **20 MB** (20,971,520 bytes). Requests below this threshold are rejected.
6969

7070
## Configuration
7171

@@ -116,7 +116,7 @@ username=admin;password=secretpass;svmName=svm1;protocol=ISCSI;managementLIF=192
116116

117117
3. **Capacity Errors**
118118
- Check aggregate space availability
119-
- Ensure requested volume size meets minimum requirements (1.56 GB)
119+
- Ensure requested volume size meets minimum requirements (20 MB)
120120

121121
4. **Host Connection Issues**
122122
- For iSCSI: Verify host IQN is properly configured in host's storage URL

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public class OntapPrimaryDatastoreLifecycle extends BasePrimaryDataStoreLifeCycl
8383
@Inject private AlertManager _alertMgr;
8484
private static final Logger logger = LogManager.getLogger(OntapPrimaryDatastoreLifecycle.class);
8585

86-
private static final long ONTAP_MIN_VOLUME_SIZE_IN_BYTES = 1677721600L;
86+
private static final long ONTAP_MIN_VOLUME_SIZE_IN_BYTES = 20971520L;
8787

8888
/**
8989
* Creates primary storage on NetApp storage
@@ -113,6 +113,8 @@ public DataStore initialize(Map<String, Object> dsInfos) {
113113
@SuppressWarnings("unchecked")
114114
Map<String, String> details = (Map<String, String>) dsInfos.get("details");
115115

116+
validateInitializeInputs(capacityBytes, podId, clusterId, zoneId, storagePoolName, providerName, managed, details);
117+
116118
PrimaryDataStoreParameters parameters = new PrimaryDataStoreParameters();
117119
if (clusterId != null) {
118120
ClusterVO clusterVO = _clusterDao.findById(clusterId);
@@ -212,16 +214,16 @@ public DataStore initialize(Map<String, Object> dsInfos) {
212214
return _dataStoreHelper.createPrimaryDataStore(parameters);
213215
}
214216

215-
private long validateInitializeInputs(Long capacityBytes, Long podId, Long clusterId, Long zoneId,
217+
private void validateInitializeInputs(Long capacityBytes, Long podId, Long clusterId, Long zoneId,
216218
String storagePoolName, String providerName, boolean managed, Map<String, String> details) {
217219

218-
// Validate and set capacity
219220
if (capacityBytes == null || capacityBytes <= 0) {
220-
logger.warn("capacityBytes not provided or invalid (" + capacityBytes + "), using ONTAP minimum size: " + ONTAP_MIN_VOLUME_SIZE_IN_BYTES);
221-
capacityBytes = ONTAP_MIN_VOLUME_SIZE_IN_BYTES;
222-
} else if (capacityBytes < ONTAP_MIN_VOLUME_SIZE_IN_BYTES) {
223-
logger.warn("capacityBytes (" + capacityBytes + ") is below ONTAP minimum (" + ONTAP_MIN_VOLUME_SIZE_IN_BYTES + "), adjusting to minimum");
224-
capacityBytes = ONTAP_MIN_VOLUME_SIZE_IN_BYTES;
221+
throw new InvalidParameterValueException("Storage pool capacity is required for ONTAP primary storage and must be at least "
222+
+ ONTAP_MIN_VOLUME_SIZE_IN_BYTES + " bytes (20 MB)");
223+
}
224+
if (capacityBytes < ONTAP_MIN_VOLUME_SIZE_IN_BYTES) {
225+
throw new InvalidParameterValueException("Storage pool capacity " + capacityBytes + " bytes is below the ONTAP minimum volume size of "
226+
+ ONTAP_MIN_VOLUME_SIZE_IN_BYTES + " bytes (20 MB)");
225227
}
226228

227229
// Validate scope
@@ -278,8 +280,6 @@ private long validateInitializeInputs(Long capacityBytes, Long podId, Long clust
278280
missing.removeAll(providedKeys);
279281
throw new CloudRuntimeException("ONTAP primary storage creation failed, missing detail(s): " + missing);
280282
}
281-
282-
return capacityBytes;
283283
}
284284

285285
private void processDataLifSelection(Pair<String, String> lifResult, Map<String, String> details,

‎plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java‎

Lines changed: 47 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.mockito.quality.Strictness;
3333
import org.apache.cloudstack.storage.feign.model.Volume;
3434
import com.cloud.dc.dao.ClusterDao;
35+
import com.cloud.exception.InvalidParameterValueException;
3536
import com.cloud.utils.exception.CloudRuntimeException;
3637
import com.cloud.dc.ClusterVO;
3738
import com.cloud.host.HostVO;
@@ -178,7 +179,7 @@ public void testInitialize_positive() {
178179
dsInfos.put("clusterId", 1L);
179180
dsInfos.put("name", "testStoragePool");
180181
dsInfos.put("providerName", "testProvider");
181-
dsInfos.put("capacityBytes",200000L);
182+
dsInfos.put("capacityBytes", 1073741824L);
182183
dsInfos.put("managed",true);
183184
dsInfos.put("tags", "testTag");
184185
dsInfos.put("isTagARule", false);
@@ -212,7 +213,7 @@ public void testInitialize_missingRequiredDetailKey() {
212213
dsInfos.put("clusterId", 1L);
213214
dsInfos.put("name", "testStoragePool");
214215
dsInfos.put("providerName", "testProvider");
215-
dsInfos.put("capacityBytes",200000L);
216+
dsInfos.put("capacityBytes", 1073741824L);
216217
dsInfos.put("managed",true);
217218
dsInfos.put("tags", "testTag");
218219
dsInfos.put("isTagARule", false);
@@ -249,7 +250,37 @@ public void testInitialize_invalidCapacityBytes() {
249250

250251
try (MockedStatic<StorageProviderFactory> storageProviderFactory = Mockito.mockStatic(StorageProviderFactory.class)) {
251252
storageProviderFactory.when(() -> StorageProviderFactory.getStrategy(any())).thenReturn(storageStrategy);
252-
ontapPrimaryDatastoreLifecycle.initialize(dsInfos);
253+
Exception ex = assertThrows(InvalidParameterValueException.class, () -> ontapPrimaryDatastoreLifecycle.initialize(dsInfos));
254+
assertTrue(ex.getMessage().contains("must be at least"));
255+
}
256+
}
257+
258+
@Test
259+
public void testInitialize_capacityBelowOntapMinimum() {
260+
261+
HashMap<String, String> detailsMap = new HashMap<String, String>();
262+
detailsMap.put(OntapStorageConstants.USERNAME, "testUser");
263+
detailsMap.put(OntapStorageConstants.PASSWORD, "testPassword");
264+
detailsMap.put(OntapStorageConstants.STORAGE_IP, "10.10.10.10");
265+
detailsMap.put(OntapStorageConstants.SVM_NAME, "vs0");
266+
detailsMap.put(OntapStorageConstants.PROTOCOL, "NFS3");
267+
268+
Map<String, Object> dsInfos = new HashMap<>();
269+
dsInfos.put("zoneId",1L);
270+
dsInfos.put("podId",1L);
271+
dsInfos.put("clusterId", 1L);
272+
dsInfos.put("name", "testStoragePool");
273+
dsInfos.put("providerName", "testProvider");
274+
dsInfos.put("capacityBytes", 20971519L);
275+
dsInfos.put("managed",true);
276+
dsInfos.put("tags", "testTag");
277+
dsInfos.put("isTagARule", false);
278+
dsInfos.put("details", detailsMap);
279+
280+
try (MockedStatic<StorageProviderFactory> storageProviderFactory = Mockito.mockStatic(StorageProviderFactory.class)) {
281+
storageProviderFactory.when(() -> StorageProviderFactory.getStrategy(any())).thenReturn(storageStrategy);
282+
Exception ex = assertThrows(InvalidParameterValueException.class, () -> ontapPrimaryDatastoreLifecycle.initialize(dsInfos));
283+
assertTrue(ex.getMessage().contains("below the ONTAP minimum volume size"));
253284
}
254285
}
255286

@@ -261,7 +292,7 @@ public void testInitialize_unmanagedStorage() {
261292
dsInfos.put("clusterId", 1L);
262293
dsInfos.put("name", "testStoragePool");
263294
dsInfos.put("providerName", "testProvider");
264-
dsInfos.put("capacityBytes",200000L);
295+
dsInfos.put("capacityBytes", 1073741824L);
265296
dsInfos.put("managed",false);
266297
dsInfos.put("tags", "testTag");
267298
dsInfos.put("isTagARule", false);
@@ -284,7 +315,7 @@ public void testInitialize_nullStoragePoolName() {
284315
dsInfos.put("clusterId", 1L);
285316
dsInfos.put("name", null);
286317
dsInfos.put("providerName", "testProvider");
287-
dsInfos.put("capacityBytes",200000L);
318+
dsInfos.put("capacityBytes", 1073741824L);
288319
dsInfos.put("managed",true);
289320
dsInfos.put("tags", "testTag");
290321
dsInfos.put("isTagARule", false);
@@ -307,7 +338,7 @@ public void testInitialize_nullProviderName() {
307338
dsInfos.put("clusterId", 1L);
308339
dsInfos.put("name", "testStoragePool");
309340
dsInfos.put("providerName", null);
310-
dsInfos.put("capacityBytes",200000L);
341+
dsInfos.put("capacityBytes", 1073741824L);
311342
dsInfos.put("managed",true);
312343
dsInfos.put("tags", "testTag");
313344
dsInfos.put("isTagARule", false);
@@ -330,7 +361,7 @@ public void testInitialize_nullPodAndClusterAndZone() {
330361
dsInfos.put("clusterId", null);
331362
dsInfos.put("name", "testStoragePool");
332363
dsInfos.put("providerName", "testProvider");
333-
dsInfos.put("capacityBytes",200000L);
364+
dsInfos.put("capacityBytes", 1073741824L);
334365
dsInfos.put("managed",true);
335366
dsInfos.put("tags", "testTag");
336367
dsInfos.put("isTagARule", false);
@@ -364,7 +395,7 @@ public void testInitialize_clusterNotKVM() {
364395
dsInfos.put("clusterId", 2L);
365396
dsInfos.put("name", "testStoragePool");
366397
dsInfos.put("providerName", "testProvider");
367-
dsInfos.put("capacityBytes", 200000L);
398+
dsInfos.put("capacityBytes", 1073741824L);
368399
dsInfos.put("managed", true);
369400
dsInfos.put("tags", "testTag");
370401
dsInfos.put("isTagARule", false);
@@ -396,7 +427,7 @@ public void testInitialize_unexpectedDetailKey() {
396427
dsInfos.put("clusterId", 1L);
397428
dsInfos.put("name", "testStoragePool");
398429
dsInfos.put("providerName", "testProvider");
399-
dsInfos.put("capacityBytes",200000L);
430+
dsInfos.put("capacityBytes", 1073741824L);
400431
dsInfos.put("managed",true);
401432
dsInfos.put("tags", "testTag");
402433
dsInfos.put("isTagARule", false);
@@ -428,7 +459,7 @@ public void testInitialize_dataLifWithWarning() {
428459
dsInfos.put("clusterId", 1L);
429460
dsInfos.put("name", "testStoragePool");
430461
dsInfos.put("providerName", "testProvider");
431-
dsInfos.put("capacityBytes", 200000L);
462+
dsInfos.put("capacityBytes", 1073741824L);
432463
dsInfos.put("managed", true);
433464
dsInfos.put("tags", "testTag");
434465
dsInfos.put("isTagARule", false);
@@ -464,7 +495,7 @@ public void testInitialize_nullDataLif() {
464495
dsInfos.put("clusterId", 1L);
465496
dsInfos.put("name", "testStoragePool");
466497
dsInfos.put("providerName", "testProvider");
467-
dsInfos.put("capacityBytes", 200000L);
498+
dsInfos.put("capacityBytes", 1073741824L);
468499
dsInfos.put("managed", true);
469500
dsInfos.put("tags", "testTag");
470501
dsInfos.put("isTagARule", false);
@@ -495,7 +526,7 @@ public void testInitialize_emptyDataLif() {
495526
dsInfos.put("clusterId", 1L);
496527
dsInfos.put("name", "testStoragePool");
497528
dsInfos.put("providerName", "testProvider");
498-
dsInfos.put("capacityBytes", 200000L);
529+
dsInfos.put("capacityBytes", 1073741824L);
499530
dsInfos.put("managed", true);
500531
dsInfos.put("tags", "testTag");
501532
dsInfos.put("isTagARule", false);
@@ -526,7 +557,7 @@ public void testInitialize_getNetworkInterfaceException() {
526557
dsInfos.put("clusterId", 1L);
527558
dsInfos.put("name", "testStoragePool");
528559
dsInfos.put("providerName", "testProvider");
529-
dsInfos.put("capacityBytes", 200000L);
560+
dsInfos.put("capacityBytes", 1073741824L);
530561
dsInfos.put("managed", true);
531562
dsInfos.put("tags", "testTag");
532563
dsInfos.put("isTagARule", false);
@@ -558,7 +589,7 @@ public void testInitialize_volumeCreationFailure_nullVolume() {
558589
dsInfos.put("clusterId", 1L);
559590
dsInfos.put("name", "testStoragePool");
560591
dsInfos.put("providerName", "testProvider");
561-
dsInfos.put("capacityBytes", 200000L);
592+
dsInfos.put("capacityBytes", 1073741824L);
562593
dsInfos.put("managed", true);
563594
dsInfos.put("tags", "testTag");
564595
dsInfos.put("isTagARule", false);
@@ -589,7 +620,7 @@ public void testInitialize_volumeCreationException() {
589620
dsInfos.put("clusterId", 1L);
590621
dsInfos.put("name", "testStoragePool");
591622
dsInfos.put("providerName", "testProvider");
592-
dsInfos.put("capacityBytes", 200000L);
623+
dsInfos.put("capacityBytes", 1073741824L);
593624
dsInfos.put("managed", true);
594625
dsInfos.put("tags", "testTag");
595626
dsInfos.put("isTagARule", false);
@@ -621,7 +652,7 @@ public void testInitialize_positiveWithDetailAssertions() {
621652
dsInfos.put("clusterId", 1L);
622653
dsInfos.put("name", "testStoragePool");
623654
dsInfos.put("providerName", "testProvider");
624-
dsInfos.put("capacityBytes", 200000L);
655+
dsInfos.put("capacityBytes", 1073741824L);
625656
dsInfos.put("managed", true);
626657
dsInfos.put("tags", "testTag");
627658
dsInfos.put("isTagARule", false);

0 commit comments

Comments
 (0)