Skip to content

Commit 028e1ef

Browse files
piyush5netapppiyush5netapp
authored
Netapp ONTAP bugfixes for 4.23.0 (#13578)
* bugfix: Fix volume attach in iSCSI for ONTAP plugin (#54) * Bugfix: Zone scoped Instance creation fix
1 parent a6b3128 commit 028e1ef

15 files changed

Lines changed: 378 additions & 67 deletions

File tree

‎engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
5050
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreCapabilities;
5151
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreDriver;
52+
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProvider;
5253
import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
5354
import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector;
5455
import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine;
@@ -1368,11 +1369,13 @@ private void createManagedVolumeCopyTemplateAsync(VolumeInfo volumeInfo, Primary
13681369
primaryDataStore.setDetails(details);
13691370

13701371
grantAccess(volumeInfo, destHost, primaryDataStore);
1371-
volumeInfo = volFactory.getVolume(volumeInfo.getId(), primaryDataStore);
1372-
// For Netapp ONTAP iscsiName or Lun path is available only after grantAccess
1373-
String managedStoreTarget = ObjectUtils.defaultIfNull(volumeInfo.get_iScsiName(), volumeInfo.getUuid());
1374-
details.put(PrimaryDataStore.MANAGED_STORE_TARGET, managedStoreTarget);
1375-
primaryDataStore.setDetails(details);
1372+
if (DataStoreProvider.ONTAP_PLUGIN_NAME.equals(primaryDataStore.getStorageProviderName())) {
1373+
// For Netapp ONTAP iscsiName or Lun path is available only after grantAccess
1374+
volumeInfo = volFactory.getVolume(volumeInfo.getId(), primaryDataStore);
1375+
String managedStoreTarget = ObjectUtils.defaultIfNull(volumeInfo.get_iScsiName(), volumeInfo.getUuid());
1376+
details.put(PrimaryDataStore.MANAGED_STORE_TARGET, managedStoreTarget);
1377+
primaryDataStore.setDetails(details);
1378+
}
13761379

13771380
try {
13781381
motionSrv.copyAsync(srcTemplateInfo, destTemplateInfo, destHost, caller);

‎plugins/storage/volume/ontap/pom.xml‎

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,7 @@
3737
<jackson-databind.version>2.13.4</jackson-databind.version>
3838
<assertj.version>3.24.2</assertj.version>
3939
<junit-jupiter.version>5.8.1</junit-jupiter.version>
40-
<mockito.version>3.12.4</mockito.version>
41-
<mockito-junit-jupiter.version>5.2.0</mockito-junit-jupiter.version>
42-
<byte-buddy-agent.version>1.11.13</byte-buddy-agent.version>
40+
<byte-buddy-agent.version>1.15.11</byte-buddy-agent.version>
4341
</properties>
4442
<dependencyManagement>
4543
<dependencies>
@@ -105,21 +103,13 @@
105103
<dependency>
106104
<groupId>org.mockito</groupId>
107105
<artifactId>mockito-core</artifactId>
108-
<version>${mockito.version}</version>
106+
<version>${cs.mockito.version}</version>
109107
<scope>test</scope>
110108
</dependency>
111109
<dependency>
112110
<groupId>org.mockito</groupId>
113111
<artifactId>mockito-junit-jupiter</artifactId>
114-
<version>${mockito-junit-jupiter.version}</version>
115-
<scope>test</scope>
116-
</dependency>
117-
118-
<!-- Mockito Inline (for static method mocking) -->
119-
<dependency>
120-
<groupId>org.mockito</groupId>
121-
<artifactId>mockito-inline</artifactId>
122-
<version>${mockito.version}</version>
112+
<version>${cs.mockito.version}</version>
123113
<scope>test</scope>
124114
</dependency>
125115
<dependency>
@@ -163,7 +153,6 @@
163153
<artifactId>maven-surefire-plugin</artifactId>
164154
<version>${maven-surefire-plugin.version}</version>
165155
<configuration>
166-
<skipTests>false</skipTests>
167156
<includes>
168157
<include>**/*Test.java</include>
169158
</includes>

‎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/lifecycle/OntapPrimaryDatastoreLifecycle.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -337,9 +337,12 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper
337337
logger.error("attachZone : Storage Pool not found for id: " + dataStore.getId());
338338
throw new CloudRuntimeException("Storage Pool not found for id: " + dataStore.getId());
339339
}
340-
340+
if (!Hypervisor.HypervisorType.KVM.equals(hypervisorType)) {
341+
logger.error("attachZone : ONTAP primary storage is supported only for KVM hypervisor");
342+
throw new CloudRuntimeException("ONTAP primary storage is supported only for KVM hypervisor");
343+
}
341344
PrimaryDataStoreInfo primaryStore = (PrimaryDataStoreInfo)dataStore;
342-
List<HostVO> hostsToConnect = _resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(dataStore, scope.getScopeId(), Hypervisor.HypervisorType.KVM);
345+
List<HostVO> hostsToConnect = _resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(dataStore, scope.getScopeId(), hypervisorType);
343346
logger.debug(String.format("In createPool. Attaching the pool to each of the hosts in %s.", hostsToConnect));
344347

345348
Map<String, String> details = storagePoolDetailsDao.listDetailsKeyPairs(primaryStore.getId());
@@ -364,7 +367,7 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper
364367
return false;
365368
}
366369
}
367-
_dataStoreHelper.attachZone(dataStore);
370+
_dataStoreHelper.attachZone(dataStore, hypervisorType);
368371
return true;
369372
}
370373

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

Lines changed: 26 additions & 13 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)) {
@@ -227,7 +239,7 @@ public Volume createStorageVolume(String volumeName, Long size) {
227239
final long availableBytes = aggrResp.getAvailableBlockStorageSpace().longValue();
228240
logger.debug("Aggregate " + aggr.getName() + " available bytes=" + availableBytes + ", requested=" + size);
229241

230-
if (availableBytes <= size) {
242+
if (availableBytes < size) {
231243
logger.warn("Aggregate " + aggr.getName() + " does not have sufficient available space. Required=" +
232244
size + " bytes, available=" + availableBytes + " bytes. Skipping this aggregate.");
233245
continue;
@@ -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_hostUuid_svmName
144+
String sanitizedHostUuid = hostUuid.replaceAll("[^a-zA-Z0-9_-]", "_");
145+
String igroupName = OntapStorageConstants.CS + OntapStorageConstants.UNDERSCORE + sanitizedHostUuid + OntapStorageConstants.UNDERSCORE + svmName;
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);

0 commit comments

Comments
 (0)