Feature/cstackex 251: New pool type for Netapp iscsi usecase

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

@@ -185,7 +185,8 @@ public static enum StoragePoolType {
         Linstor(true, true, EncryptionSupport.Storage),
         DatastoreCluster(true, true, EncryptionSupport.Unsupported), // for VMware, to abstract pool of clusters
         StorPool(true, true, EncryptionSupport.Hypervisor),
-        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>)
+        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>)
+        OntapiSCSI(true, false, EncryptionSupport.Unsupported); // NetApp ONTAP iSCSI: one FlexVol per pool, one LUN per volume
 
         private final boolean shared;
         private final boolean overProvisioning;

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

@@ -49,6 +49,7 @@ public void isSharedStoragePool() {
         Assert.assertTrue(StoragePoolType.ManagedNFS.isShared());
         Assert.assertTrue(StoragePoolType.DatastoreCluster.isShared());
         Assert.assertTrue(StoragePoolType.Linstor.isShared());
+        Assert.assertTrue(StoragePoolType.OntapiSCSI.isShared());
     }
 
     @Test
@@ -73,6 +74,7 @@ public void supportsOverProvisioningTestAllStoragePoolTypes() {
         Assert.assertFalse(StoragePoolType.ManagedNFS.supportsOverProvisioning());
         Assert.assertTrue(StoragePoolType.DatastoreCluster.supportsOverProvisioning());
         Assert.assertTrue(StoragePoolType.Linstor.supportsOverProvisioning());
+        Assert.assertFalse(StoragePoolType.OntapiSCSI.supportsOverProvisioning());
     }
 
     @Test

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

@@ -616,8 +616,8 @@ private void handleVolumeMigrationFromManagedStorageToNonManagedStorage(VolumeIn
     private void verifyFormatWithPoolType(ImageFormat imageFormat, StoragePoolType poolType) {
         if (imageFormat != ImageFormat.VHD && imageFormat != ImageFormat.OVA && imageFormat != ImageFormat.QCOW2 &&
                 !(imageFormat == ImageFormat.RAW && (StoragePoolType.PowerFlex == poolType ||
-                StoragePoolType.FiberChannel == poolType))) {
-            throw new CloudRuntimeException(String.format("Only the following image types are currently supported: %s, %s, %s, %s (for PowerFlex and FiberChannel)",
+                StoragePoolType.FiberChannel == poolType || StoragePoolType.OntapiSCSI == poolType))) {
+            throw new CloudRuntimeException(String.format("Only the following image types are currently supported: %s, %s, %s, %s (for PowerFlex, FiberChannel and OntapiSCSI)",
                 ImageFormat.VHD.toString(), ImageFormat.OVA.toString(), ImageFormat.QCOW2.toString(), ImageFormat.RAW.toString()));
         }
     }

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

@@ -415,7 +415,8 @@ public Answer copyTemplateToPrimaryStorage(final CopyCommand cmd) {
                     StoragePoolType.PowerFlex,
                     StoragePoolType.Linstor,
                     StoragePoolType.FiberChannel,
-                    StoragePoolType.CLVM).contains(primaryPool.getType())) {
+                    StoragePoolType.CLVM,
+                    StoragePoolType.OntapiSCSI).contains(primaryPool.getType())) {
                     newTemplate.setFormat(ImageFormat.RAW);
                 } else {
                     newTemplate.setFormat(ImageFormat.QCOW2);
@@ -3431,7 +3432,8 @@ private Storage.ImageFormat getFormat(StoragePoolType poolType) {
                 StoragePoolType.PowerFlex,
                 StoragePoolType.Linstor,
                 StoragePoolType.FiberChannel,
-                StoragePoolType.CLVM).contains(poolType)) {
+                StoragePoolType.CLVM,
+                StoragePoolType.OntapiSCSI).contains(poolType)) {
             return ImageFormat.RAW;
         } else {
             return ImageFormat.QCOW2;

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

@@ -0,0 +1,38 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.hypervisor.kvm.storage;
+
+import com.cloud.storage.Storage.StoragePoolType;
+
+/**
+ * Serves {@link StoragePoolType#OntapiSCSI} pools, which are ONTAP FlexVols exposed over iSCSI with one
+ * LUN per CloudStack volume. The host-side handling is identical to a generic iSCSI target
+ *
+ * The class exists so that ONTAP-specific host behaviour can diverge here without altering the storage
+ * path of the other vendors that register as {@link StoragePoolType#Iscsi} which all share the superclass.
+ *
+ * This must stay in the {@code com.cloud.hypervisor.kvm.storage} package: {@link KVMStoragePoolManager}
+ * discovers adaptors by a Reflections scan of that package alone, and an unregistered type silently
+ * falls back to {@link LibvirtStorageAdaptor} rather than failing at startup.
+ */
+public class OntapIscsiStorageAdaptor extends IscsiAdmStorageAdaptor {
+
+    @Override
+    public StoragePoolType getStoragePoolType() {
+        return StoragePoolType.OntapiSCSI;
+    }
+}

plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/OntapIscsiStorageAdaptorTest.java

@@ -0,0 +1,78 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.hypervisor.kvm.storage;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+
+import java.lang.reflect.Modifier;
+import java.util.Set;
+
+import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat;
+import org.junit.Test;
+import org.reflections.Reflections;
+
+import com.cloud.storage.Storage.StoragePoolType;
+
+public class OntapIscsiStorageAdaptorTest {
+
+    @Test
+    public void getStoragePoolTypeReturnsOntapIscsi() {
+        assertEquals(StoragePoolType.OntapiSCSI, new OntapIscsiStorageAdaptor().getStoragePoolType());
+    }
+
+    @Test
+    public void createdPoolCarriesOntapIscsiTypeAndRawFormat() {
+        OntapIscsiStorageAdaptor adaptor = new OntapIscsiStorageAdaptor();
+
+        KVMStoragePool pool = adaptor.createStoragePool("ontap-iscsi-pool-uuid", "10.0.0.1", 3260, null, null,
+                StoragePoolType.OntapiSCSI, null, true);
+
+        assertEquals(StoragePoolType.OntapiSCSI, pool.getType());
+        // Attach builds a block-based disk off the physical disk format rather than the pool type,
+        // which is why splitting OntapiSCSI out of Iscsi leaves the generated domain XML unchanged.
+        assertEquals(PhysicalDiskFormat.RAW, pool.getDefaultFormat());
+        assertSame(pool, adaptor.getStoragePool("ontap-iscsi-pool-uuid"));
+    }
+
+    /**
+     * KVMStoragePoolManager discovers adaptors by a Reflections scan of its own package, instantiating
+     * each concrete implementation through a no-arg constructor and keying it on getStoragePoolType().
+     * A type with no adaptor silently falls back to LibvirtStorageAdaptor instead of failing at
+     * startup, so this reproduces the discovery preconditions rather than waiting for the symptom.
+     * The manager itself is not constructed here because doing so also instantiates
+     * MultipathSCSIAdapterBase, which requires agent scripts resolvable from the working directory.
+     */
+    @Test
+    public void adaptorSatisfiesThePoolManagerDiscoveryContract() throws ReflectiveOperationException {
+        String scannedPackage = KVMStoragePoolManager.class.getPackage().getName();
+        Set<Class<? extends StorageAdaptor>> discovered =
+                new Reflections(scannedPackage).getSubTypesOf(StorageAdaptor.class);
+
+        assertTrue("OntapIscsiStorageAdaptor must live in " + scannedPackage + " to be discovered",
+                discovered.contains(OntapIscsiStorageAdaptor.class));
+        assertFalse("An abstract adaptor is skipped by the scan",
+                Modifier.isAbstract(OntapIscsiStorageAdaptor.class.getModifiers()));
+
+        StorageAdaptor adaptor = OntapIscsiStorageAdaptor.class.getDeclaredConstructor().newInstance();
+        assertEquals(StoragePoolType.OntapiSCSI, adaptor.getStoragePoolType());
+        assertEquals("The superclass must keep serving the other iSCSI vendors",
+                StoragePoolType.Iscsi, new IscsiAdmStorageAdaptor().getStoragePoolType());
+    }
+}

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

@@ -160,8 +160,8 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet
 
                     volumeVO.setPoolType(storagePool.getPoolType());
                     volumeVO.setPoolId(storagePool.getId());
-                    volumeVO.setFormat(getImageFormatByHypervisorAndProtocol(storagePool.getHypervisor(), details.get(OntapStorageConstants.PROTOCOL)));
-                    logger.info("createAsync: Volume format set to [{}] for hypervisor [{}] and protocol [{}]", volumeVO.getFormat(), storagePool.getHypervisor(), details.get(OntapStorageConstants.PROTOCOL));
+                    volumeVO.setFormat(getImageFormat(storagePool));
+                    logger.info("createAsync: Volume format set to [{}] for pool type [{}]", volumeVO.getFormat(), storagePool.getPoolType());
 
                     if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL))) {
                         String lunName = created != null && created.getLun() != null ? created.getLun().getName() : null;
@@ -987,20 +987,14 @@ private String buildSnapshotName(String cloudStackSnapshotName, long snapshotId)
         return OntapStorageUtils.buildOntapSnapshotName(cloudStackSnapshotName, OntapStorageConstants.CS + snapshotId);
     }
 
-
-    private Storage.ImageFormat getImageFormatByHypervisorAndProtocol(HypervisorType hypervisorType, String protocol) {
-        if (HypervisorType.KVM.equals(hypervisorType)) {
-            ProtocolType protocolType = ProtocolType.valueOf(protocol);
-            switch (protocolType) {
-                case NFS3:
-                    return Storage.ImageFormat.QCOW2;
-                case ISCSI:
-                    return Storage.ImageFormat.RAW;
-                default:
-                    throw new CloudRuntimeException("Unsupported protocol [" + protocol + "] for ONTAP image format resolution");
-            }
+    private Storage.ImageFormat getImageFormat(StoragePoolVO storagePool) {
+        HypervisorType hypervisorType = storagePool.getHypervisor();
+        if (!HypervisorType.KVM.equals(hypervisorType)) {
+            throw new CloudRuntimeException("Unsupported hypervisor [" + hypervisorType + "] for ONTAP image format resolution");
         }
-        throw new CloudRuntimeException("Unsupported hypervisor [" + hypervisorType + "] for ONTAP image format resolution");
+        return Storage.StoragePoolType.OntapiSCSI.equals(storagePool.getPoolType())
+                ? Storage.ImageFormat.RAW
+                : Storage.ImageFormat.QCOW2;
     }
     /**
      * Persists snapshot metadata in snapshot_details table.

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

@@ -184,7 +184,7 @@ public DataStore initialize(Map<String, Object> dsInfos) {
                 logger.info("Setting NFS path for storage pool: " + path + ", port: " + port + " with mount option: vers=3");
                 break;
             case ISCSI:
-                parameters.setType(Storage.StoragePoolType.Iscsi);
+                parameters.setType(Storage.StoragePoolType.OntapiSCSI);
                 path = storageStrategy.getStoragePath();
                 port = OntapStorageConstants.ISCSI_PORT;
                 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

@@ -171,7 +171,7 @@ void testCreateAsync_VolumeWithISCSI_Success() {
 
         when(storagePoolDao.findById(1L)).thenReturn(storagePool);
         when(storagePool.getId()).thenReturn(1L);
-        when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.Iscsi);
+        when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.OntapiSCSI);
         when(storagePool.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM);
 
         when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails);

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

@@ -23,6 +23,7 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.ArgumentCaptor;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.MockedStatic;
@@ -37,10 +38,12 @@
 import com.cloud.dc.ClusterVO;
 import com.cloud.host.HostVO;
 import com.cloud.resource.ResourceManager;
+import com.cloud.storage.Storage;
 import com.cloud.storage.StorageManager;
 import org.apache.cloudstack.engine.subsystem.api.storage.ClusterScope;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
 import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreParameters;
 import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
 import org.apache.cloudstack.storage.service.model.AccessGroup;
@@ -58,6 +61,7 @@
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.withSettings;
 import static org.mockito.ArgumentMatchers.contains;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -191,6 +195,50 @@ public void testInitialize_positive() {
         }
     }
 
+    private Map<String, Object> buildDsInfosForProtocol(String protocol) {
+        HashMap<String, String> detailsMap = new HashMap<String, String>();
+        detailsMap.put(OntapStorageConstants.USERNAME, "testUser");
+        detailsMap.put(OntapStorageConstants.PASSWORD, "testPassword");
+        detailsMap.put(OntapStorageConstants.STORAGE_IP, "10.10.10.10");
+        detailsMap.put(OntapStorageConstants.SVM_NAME, "vs0");
+        detailsMap.put(OntapStorageConstants.PROTOCOL, protocol);
+
+        Map<String, Object> dsInfos = new HashMap<>();
+        dsInfos.put("zoneId", 1L);
+        dsInfos.put("podId", 1L);
+        dsInfos.put("clusterId", 1L);
+        dsInfos.put("name", "testStoragePool");
+        dsInfos.put("providerName", "testProvider");
+        dsInfos.put("capacityBytes", 200000L);
+        dsInfos.put("managed", true);
+        dsInfos.put("tags", "testTag");
+        dsInfos.put("isTagARule", false);
+        dsInfos.put("details", detailsMap);
+        return dsInfos;
+    }
+
+    private Storage.StoragePoolType initializeAndCapturePoolType(String protocol) {
+        try (MockedStatic<StorageProviderFactory> storageProviderFactory = Mockito.mockStatic(StorageProviderFactory.class)) {
+            storageProviderFactory.when(() -> StorageProviderFactory.getStrategy(any())).thenReturn(storageStrategy);
+            ontapPrimaryDatastoreLifecycle.initialize(buildDsInfosForProtocol(protocol));
+        }
+        ArgumentCaptor<PrimaryDataStoreParameters> captor = ArgumentCaptor.forClass(PrimaryDataStoreParameters.class);
+        verify(_dataStoreHelper).createPrimaryDataStore(captor.capture());
+        return captor.getValue().getType();
+    }
+
+    @Test
+    public void testInitialize_iscsiPoolUsesOntapIscsiType() {
+        when(storageStrategy.getStoragePath()).thenReturn("iqn.1992-08.com.netapp:sn.abc123");
+
+        assertEquals(Storage.StoragePoolType.OntapiSCSI, initializeAndCapturePoolType("ISCSI"));
+    }
+
+    @Test
+    public void testInitialize_nfsPoolKeepsNetworkFilesystemType() {
+        assertEquals(Storage.StoragePoolType.NetworkFilesystem, initializeAndCapturePoolType("NFS3"));
+    }
+
     @Test
     public void testInitialize_null_Arg() {
         Exception ex = assertThrows(CloudRuntimeException.class,() ->

server/src/main/java/com/cloud/api/ApiDBUtils.java

@@ -1338,7 +1338,7 @@ public static HypervisorType getHypervisorTypeFromFormat(long dcId, ImageFormat
                 type = HypervisorType.Hyperv;
             }
         } if (format == ImageFormat.RAW) {
-            // Currently, KVM only supports RBD, PowerFlex, and FiberChannel images of type RAW.
+            // Currently, KVM only supports RBD, PowerFlex, FiberChannel and OntapiSCSI images of type RAW.
             // This results in a weird collision with OVM volumes which
             // can only be raw, thus making KVM RBD volumes show up as OVM
             // rather than RBD. This block of code can (hopefully) by checking to
@@ -1355,7 +1355,8 @@ public static HypervisorType getHypervisorTypeFromFormat(long dcId, ImageFormat
                            StoragePoolType.PowerFlex,
                            StoragePoolType.CLVM,
                            StoragePoolType.Linstor,
-                           StoragePoolType.FiberChannel).contains(pool.getPoolType())) {
+                           StoragePoolType.FiberChannel,
+                           StoragePoolType.OntapiSCSI).contains(pool.getPoolType())) {
                   // This case will note the presence of non-qcow2 primary stores, suggesting KVM without NFS. Otherwse,
                   // If this check is not passed, the hypervisor type will remain OVM.
                   type = HypervisorType.KVM;