StoragePoolType as class (#8544)

* StoragePoolType as a class

* Fix agent side StoragePoolType enum to class

* Handle StoragePoolType for StoragePoolJoinVO

* Since StoragePoolType is a class, it cannot be converted by @Enumerated annotation.
Implemented conveter class and logic to utilize @convert annotation.

* Fix UserVMJoinVO for StoragePoolType

* fixed missing imports

* Since StoragePoolType is a class, it cannot be converted by @Enumerated annotation.
Implemented conveter class and logic to utilize @convert annotation.

* Fixed equals for the enum.

* removed not needed try/catch for prepareAttribute

* Added license to the file.

* Implemented "supportsPhysicalDiskCopy" for storage adaptor.

Co-authored-by: mprokopchuk <mprokopchuk@apple.com>

* Add javadoc to StoragePoolType class

* Add unit test for StoragePoolType comparisons

* StoragePoolType "==" and ".equals()" fix.

* Fix StoragePoolType for FiberChannelAdapter

* Fix for abstract storage adaptor set up issue

* review comments

* Pass StoragePoolType object for poolType dao attribute

---------

Co-authored-by: Marcus Sorensen <mls@apple.com>
Co-authored-by: mprokopchuk <mprokopchuk@apple.com>
Co-authored-by: mprokopchuk <mprokopchuk@gmail.com>

Author: 3 people

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

@@ -16,10 +16,14 @@
 // under the License.
 package com.cloud.storage;
 
-import org.apache.commons.lang.NotImplementedException;
-
 import java.util.ArrayList;
+import java.util.LinkedHashMap;
 import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+
+import org.apache.commons.lang.NotImplementedException;
+import org.apache.commons.lang3.StringUtils;
 
 public class Storage {
     public static enum ImageFormat {
@@ -135,37 +139,72 @@ public static enum TemplateType {
         ISODISK /* Template corresponding to a iso (non root disk) present in an OVA */
     }
 
-    public static enum StoragePoolType {
-        Filesystem(false, true, true), // local directory
-        NetworkFilesystem(true, true, true), // NFS
-        IscsiLUN(true, false, false), // shared LUN, with a clusterfs overlay
-        Iscsi(true, false, false), // for e.g., ZFS Comstar
-        ISO(false, false, false), // for iso image
-        LVM(false, false, false), // XenServer local LVM SR
-        CLVM(true, false, false),
-        RBD(true, true, false), // http://libvirt.org/storage.html#StorageBackendRBD
-        SharedMountPoint(true, false, true),
-        VMFS(true, true, false), // VMware VMFS storage
-        PreSetup(true, true, false), // for XenServer, Storage Pool is set up by customers.
-        EXT(false, true, false), // XenServer local EXT SR
-        OCFS2(true, false, false),
-        SMB(true, false, false),
-        Gluster(true, false, false),
-        PowerFlex(true, true, true), // Dell EMC PowerFlex/ScaleIO (formerly VxFlexOS)
-        ManagedNFS(true, false, false),
-        Linstor(true, true, false),
-        DatastoreCluster(true, true, false), // for VMware, to abstract pool of clusters
-        StorPool(true, true, true),
-        FiberChannel(true, true, false); // Fiber Channel Pool for KVM hypervisors is used to find the volume by WWN value (/dev/disk/by-id/wwn-<wwnvalue>)
-
+    /**
+     * StoragePoolTypes carry some details about the format and capabilities of a storage pool. While not necessarily a
+     * 1:1 with PrimaryDataStoreDriver (and for KVM agent, KVMStoragePool and StorageAdaptor) implementations, it is
+     * often used to decide which storage plugin or storage command to call, so it may be necessary for new storage
+     * plugins to add a StoragePoolType.  This can be done by adding it below, or by creating a new public static final
+     * instance of StoragePoolType in the plugin itself, which registers it with the map.
+     *
+     * Note that if the StoragePoolType is for KVM and defined in plugin code rather than below, care must be taken to
+     * ensure this is available on the agent side as well. This is best done by defining the StoragePoolType in a common
+     * package available on both management server and agent plugin jars.
+     */
+    public static class StoragePoolType {
+        private static final Map<String, StoragePoolType> map = new LinkedHashMap<>();
+
+        public static final StoragePoolType Filesystem = new StoragePoolType("Filesystem", false, true, true);
+        public static final StoragePoolType NetworkFilesystem = new StoragePoolType("NetworkFilesystem", true, true, true);
+        public static final StoragePoolType IscsiLUN = new StoragePoolType("IscsiLUN", true, false, false);
+        public static final StoragePoolType Iscsi = new StoragePoolType("Iscsi", true, false, false);
+        public static final StoragePoolType ISO = new StoragePoolType("ISO", false, false, false);
+        public static final StoragePoolType LVM = new StoragePoolType("LVM", false, false, false);
+        public static final StoragePoolType CLVM = new StoragePoolType("CLVM", true, false, false);
+        public static final StoragePoolType RBD = new StoragePoolType("RBD", true, true, false);
+        public static final StoragePoolType SharedMountPoint = new StoragePoolType("SharedMountPoint", true, false, true);
+        public static final StoragePoolType VMFS = new StoragePoolType("VMFS", true, true, false);
+        public static final StoragePoolType PreSetup = new StoragePoolType("PreSetup", true, true, false);
+        public static final StoragePoolType EXT = new StoragePoolType("EXT", false, true, false);
+        public static final StoragePoolType OCFS2 = new StoragePoolType("OCFS2", true, false, false);
+        public static final StoragePoolType SMB = new StoragePoolType("SMB", true, false, false);
+        public static final StoragePoolType Gluster = new StoragePoolType("Gluster", true, false, false);
+        public static final StoragePoolType PowerFlex = new StoragePoolType("PowerFlex", true, true, true);
+        public static final StoragePoolType ManagedNFS = new StoragePoolType("ManagedNFS", true, false, false);
+        public static final StoragePoolType Linstor = new StoragePoolType("Linstor", true, true, false);
+        public static final StoragePoolType DatastoreCluster = new StoragePoolType("DatastoreCluster", true, true, false);
+        public static final StoragePoolType StorPool = new StoragePoolType("StorPool", true,true,true);
+        public static final StoragePoolType FiberChannel = new StoragePoolType("FiberChannel", true,true,false);
+
+
+        private final String name;
         private final boolean shared;
         private final boolean overprovisioning;
         private final boolean encryption;
 
-        StoragePoolType(boolean shared, boolean overprovisioning, boolean encryption) {
+        /**
+         * New StoragePoolType, set the name to check with it in Dao (Note: Do not register it into the map of pool types).
+         * @param name name of the StoragePoolType.
+         */
+        public StoragePoolType(String name) {
+            this.name = name;
+            this.shared = false;
+            this.overprovisioning = false;
+            this.encryption = false;
+        }
+
+        /**
+         * Define a new StoragePoolType, and register it into the map of pool types known to the management server.
+         * @param name Simple unique name of the StoragePoolType.
+         * @param shared Storage pool is shared/accessible to multiple hypervisors
+         * @param overprovisioning Storage pool supports overprovisioning
+         * @param encryption Storage pool supports encrypted volumes
+         */
+        public StoragePoolType(String name, boolean shared, boolean overprovisioning, boolean encryption) {
+            this.name = name;
             this.shared = shared;
             this.overprovisioning = overprovisioning;
             this.encryption = encryption;
+            addStoragePoolType(this);
         }
 
         public boolean isShared() {
@@ -177,6 +216,48 @@ public boolean supportsOverProvisioning() {
         }
 
         public boolean supportsEncryption() { return encryption; }
+
+        private static void addStoragePoolType(StoragePoolType storagePoolType) {
+            map.putIfAbsent(storagePoolType.name, storagePoolType);
+        }
+
+        public static StoragePoolType[] values() {
+            return map.values().toArray(StoragePoolType[]::new).clone();
+        }
+
+        public static StoragePoolType valueOf(String name) {
+            if (StringUtils.isBlank(name)) {
+                return null;
+            }
+
+            StoragePoolType storage = map.get(name);
+            if (storage == null) {
+                throw new IllegalArgumentException("StoragePoolType '" + name + "' not found");
+            }
+            return storage;
+        }
+
+        @Override
+        public String toString() {
+            return name;
+        }
+
+        public String name() {
+            return name;
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) return true;
+            if (o == null || getClass() != o.getClass()) return false;
+            StoragePoolType that = (StoragePoolType) o;
+            return Objects.equals(name, that.name);
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(name);
+        }
     }
 
     public static List<StoragePoolType> getNonSharedStoragePoolTypes() {

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

@@ -74,4 +74,13 @@ public void supportsOverprovisioningStoragePool() {
         Assert.assertTrue(StoragePoolType.DatastoreCluster.supportsOverProvisioning());
         Assert.assertTrue(StoragePoolType.Linstor.supportsOverProvisioning());
     }
+
+    @Test
+    public void equalityTest() {
+        StoragePoolType t1 = StoragePoolType.NetworkFilesystem;
+        StoragePoolType t2 = StoragePoolType.NetworkFilesystem;
+        Assert.assertTrue(t1 == StoragePoolType.NetworkFilesystem);
+        Assert.assertTrue(t1.equals(StoragePoolType.NetworkFilesystem));
+        Assert.assertFalse(t1.equals(StoragePoolType.EXT));
+    }
 }

core/src/main/java/com/cloud/agent/transport/StoragePoolTypeAdaptor.java

@@ -0,0 +1,53 @@
+// 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.agent.transport;
+
+import com.cloud.storage.Storage.StoragePoolType;
+import com.google.gson.JsonDeserializationContext;
+import com.google.gson.JsonDeserializer;
+import com.google.gson.JsonElement;
+import com.google.gson.JsonNull;
+import com.google.gson.JsonParseException;
+import com.google.gson.JsonPrimitive;
+import com.google.gson.JsonSerializationContext;
+import com.google.gson.JsonSerializer;
+
+import java.lang.reflect.Type;
+
+/**
+ * {@link StoragePoolType} acts as extendable set of singleton objects and should return same result when used "=="
+ * or {@link Object#equals(Object)}.
+ * To support that, need to return existing object for a given name instead of creating new.
+ */
+public class StoragePoolTypeAdaptor implements JsonDeserializer<StoragePoolType>, JsonSerializer<StoragePoolType> {
+    @Override
+    public StoragePoolType deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) throws JsonParseException {
+        if (json instanceof JsonPrimitive && ((JsonPrimitive) json).isString()) {
+            return StoragePoolType.valueOf(json.getAsString());
+        }
+        return null;
+    }
+
+    @Override
+    public JsonElement serialize(StoragePoolType src, Type typeOfSrc, JsonSerializationContext context) {
+        String name = src.name();
+        if (name == null) {
+            return new JsonNull();
+        }
+        return new JsonPrimitive(name);
+    }
+}

core/src/main/java/com/cloud/serializer/GsonHelper.java

@@ -37,6 +37,8 @@
 import com.cloud.agent.transport.LoggingExclusionStrategy;
 import com.cloud.agent.transport.Request.NwGroupsCommandTypeAdaptor;
 import com.cloud.agent.transport.Request.PortConfigListTypeAdaptor;
+import com.cloud.agent.transport.StoragePoolTypeAdaptor;
+import com.cloud.storage.Storage;
 import com.cloud.utils.Pair;
 
 public class GsonHelper {
@@ -69,6 +71,7 @@ static Gson setDefaultGsonConfig(GsonBuilder builder) {
         }.getType(), new PortConfigListTypeAdaptor());
         builder.registerTypeAdapter(new TypeToken<Pair<Long, Long>>() {
         }.getType(), new NwGroupsCommandTypeAdaptor());
+        builder.registerTypeAdapter(Storage.StoragePoolType.class, new StoragePoolTypeAdaptor());
         Gson gson = builder.create();
         dsAdaptor.initGson(gson);
         dtAdaptor.initGson(gson);

core/src/test/java/com/cloud/agent/transport/RequestTest.java

@@ -255,7 +255,7 @@ protected void compareRequest(Request req1, Request req2) {
     public void testGoodCommand() {
         s_logger.info("Testing good Command");
         String content = "[{\"com.cloud.agent.api.GetVolumeStatsCommand\":{\"volumeUuids\":[\"dcc860ac-4a20-498f-9cb3-bab4d57aa676\"],"
-                + "\"poolType\":\"NetworkFilesystem\",\"poolUuid\":\"e007c270-2b1b-3ce9-ae92-a98b94eef7eb\",\"contextMap\":{},\"wait\":5}}]";
+                + "\"poolType\":{\"name\":\"NetworkFilesystem\"},\"poolUuid\":\"e007c270-2b1b-3ce9-ae92-a98b94eef7eb\",\"contextMap\":{},\"wait\":5}}]";
         Request sreq = new Request(Version.v2, 1L, 2L, 3L, 1L, (short)1, content);
         sreq.setSequence(1);
         Command cmds[] = sreq.getCommands();
@@ -266,7 +266,7 @@ public void testGoodCommand() {
     public void testBadCommand() {
         s_logger.info("Testing Bad Command");
         String content = "[{\"com.cloud.agent.api.SomeJunkCommand\":{\"volumeUuids\":[\"dcc860ac-4a20-498f-9cb3-bab4d57aa676\"],"
-                + "\"poolType\":\"NetworkFilesystem\",\"poolUuid\":\"e007c270-2b1b-3ce9-ae92-a98b94eef7eb\",\"contextMap\":{},\"wait\":5}}]";
+                + "\"poolType\":{\"name\":\"NetworkFilesystem\"},\"poolUuid\":\"e007c270-2b1b-3ce9-ae92-a98b94eef7eb\",\"contextMap\":{},\"wait\":5}}]";
         Request sreq = new Request(Version.v2, 1L, 2L, 3L, 1L, (short)1, content);
         sreq.setSequence(1);
         Command cmds[] = sreq.getCommands();

engine/orchestration/src/main/java/org/apache/cloudstack/engine/datacenter/entity/api/db/EngineHostVO.java

@@ -22,6 +22,7 @@
 import java.util.UUID;
 
 import javax.persistence.Column;
+import javax.persistence.Convert;
 import javax.persistence.DiscriminatorColumn;
 import javax.persistence.DiscriminatorType;
 import javax.persistence.Entity;
@@ -45,6 +46,7 @@
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
 import com.cloud.resource.ResourceState;
 import com.cloud.storage.Storage.StoragePoolType;
+import com.cloud.util.StoragePoolTypeConverter;
 import com.cloud.utils.NumbersUtil;
 import com.cloud.utils.db.GenericDao;
 import com.cloud.utils.db.StateMachine;
@@ -126,6 +128,7 @@ public class EngineHostVO implements EngineHost, Identity {
     private String resource;
 
     @Column(name = "fs_type")
+    @Convert(converter = StoragePoolTypeConverter.class)
     private StoragePoolType fsType;
 
     @Column(name = "available")

engine/schema/src/main/java/com/cloud/host/HostVO.java

@@ -23,6 +23,7 @@
 import java.util.UUID;
 
 import javax.persistence.Column;
+import javax.persistence.Convert;
 import javax.persistence.DiscriminatorColumn;
 import javax.persistence.DiscriminatorType;
 import javax.persistence.Entity;
@@ -44,6 +45,7 @@
 import com.cloud.offering.ServiceOffering;
 import com.cloud.resource.ResourceState;
 import com.cloud.storage.Storage.StoragePoolType;
+import com.cloud.util.StoragePoolTypeConverter;
 import com.cloud.utils.NumbersUtil;
 import com.cloud.utils.db.GenericDao;
 import java.util.Arrays;
@@ -130,6 +132,7 @@ public class HostVO implements Host {
     private String resource;
 
     @Column(name = "fs_type")
+    @Convert(converter = StoragePoolTypeConverter.class)
     private StoragePoolType fsType;
 
     @Column(name = "available")

engine/schema/src/main/java/com/cloud/storage/VolumeVO.java

@@ -20,6 +20,7 @@
 import java.util.UUID;
 
 import javax.persistence.Column;
+import javax.persistence.Convert;
 import javax.persistence.Entity;
 import javax.persistence.EnumType;
 import javax.persistence.Enumerated;
@@ -32,6 +33,7 @@
 import javax.persistence.TemporalType;
 import javax.persistence.Transient;
 
+import com.cloud.util.StoragePoolTypeConverter;
 import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
 
 import com.cloud.storage.Storage.ProvisioningType;
@@ -114,7 +116,7 @@ public class VolumeVO implements Volume {
     Type volumeType = Volume.Type.UNKNOWN;
 
     @Column(name = "pool_type")
-    @Enumerated(EnumType.STRING)
+    @Convert(converter = StoragePoolTypeConverter.class)
     StoragePoolType poolType;
 
     @Column(name = GenericDao.REMOVED_COLUMN)
@@ -331,9 +333,7 @@ public void setPoolType(StoragePoolType poolType) {
         this.poolType = poolType;
     }
 
-    public StoragePoolType getPoolType() {
-        return poolType;
-    }
+    public StoragePoolType getPoolType() { return poolType; }
 
     @Override
     public long getDomainId() {

engine/schema/src/main/java/com/cloud/util/StoragePoolTypeConverter.java

@@ -0,0 +1,40 @@
+// 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
+// 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.util;
+
+import com.cloud.storage.Storage.StoragePoolType;
+
+import javax.persistence.AttributeConverter;
+import javax.persistence.Converter;
+
+/**
+ * Converts {@link StoragePoolType} to and from {@link String} using {@link StoragePoolType#name()}.
+ *
+ * @author mprokopchuk
+ */
+@Converter
+public class StoragePoolTypeConverter implements AttributeConverter<StoragePoolType, String> {
+    @Override
+    public String convertToDatabaseColumn(StoragePoolType attribute) {
+        return attribute != null ? attribute.name() : null;
+    }
+
+    @Override
+    public StoragePoolType convertToEntityAttribute(String dbData) {
+        return dbData != null ? StoragePoolType.valueOf(dbData) : null;
+    }
+}