Skip to content

Commit ef67042

Browse files
piyush5netappSrivastava, Piyush
andauthored
Feature/cstackex 251: New pool type for Netapp iscsi usecase (#95)
### Description This PR... <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. --> <!-- 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) - [x] New feature (non-breaking change which adds functionality) - [ ] 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 - [x] Major - [ ] Minor #### Bug Severity - [ ] BLOCKER - [ ] Critical - [ ] Major - [ ] Minor - [ ] Trivial ### Screenshots (if appropriate): <img width="1244" height="380" alt="image" src="https://github.com/user-attachments/assets/778546db-5cb2-4fcd-a8c5-c3cc5150d096" /> <img width="1231" height="85" alt="image" src="https://github.com/user-attachments/assets/bef9134e-742e-4415-ad29-e32b8a6ddb61" /> <img width="1244" height="276" alt="image" src="https://github.com/user-attachments/assets/d15cab52-f1e8-40cb-aff6-ffceb8837781" /> ### How Has This Been Tested? Automation has been run ================================================================ TEST SUMMARY ================================================================ [iscsi_workflow] iSCSI pool lifecycle PASS test_01_create_primary_storage_pool PASS test_02_disable_storage_pool PASS test_03_enable_storage_pool PASS test_04_enter_maintenance_mode PASS test_05_cancel_maintenance_mode PASS test_06_enter_maintenance_and_delete_pool PASS test_07_create_volume_on_pool PASS test_08_delete_volume_and_pool [iscsi_with_volumes] iSCSI pool with volumes PASS test_01_create_pool_and_volume PASS test_02_disable_pool_volume_survives PASS test_03_enable_pool_volume_intact PASS test_04_enter_maintenance_volume_present PASS test_05_cancel_maintenance_volume_present PASS test_06_forced_false_delete_rejected PASS test_07_delete_volume_and_force_delete_pool [iscsi_volume] iSCSI volume lifecycle PASS test_01_create_pool_and_volume PASS test_02_delete_volume PASS test_03_recreate_volume_for_delete_tests PASS test_04_forced_false_delete_with_volume_fails PASS test_05_delete_volume_and_force_delete_pool [iscsi_zone_pool] iSCSI zone-scoped pool PASS test_01_create_zone_scoped_pool PASS test_02_disable_zone_scoped_pool PASS test_03_enable_zone_scoped_pool PASS test_04_delete_zone_scoped_pool [iscsi_vm_workflow] iSCSI VM volume workflow PASS test_01_create_iscsi_pool PASS test_02_create_ontap_data_volume PASS test_03_deploy_vm PASS test_04_attach_volume_to_vm PASS test_05_stop_vm_lun_unmapped PASS test_06_start_vm_lun_remapped PASS test_07_detach_volume_from_vm PASS test_08_destroy_vm_and_cleanup ================================================================ TOTAL: 32 passed, 0 failed, 0 skipped (32 tests) ================================================================ <!-- 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: Srivastava, Piyush <Piyush.Srivastava@netapp.com>
1 parent d7f9576 commit ef67042

11 files changed

Lines changed: 215 additions & 36 deletions

File tree

‎api/src/main/java/com/cloud/storage/Storage.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,8 @@ public static enum StoragePoolType {
185185
Linstor(true, true, EncryptionSupport.Storage),
186186
DatastoreCluster(true, true, EncryptionSupport.Unsupported), // for VMware, to abstract pool of clusters
187187
StorPool(true, true, EncryptionSupport.Hypervisor),
188-
FiberChannel(true, true, EncryptionSupport.Unsupported); // Fiber Channel Pool for KVM hypervisors is used to find the volume by WWN value (/dev/disk/by-id/wwn-<wwnvalue>)
188+
FiberChannel(true, true, EncryptionSupport.Unsupported), // Fiber Channel Pool for KVM hypervisors is used to find the volume by WWN value (/dev/disk/by-id/wwn-<wwnvalue>)
189+
OntapiSCSI(true, true, EncryptionSupport.Unsupported); // NetApp ONTAP iSCSI
189190

190191
private final boolean shared;
191192
private final boolean overProvisioning;

‎api/src/test/java/com/cloud/storage/StorageTest.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ public void isSharedStoragePool() {
4949
Assert.assertTrue(StoragePoolType.ManagedNFS.isShared());
5050
Assert.assertTrue(StoragePoolType.DatastoreCluster.isShared());
5151
Assert.assertTrue(StoragePoolType.Linstor.isShared());
52+
Assert.assertTrue(StoragePoolType.OntapiSCSI.isShared());
5253
}
5354

5455
@Test
@@ -73,6 +74,7 @@ public void supportsOverProvisioningTestAllStoragePoolTypes() {
7374
Assert.assertFalse(StoragePoolType.ManagedNFS.supportsOverProvisioning());
7475
Assert.assertTrue(StoragePoolType.DatastoreCluster.supportsOverProvisioning());
7576
Assert.assertTrue(StoragePoolType.Linstor.supportsOverProvisioning());
77+
Assert.assertTrue(StoragePoolType.OntapiSCSI.supportsOverProvisioning());
7678
}
7779

7880
@Test

‎engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -616,8 +616,8 @@ private void handleVolumeMigrationFromManagedStorageToNonManagedStorage(VolumeIn
616616
private void verifyFormatWithPoolType(ImageFormat imageFormat, StoragePoolType poolType) {
617617
if (imageFormat != ImageFormat.VHD && imageFormat != ImageFormat.OVA && imageFormat != ImageFormat.QCOW2 &&
618618
!(imageFormat == ImageFormat.RAW && (StoragePoolType.PowerFlex == poolType ||
619-
StoragePoolType.FiberChannel == poolType))) {
620-
throw new CloudRuntimeException(String.format("Only the following image types are currently supported: %s, %s, %s, %s (for PowerFlex and FiberChannel)",
619+
StoragePoolType.FiberChannel == poolType || StoragePoolType.OntapiSCSI == poolType))) {
620+
throw new CloudRuntimeException(String.format("Only the following image types are currently supported: %s, %s, %s, %s (for PowerFlex, FiberChannel and OntapiSCSI)",
621621
ImageFormat.VHD.toString(), ImageFormat.OVA.toString(), ImageFormat.QCOW2.toString(), ImageFormat.RAW.toString()));
622622
}
623623
}

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,8 @@ public Answer copyTemplateToPrimaryStorage(final CopyCommand cmd) {
415415
StoragePoolType.PowerFlex,
416416
StoragePoolType.Linstor,
417417
StoragePoolType.FiberChannel,
418-
StoragePoolType.CLVM).contains(primaryPool.getType())) {
418+
StoragePoolType.CLVM,
419+
StoragePoolType.OntapiSCSI).contains(primaryPool.getType())) {
419420
newTemplate.setFormat(ImageFormat.RAW);
420421
} else {
421422
newTemplate.setFormat(ImageFormat.QCOW2);
@@ -3431,7 +3432,8 @@ private Storage.ImageFormat getFormat(StoragePoolType poolType) {
34313432
StoragePoolType.PowerFlex,
34323433
StoragePoolType.Linstor,
34333434
StoragePoolType.FiberChannel,
3434-
StoragePoolType.CLVM).contains(poolType)) {
3435+
StoragePoolType.CLVM,
3436+
StoragePoolType.OntapiSCSI).contains(poolType)) {
34353437
return ImageFormat.RAW;
34363438
} else {
34373439
return ImageFormat.QCOW2;
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
package com.cloud.hypervisor.kvm.storage;
18+
19+
import com.cloud.storage.Storage.StoragePoolType;
20+
21+
/**
22+
* Serves {@link StoragePoolType#OntapiSCSI} pools, which are ONTAP FlexVols exposed over iSCSI with one
23+
* LUN per CloudStack volume. The host-side handling is identical to a generic iSCSI target
24+
*
25+
* The class exists so that ONTAP-specific host behaviour can diverge here without altering the storage
26+
* path of the other vendors that register as {@link StoragePoolType#Iscsi} which all share the superclass.
27+
*
28+
* This must stay in the {@code com.cloud.hypervisor.kvm.storage} package: {@link KVMStoragePoolManager}
29+
* discovers adaptors by a Reflections scan of that package alone, and an unregistered type silently
30+
* falls back to {@link LibvirtStorageAdaptor} rather than failing at startup.
31+
*/
32+
public class OntapIscsiStorageAdaptor extends IscsiAdmStorageAdaptor {
33+
34+
@Override
35+
public StoragePoolType getStoragePoolType() {
36+
return StoragePoolType.OntapiSCSI;
37+
}
38+
}
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
package com.cloud.hypervisor.kvm.storage;
18+
19+
import static org.junit.Assert.assertEquals;
20+
import static org.junit.Assert.assertFalse;
21+
import static org.junit.Assert.assertSame;
22+
import static org.junit.Assert.assertTrue;
23+
24+
import java.lang.reflect.Modifier;
25+
import java.util.Set;
26+
27+
import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat;
28+
import org.junit.Test;
29+
import org.reflections.Reflections;
30+
31+
import com.cloud.storage.Storage.StoragePoolType;
32+
33+
public class OntapIscsiStorageAdaptorTest {
34+
35+
@Test
36+
public void getStoragePoolTypeReturnsOntapIscsi() {
37+
assertEquals(StoragePoolType.OntapiSCSI, new OntapIscsiStorageAdaptor().getStoragePoolType());
38+
}
39+
40+
@Test
41+
public void createdPoolCarriesOntapIscsiTypeAndRawFormat() {
42+
OntapIscsiStorageAdaptor adaptor = new OntapIscsiStorageAdaptor();
43+
44+
KVMStoragePool pool = adaptor.createStoragePool("ontap-iscsi-pool-uuid", "10.0.0.1", 3260, null, null,
45+
StoragePoolType.OntapiSCSI, null, true);
46+
47+
assertEquals(StoragePoolType.OntapiSCSI, pool.getType());
48+
// Attach builds a block-based disk off the physical disk format rather than the pool type,
49+
// which is why splitting OntapiSCSI out of Iscsi leaves the generated domain XML unchanged.
50+
assertEquals(PhysicalDiskFormat.RAW, pool.getDefaultFormat());
51+
assertSame(pool, adaptor.getStoragePool("ontap-iscsi-pool-uuid"));
52+
}
53+
54+
/**
55+
* KVMStoragePoolManager discovers adaptors by a Reflections scan of its own package, instantiating
56+
* each concrete implementation through a no-arg constructor and keying it on getStoragePoolType().
57+
* A type with no adaptor silently falls back to LibvirtStorageAdaptor instead of failing at
58+
* startup, so this reproduces the discovery preconditions rather than waiting for the symptom.
59+
* The manager itself is not constructed here because doing so also instantiates
60+
* MultipathSCSIAdapterBase, which requires agent scripts resolvable from the working directory.
61+
*/
62+
@Test
63+
public void adaptorSatisfiesThePoolManagerDiscoveryContract() throws ReflectiveOperationException {
64+
String scannedPackage = KVMStoragePoolManager.class.getPackage().getName();
65+
Set<Class<? extends StorageAdaptor>> discovered =
66+
new Reflections(scannedPackage).getSubTypesOf(StorageAdaptor.class);
67+
68+
assertTrue("OntapIscsiStorageAdaptor must live in " + scannedPackage + " to be discovered",
69+
discovered.contains(OntapIscsiStorageAdaptor.class));
70+
assertFalse("An abstract adaptor is skipped by the scan",
71+
Modifier.isAbstract(OntapIscsiStorageAdaptor.class.getModifiers()));
72+
73+
StorageAdaptor adaptor = OntapIscsiStorageAdaptor.class.getDeclaredConstructor().newInstance();
74+
assertEquals(StoragePoolType.OntapiSCSI, adaptor.getStoragePoolType());
75+
assertEquals("The superclass must keep serving the other iSCSI vendors",
76+
StoragePoolType.Iscsi, new IscsiAdmStorageAdaptor().getStoragePoolType());
77+
}
78+
}

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

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@
7676

7777
import javax.inject.Inject;
7878
import java.util.ArrayList;
79+
import java.util.Arrays;
7980
import java.util.HashMap;
8081
import java.util.List;
8182
import java.util.Map;
@@ -148,6 +149,7 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet
148149
}
149150

150151
Map<String, String> details = storagePoolDetailsDao.listDetailsKeyPairs(dataStore.getId());
152+
validateProtocol(details, dataStore);
151153

152154
if (dataObject.getType() == DataObjectType.VOLUME) {
153155
VolumeInfo volInfo = (VolumeInfo) dataObject;
@@ -160,8 +162,8 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet
160162

161163
volumeVO.setPoolType(storagePool.getPoolType());
162164
volumeVO.setPoolId(storagePool.getId());
163-
volumeVO.setFormat(getImageFormatByHypervisorAndProtocol(storagePool.getHypervisor(), details.get(OntapStorageConstants.PROTOCOL)));
164-
logger.info("createAsync: Volume format set to [{}] for hypervisor [{}] and protocol [{}]", volumeVO.getFormat(), storagePool.getHypervisor(), details.get(OntapStorageConstants.PROTOCOL));
165+
volumeVO.setFormat(getImageFormat(storagePool));
166+
logger.info("createAsync: Volume format set to [{}] for pool type [{}]", volumeVO.getFormat(), storagePool.getPoolType());
165167

166168
if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL))) {
167169
String lunName = created != null && created.getLun() != null ? created.getLun().getName() : null;
@@ -987,21 +989,36 @@ private String buildSnapshotName(String cloudStackSnapshotName, long snapshotId)
987989
return OntapStorageUtils.buildOntapSnapshotName(cloudStackSnapshotName, OntapStorageConstants.CS + snapshotId);
988990
}
989991

992+
/**
993+
* Only ISCSI and NFS3 pools can be provisioned; any other protocol would fall through
994+
* createAsync without producing a result, leaving the caller with a null callback value.
995+
*/
996+
private void validateProtocol(Map<String, String> details, DataStore dataStore) {
997+
String protocol = details == null ? null : details.get(OntapStorageConstants.PROTOCOL);
998+
boolean supported = protocol != null && Arrays.stream(ProtocolType.values())
999+
.anyMatch(type -> type.name().equalsIgnoreCase(protocol));
1000+
if (!supported) {
1001+
throw new CloudRuntimeException("Unsupported protocol [" + protocol + "] on storage pool ["
1002+
+ dataStore.getName() + "]; supported protocols are " + Arrays.toString(ProtocolType.values()));
1003+
}
1004+
}
9901005

991-
private Storage.ImageFormat getImageFormatByHypervisorAndProtocol(HypervisorType hypervisorType, String protocol) {
992-
if (HypervisorType.KVM.equals(hypervisorType)) {
993-
ProtocolType protocolType = ProtocolType.valueOf(protocol);
994-
switch (protocolType) {
995-
case NFS3:
996-
return Storage.ImageFormat.QCOW2;
997-
case ISCSI:
998-
return Storage.ImageFormat.RAW;
999-
default:
1000-
throw new CloudRuntimeException("Unsupported protocol [" + protocol + "] for ONTAP image format resolution");
1001-
}
1006+
private Storage.ImageFormat getImageFormat(StoragePoolVO storagePool) {
1007+
HypervisorType hypervisorType = storagePool.getHypervisor();
1008+
if (!HypervisorType.KVM.equals(hypervisorType)) {
1009+
throw new CloudRuntimeException("Unsupported hypervisor [" + hypervisorType + "] for ONTAP image format resolution");
1010+
}
1011+
Storage.StoragePoolType spType = storagePool.getPoolType();
1012+
switch (spType) {
1013+
case OntapiSCSI:
1014+
return Storage.ImageFormat.RAW;
1015+
case NetworkFilesystem:
1016+
return Storage.ImageFormat.QCOW2;
1017+
default:
1018+
throw new CloudRuntimeException("Unsupported pool type [" + spType + "] for ONTAP image format resolution");
10021019
}
1003-
throw new CloudRuntimeException("Unsupported hypervisor [" + hypervisorType + "] for ONTAP image format resolution");
10041020
}
1021+
10051022
/**
10061023
* Persists snapshot metadata in snapshot_details table.
10071024
*

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ public DataStore initialize(Map<String, Object> dsInfos) {
184184
logger.info("Setting NFS path for storage pool: " + path + ", port: " + port + " with mount option: vers=3");
185185
break;
186186
case ISCSI:
187-
parameters.setType(Storage.StoragePoolType.Iscsi);
187+
parameters.setType(Storage.StoragePoolType.OntapiSCSI);
188188
path = storageStrategy.getStoragePath();
189189
port = OntapStorageConstants.ISCSI_PORT;
190190
logger.info("Setting iSCSI path for storage pool: " + path + ", port: " + port);

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

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ void testCreateAsync_VolumeWithISCSI_Success() {
171171

172172
when(storagePoolDao.findById(1L)).thenReturn(storagePool);
173173
when(storagePool.getId()).thenReturn(1L);
174-
when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.Iscsi);
174+
when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.OntapiSCSI);
175175
when(storagePool.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM);
176176

177177
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails);
@@ -292,25 +292,17 @@ void testCreateAsync_KvmUnsupportedProtocol_FailsWithError() {
292292
when(dataStore.getId()).thenReturn(1L);
293293
when(dataStore.getName()).thenReturn("ontap-pool");
294294
when(volumeInfo.getType()).thenReturn(VOLUME);
295-
when(volumeInfo.getId()).thenReturn(100L);
296295
when(volumeInfo.getName()).thenReturn("test-volume");
297296

298297
when(storagePoolDao.findById(1L)).thenReturn(storagePool);
299-
when(storagePool.getId()).thenReturn(1L);
300-
when(storagePool.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM);
301298
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails);
302-
when(volumeDao.findById(100L)).thenReturn(volumeVO);
303-
304-
try (MockedStatic<OntapStorageUtils> utilityMock = mockStatic(OntapStorageUtils.class)) {
305-
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(sanStrategy);
306299

307-
driver.createAsync(dataStore, volumeInfo, createCallback);
300+
driver.createAsync(dataStore, volumeInfo, createCallback);
308301

309-
ArgumentCaptor<CreateCmdResult> resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class);
310-
verify(createCallback).complete(resultCaptor.capture());
311-
assertFalse(resultCaptor.getValue().isSuccess());
312-
assertTrue(resultCaptor.getValue().getResult().contains("No enum constant"));
313-
}
302+
ArgumentCaptor<CreateCmdResult> resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class);
303+
verify(createCallback).complete(resultCaptor.capture());
304+
assertFalse(resultCaptor.getValue().isSuccess());
305+
assertTrue(resultCaptor.getValue().getResult().contains("Unsupported protocol [FC]"));
314306
}
315307

316308
@Test

0 commit comments

Comments
 (0)