Skip to content

Commit b42f1ca

Browse files
suryag1201Gupta, Surya
andauthored
BugFixes For CSTACKEX-176, CSTACKEX-172, CSTACKEX-199 and CSTACKEX-181 (#65)
### Description This PR... CSTACKEX-176: Storage Pool Backend by ONTAP FlexVolume is not Thin Provisioned CSTACKEX-172: VM create failure due to aggrs not available and UI says "Unable to orchestrate the start of VM instance" CSTACKEX-199: Error message when cluster password is wrong while creating primary storage pool is not user intuitive CSTACKEX-181: Multiple host with same hostname will not follow per host igroup design ### 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 - [ ] Minor - [ ] Trivial ### Screenshots (if appropriate): <img width="1125" height="861" alt="Screenshot 2026-06-18 at 2 33 52 PM" src="https://github.com/user-attachments/assets/e2f5bc20-0109-4b5a-a720-dbd3d39ee254" /> <img width="679" height="280" alt="image" src="https://github.com/user-attachments/assets/b7dac4f1-1a6b-4234-bcd5-47c5efe8f798" /> ### How Has This Been Tested? 1- Create a storage pool with invalid ontap cred - throw the error 2- Create a storage pool with valid ontap cred - success 3- Verify Ontap Volume - thin provisioned 4- VM instance VM1 creation - suceess and igroup created with cs_svmName_hostUUID 5- VM instance VM2 creation - reused the same igroup 6- VM instance VM1 and VM2 delete - igroup also got deleted #### 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 5af4bf9 commit b42f1ca

10 files changed

Lines changed: 284 additions & 28 deletions

File tree

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,7 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore
413413
private void grantAccessIscsi(Host host, VolumeVO volumeVO, Map<String, String> details, String svmName, StoragePoolVO storagePool) {
414414
String cloudStackVolumeName = volumeDetailsDao.findDetail(volumeVO.getId(), OntapStorageConstants.LUN_DOT_NAME).getValue();
415415
UnifiedSANStrategy sanStrategy = (UnifiedSANStrategy) OntapStorageUtils.getStrategyByStoragePoolDetails(details);
416-
String accessGroupName = OntapStorageUtils.getIgroupName(svmName, host.getName());
416+
String accessGroupName = OntapStorageUtils.getIgroupName(svmName, host.getUuid());
417417

418418
// Validate if Igroup exist ONTAP for this host as we may be using delete_on_unmap= true and igroup may be deleted by ONTAP automatically
419419
Map<String, String> getAccessGroupMap = Map.of(
@@ -506,7 +506,7 @@ private void revokeAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO,
506506
String svmName = details.get(OntapStorageConstants.SVM_NAME);
507507

508508
if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL))) {
509-
String accessGroupName = OntapStorageUtils.getIgroupName(svmName, host.getName());
509+
String accessGroupName = OntapStorageUtils.getIgroupName(svmName, host.getUuid());
510510

511511
// Retrieve LUN name from volume details; if missing, volume may not have been fully created
512512
VolumeDetailVO lunDetail = volumeDetailsDao.findDetail(volumeVO.getId(), OntapStorageConstants.LUN_DOT_NAME);

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

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,11 @@
1919

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

22+
import com.fasterxml.jackson.annotation.JsonCreator;
2223
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
2324
import com.fasterxml.jackson.annotation.JsonInclude;
2425
import com.fasterxml.jackson.annotation.JsonProperty;
26+
import com.fasterxml.jackson.annotation.JsonValue;
2527

2628
import java.util.List;
2729
import java.util.Objects;
@@ -50,6 +52,9 @@ public class Volume {
5052
@JsonProperty("space")
5153
private VolumeSpace space;
5254

55+
@JsonProperty("guarantee")
56+
private Guarantee guarantee;
57+
5358
@JsonProperty("anti_ransomware")
5459
private AntiRansomware antiRansomware;
5560

@@ -112,6 +117,14 @@ public void setSpace(VolumeSpace space) {
112117
this.space = space;
113118
}
114119

120+
public Guarantee getGuarantee() {
121+
return guarantee;
122+
}
123+
124+
public void setGuarantee(Guarantee guarantee) {
125+
this.guarantee = guarantee;
126+
}
127+
115128
public AntiRansomware getAntiRansomware() {
116129
return antiRansomware;
117130
}
@@ -139,4 +152,66 @@ public boolean equals(Object o) {
139152
public int hashCode() {
140153
return Objects.hashCode(uuid);
141154
}
155+
156+
public static class Guarantee {
157+
158+
/**
159+
* ONTAP FlexVolume space guarantee (provisioning) type.
160+
* <ul>
161+
* <li>{@link #NONE} - thin provisioning (space is not reserved up front)</li>
162+
* <li>{@link #VOLUME} - thick provisioning (full volume size is reserved on the aggregate)</li>
163+
* </ul>
164+
*/
165+
public enum TypeEnum {
166+
NONE("none"),
167+
168+
VOLUME("volume");
169+
170+
private String value;
171+
172+
TypeEnum(String value) {
173+
this.value = value;
174+
}
175+
176+
@JsonValue
177+
public String getValue() {
178+
return value;
179+
}
180+
181+
@Override
182+
public String toString() {
183+
return String.valueOf(value);
184+
}
185+
186+
@JsonCreator
187+
public static TypeEnum fromValue(String text) {
188+
if (text == null) return null;
189+
for (TypeEnum b : TypeEnum.values()) {
190+
if (text.equalsIgnoreCase(b.value)) {
191+
return b;
192+
}
193+
}
194+
return null;
195+
}
196+
}
197+
198+
@JsonProperty("type")
199+
private TypeEnum type;
200+
201+
public Guarantee() {
202+
}
203+
204+
public Guarantee(TypeEnum type) {
205+
this.type = type;
206+
}
207+
208+
public TypeEnum getType() {
209+
return type;
210+
}
211+
212+
public void setType(TypeEnum type) {
213+
this.type = type;
214+
}
215+
}
216+
142217
}

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

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import org.apache.logging.log4j.Logger;
5050

5151
import java.util.HashMap;
52+
import java.util.ArrayList;
5253
import java.util.List;
5354
import java.util.Map;
5455
import java.util.Objects;
@@ -135,31 +136,42 @@ public boolean connect() {
135136
logger.error("No aggregates are assigned to SVM " + svmName);
136137
throw new CloudRuntimeException("No aggregates are assigned to SVM " + svmName);
137138
}
139+
// Collect all online aggregates assigned to the SVM. Capacity-based selection is
140+
// intentionally deferred to createStorageVolume(name, size), which validates the
141+
// available space against the actual requested volume size.
142+
List<Aggregate> eligibleAggregates = new ArrayList<>();
138143
for (Aggregate aggr : aggrs) {
139144
logger.debug("Found aggregate: " + aggr.getName() + " with UUID: " + aggr.getUuid());
140145
Aggregate aggrResp = aggregateFeignClient.getAggregateByUUID(authHeader, aggr.getUuid());
141146
if (aggrResp == null) {
142147
logger.warn("Aggregate details response is null for aggregate " + aggr.getName() + ". Skipping.");
143-
break;
148+
continue;
144149
}
145150
if (!Objects.equals(aggrResp.getState(), Aggregate.StateEnum.ONLINE)) {
146151
logger.warn("Aggregate " + aggr.getName() + " is not in online state. Skipping this aggregate.");
147152
continue;
148-
} else if (aggrResp.getSpace() == null || aggrResp.getAvailableBlockStorageSpace() == null ||
149-
aggrResp.getAvailableBlockStorageSpace() <= storage.getSize().doubleValue()) {
150-
logger.warn("Aggregate " + aggr.getName() + " does not have sufficient available space. Skipping this aggregate.");
151-
continue;
152153
}
153-
logger.info("Selected aggregate: " + aggr.getName() + " for volume operations.");
154-
this.aggregates = List.of(aggr);
155-
break;
154+
logger.debug("Aggregate " + aggr.getName() + " is online and eligible for volume operations.");
155+
eligibleAggregates.add(aggr);
156156
}
157-
if (this.aggregates == null || this.aggregates.isEmpty()) {
158-
logger.error("No suitable aggregates found on SVM " + svmName + " for volume creation.");
159-
throw new CloudRuntimeException("No suitable aggregates found on SVM " + svmName + " for volume creation.");
157+
if (eligibleAggregates.isEmpty()) {
158+
logger.error("No suitable aggregates found on SVM " + svmName + " for volume operations.");
159+
throw new CloudRuntimeException("No suitable aggregates found on SVM " + svmName + " for volume operations.");
160160
}
161+
this.aggregates = eligibleAggregates;
162+
logger.info("Found " + eligibleAggregates.size() + " online aggregate(s) on SVM " + svmName + " for volume operations.");
161163

162164
logger.info("Successfully connected to ONTAP cluster and validated ONTAP details provided");
165+
} catch (FeignException.Unauthorized e) {
166+
logger.error("Authentication failed while connecting to ONTAP cluster at " + storage.getStorageIP() +
167+
". Please verify the username and password.", e);
168+
throw new CloudRuntimeException("Authentication failed: Invalid credentials for ONTAP cluster at " +
169+
storage.getStorageIP() + ". Please verify the username and password.");
170+
} catch (FeignException.Forbidden e) {
171+
logger.error("Authorization failed while connecting to ONTAP cluster at " + storage.getStorageIP() +
172+
". The user does not have sufficient privileges.", e);
173+
throw new CloudRuntimeException("Authorization failed: User does not have sufficient privileges on ONTAP cluster at " +
174+
storage.getStorageIP() + ". Please verify user permissions.");
163175
} catch (Exception e) {
164176
logger.error("Failed to connect to ONTAP cluster: " + e.getMessage(), e);
165177
throw new CloudRuntimeException("Failed to connect to ONTAP cluster: " + e.getMessage(), e);
@@ -211,7 +223,7 @@ public Volume createStorageVolume(String volumeName, Long size) {
211223

212224
if (aggrResp == null) {
213225
logger.warn("Aggregate details response is null for aggregate " + aggr.getName() + ". Skipping.");
214-
break;
226+
continue;
215227
}
216228

217229
if (!Objects.equals(aggrResp.getState(), Aggregate.StateEnum.ONLINE)) {
@@ -251,6 +263,7 @@ public Volume createStorageVolume(String volumeName, Long size) {
251263
volumeRequest.setAggregates(List.of(aggr));
252264
volumeRequest.setSize(size);
253265
volumeRequest.setNas(nas);
266+
volumeRequest.setGuarantee(new Volume.Guarantee(Volume.Guarantee.TypeEnum.NONE));
254267
try {
255268
JobResponse jobResponse = volumeFeignClient.createVolumeWithJob(authHeader, volumeRequest);
256269
if (jobResponse == null || jobResponse.getJob() == null) {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ public AccessGroup createAccessGroup(AccessGroup accessGroup) {
207207
igroupRequest.setOsType(Igroup.OsTypeEnum.Linux);
208208

209209
for (HostVO host : accessGroup.getHostsToConnect()) {
210-
igroupName = OntapStorageUtils.getIgroupName(svmName, host.getName());
210+
igroupName = OntapStorageUtils.getIgroupName(svmName, host.getUuid());
211211
igroupRequest.setName(igroupName);
212212

213213
List<Initiator> initiators = new ArrayList<>();
@@ -271,7 +271,7 @@ public void deleteAccessGroup(AccessGroup accessGroup) {
271271
//Get iGroup name per host
272272
if(!CollectionUtils.isEmpty(accessGroup.getHostsToConnect())) {
273273
for (HostVO host : accessGroup.getHostsToConnect()) {
274-
String igroupName = OntapStorageUtils.getIgroupName(svmName, host.getName());
274+
String igroupName = OntapStorageUtils.getIgroupName(svmName, host.getUuid());
275275
logger.info("deleteAccessGroup: iGroup name '{}'", igroupName);
276276

277277
// Get the iGroup to retrieve its UUID

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ public class OntapStorageConstants {
9696
public static final String IGROUP_DOT_UUID = "igroup.uuid";
9797
public static final String UNDERSCORE = "_";
9898
public static final String CS = "cs";
99+
public static final int IGROUP_NAME_MAX_LENGTH = 96;
99100
public static final String SRC_CS_VOLUME_ID = "src_cs_volume_id";
100101
public static final String BASE_ONTAP_FV_ID = "base_ontap_fv_id";
101102
public static final String ONTAP_SNAP_ID = "ontap_snap_id";

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,10 +139,15 @@ public static StorageStrategy getStrategyByStoragePoolDetails(Map<String, String
139139
}
140140
}
141141

142-
public static String getIgroupName(String svmName, String hostName) {
143-
//Igroup name format: cs_svmName_hostName
144-
String sanitizedHostName = hostName.split("\\.")[0].replaceAll("[^a-zA-Z0-9_-]", "_");
145-
return OntapStorageConstants.CS + OntapStorageConstants.UNDERSCORE + svmName + OntapStorageConstants.UNDERSCORE + sanitizedHostName;
142+
public static String getIgroupName(String svmName, String hostUuid) {
143+
//Igroup name format: cs_svmName_hostUuid
144+
String sanitizedHostUuid = hostUuid.replaceAll("[^a-zA-Z0-9_-]", "_");
145+
String igroupName = OntapStorageConstants.CS + OntapStorageConstants.UNDERSCORE + svmName + OntapStorageConstants.UNDERSCORE + sanitizedHostUuid;
146+
// ONTAP igroup names are limited to 96 characters; truncate if longer.
147+
if (igroupName.length() > OntapStorageConstants.IGROUP_NAME_MAX_LENGTH) {
148+
igroupName = igroupName.substring(0, OntapStorageConstants.IGROUP_NAME_MAX_LENGTH);
149+
}
150+
return igroupName;
146151
}
147152

148153
public static String generateExportPolicyName(String svmName, String volumeName){

plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,7 @@ void testGrantAccess_ClusterScope_Success() {
348348
when(volumeVO.getId()).thenReturn(100L);
349349

350350
when(host.getName()).thenReturn("host1");
351+
when(host.getUuid()).thenReturn("host-uuid-1");
351352

352353
VolumeDetailVO lunNameDetail = new VolumeDetailVO(100L, OntapStorageConstants.LUN_DOT_NAME, "/vol/vol1/lun1", false);
353354
when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.LUN_DOT_NAME)).thenReturn(lunNameDetail);
@@ -384,6 +385,7 @@ void testGrantAccess_IgroupNotFound_CreatesNewIgroup() {
384385
// Setup - use HostVO mock since production code casts Host to HostVO
385386
HostVO hostVO = mock(HostVO.class);
386387
when(hostVO.getName()).thenReturn("host1");
388+
when(hostVO.getUuid()).thenReturn("host-uuid-1");
387389

388390
when(dataStore.getId()).thenReturn(1L);
389391
when(volumeInfo.getType()).thenReturn(VOLUME);
@@ -477,6 +479,7 @@ void testRevokeAccess_ISCSIVolume_Success() {
477479

478480
when(host.getStorageUrl()).thenReturn("iqn.1993-08.org.debian:01:host1");
479481
when(host.getName()).thenReturn("host1");
482+
when(host.getUuid()).thenReturn("host-uuid-1");
480483

481484
VolumeDetailVO lunNameDetail = new VolumeDetailVO(100L, OntapStorageConstants.LUN_DOT_NAME, "/vol/vol1/lun1", false);
482485
when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.LUN_DOT_NAME)).thenReturn(lunNameDetail);

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

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,69 @@ public void testConnect_positive() {
230230
verify(svmFeignClient, times(1)).getSvmResponse(anyMap(), anyString());
231231
}
232232

233+
@Test
234+
public void testConnect_succeedsWhenAggregateSpaceBelowPoolCapacity() {
235+
// Regression: connect() must validate connectivity/SVM/aggregate-state ONLY.
236+
// Capacity is validated per-volume in createStorageVolume(name, size). Previously
237+
// connect() compared aggregate free space against the whole storage pool size
238+
// (storage.getSize()), which incorrectly failed data-path operations (volume/LUN
239+
// create, grant/revoke access, delete) once the pool FlexVolume already existed.
240+
Svm svm = new Svm();
241+
svm.setName("svm1");
242+
svm.setState(OntapStorageConstants.RUNNING);
243+
svm.setNfsEnabled(true);
244+
245+
Aggregate aggregate = new Aggregate();
246+
aggregate.setName("aggr1");
247+
aggregate.setUuid("aggr-uuid-1");
248+
svm.setAggregates(List.of(aggregate));
249+
250+
OntapResponse<Svm> svmResponse = new OntapResponse<>();
251+
svmResponse.setRecords(List.of(svm));
252+
253+
when(svmFeignClient.getSvmResponse(anyMap(), anyString())).thenReturn(svmResponse);
254+
255+
// Aggregate is ONLINE but has far less free space than the configured pool size (5GB).
256+
Aggregate aggregateDetail = mock(Aggregate.class);
257+
when(aggregateDetail.getName()).thenReturn("aggr1");
258+
when(aggregateDetail.getUuid()).thenReturn("aggr-uuid-1");
259+
when(aggregateDetail.getState()).thenReturn(Aggregate.StateEnum.ONLINE);
260+
when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1"))).thenReturn(aggregateDetail);
261+
262+
// Execute & Verify - connect() should succeed regardless of available space.
263+
boolean result = storageStrategy.connect();
264+
assertTrue(result, "connect() should succeed for an online aggregate even when its free space is below the pool capacity");
265+
}
266+
267+
@Test
268+
public void testConnect_noOnlineAggregates() {
269+
// Setup - aggregate assigned to the SVM exists but is not ONLINE
270+
Svm svm = new Svm();
271+
svm.setName("svm1");
272+
svm.setState(OntapStorageConstants.RUNNING);
273+
svm.setNfsEnabled(true);
274+
275+
Aggregate aggregate = new Aggregate();
276+
aggregate.setName("aggr1");
277+
aggregate.setUuid("aggr-uuid-1");
278+
svm.setAggregates(List.of(aggregate));
279+
280+
OntapResponse<Svm> svmResponse = new OntapResponse<>();
281+
svmResponse.setRecords(List.of(svm));
282+
283+
when(svmFeignClient.getSvmResponse(anyMap(), anyString())).thenReturn(svmResponse);
284+
285+
Aggregate aggregateDetail = mock(Aggregate.class);
286+
when(aggregateDetail.getName()).thenReturn("aggr1");
287+
when(aggregateDetail.getUuid()).thenReturn("aggr-uuid-1");
288+
when(aggregateDetail.getState()).thenReturn(null); // not online
289+
when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1"))).thenReturn(aggregateDetail);
290+
291+
// Execute & Verify
292+
CloudRuntimeException ex = assertThrows(CloudRuntimeException.class, () -> storageStrategy.connect());
293+
assertTrue(ex.getMessage().contains("No suitable aggregates found"));
294+
}
295+
233296
@Test
234297
public void testConnect_svmNotFound() {
235298
// Setup
@@ -342,6 +405,20 @@ public void testConnect_nullSvmResponse() {
342405
assertTrue(ex.getMessage().contains("No SVM found"));
343406
}
344407

408+
@Test
409+
public void testConnect_invalidCredentials() {
410+
// Setup - ONTAP rejects the supplied username/password with HTTP 401 Unauthorized.
411+
when(svmFeignClient.getSvmResponse(anyMap(), anyString()))
412+
.thenThrow(mock(FeignException.Unauthorized.class));
413+
414+
// Execute & Verify - connect() must surface a clear "invalid credentials" error.
415+
CloudRuntimeException ex = assertThrows(CloudRuntimeException.class, () -> storageStrategy.connect());
416+
assertTrue(ex.getMessage().contains("Authentication failed: Invalid credentials"),
417+
"Expected an authentication failure message but got: " + ex.getMessage());
418+
assertTrue(ex.getMessage().contains("Please verify the username and password"),
419+
"Expected the message to prompt verifying username/password but got: " + ex.getMessage());
420+
}
421+
345422
// ========== createStorageVolume() Tests ==========
346423

347424
@Test

0 commit comments

Comments
 (0)