Merge pull request #1615 from nvazquez/nfsConfigKey

CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Make NFS version changeable in UIJIRA TICKET: https://issues.apache.org/jira/browse/CLOUDSTACK-9438

### Introduction

From #1361 it was possible to configure NFS version for secondary storage mount.
However, changing NFS version requires inserting an new detail on `image_store_details` table, with `name = 'nfs.version'` and `value = X` where X is desired NFS version, and then restarting management server for changes to take effect.

Our improvement aims to make NFS version changeable from UI, instead of previously described workflow.

### Proposed solution
Basically, NFS version is defined as an image store ConfigKey, this implied:
* Adding a new Config scope: **ImageStore**
* Make `ImageStoreDetailsDao` class to extend `ResourceDetailsDaoBase` and `ImageStoreDetailVO` implement `ResourceDetail`
* Insert `'display'` column on `image_store_details` table
* Extending `ListCfgsCmd` and `UpdateCfgCmd` to support **ImageStore** scope, which implied:
** Injecting `ImageStoreDetailsDao` and `ImageStoreDao` on `ConfigurationManagerImpl` class, on `cloud-server` module.

### Important
It is important to mention that `ImageStoreDaoImpl` and `ImageStoreDetailsDaoImpl` classes were moved from `cloud-engine-storage` to `cloud-engine-schema` module in order to Spring find those beans to inject on `ConfigurationManagerImpl` in `cloud-server` module.

We had this maven dependencies between modules:
* `cloud-server --> cloud-engine-schema`
* `cloud-engine-storage --> cloud-secondary-storage --> cloud-server`

As `ImageStoreDaoImpl` and `ImageStoreDetailsDao` were defined in `cloud-engine-storage`, and they needed in `cloud-server` module, to be injected on `ConfigurationManagerImpl`, if we added dependency from `cloud-server` to `cloud-engine-storage` we would introduce a dependency cycle. To avoid this cycle, we moved those classes to `cloud-engine-schema` module

* pr/1615:
  CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Make NFS version changeable in UI

Signed-off-by: Rajani Karuturi <rajani.karuturi@accelerite.com>

Author: karuturi

api/src/org/apache/cloudstack/api/command/admin/config/ListCfgsByCmd.java

@@ -28,6 +28,7 @@
 import org.apache.cloudstack.api.response.AccountResponse;
 import org.apache.cloudstack.api.response.ClusterResponse;
 import org.apache.cloudstack.api.response.ConfigurationResponse;
+import org.apache.cloudstack.api.response.ImageStoreResponse;
 import org.apache.cloudstack.api.response.ListResponse;
 import org.apache.cloudstack.api.response.StoragePoolResponse;
 import org.apache.cloudstack.api.response.ZoneResponse;
@@ -76,6 +77,12 @@ public class ListCfgsByCmd extends BaseListCmd {
                description = "the ID of the Account to update the parameter value for corresponding account")
     private Long accountId;
 
+    @Parameter(name = ApiConstants.IMAGE_STORE_UUID,
+            type = CommandType.UUID,
+            entityType = ImageStoreResponse.class,
+            description = "the ID of the Image Store to update the parameter value for corresponding image store")
+    private Long imageStoreId;
+
     // ///////////////////////////////////////////////////
     // ///////////////// Accessors ///////////////////////
     // ///////////////////////////////////////////////////
@@ -104,6 +111,10 @@ public Long getAccountId() {
         return accountId;
     }
 
+    public Long getImageStoreId() {
+        return imageStoreId;
+    }
+
     @Override
     public Long getPageSizeVal() {
         Long defaultPageSize = 500L;
@@ -147,6 +158,9 @@ public void execute() {
             if (getAccountId() != null) {
                 cfgResponse.setScope("account");
             }
+            if (getImageStoreId() != null){
+                cfgResponse.setScope("imagestore");
+            }
             configResponses.add(cfgResponse);
         }
 

api/src/org/apache/cloudstack/api/command/admin/config/UpdateCfgCmd.java

@@ -19,8 +19,8 @@
 import com.google.common.base.Strings;
 import org.apache.cloudstack.acl.RoleService;
 import org.apache.log4j.Logger;
-
 import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiArgValidator;
 import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.api.ApiErrorCode;
 import org.apache.cloudstack.api.BaseCmd;
@@ -29,6 +29,7 @@
 import org.apache.cloudstack.api.response.AccountResponse;
 import org.apache.cloudstack.api.response.ClusterResponse;
 import org.apache.cloudstack.api.response.ConfigurationResponse;
+import org.apache.cloudstack.api.response.ImageStoreResponse;
 import org.apache.cloudstack.api.response.StoragePoolResponse;
 import org.apache.cloudstack.api.response.ZoneResponse;
 import org.apache.cloudstack.config.Configuration;
@@ -75,6 +76,13 @@ public class UpdateCfgCmd extends BaseCmd {
                description = "the ID of the Account to update the parameter value for corresponding account")
     private Long accountId;
 
+    @Parameter(name = ApiConstants.IMAGE_STORE_UUID,
+            type = CommandType.UUID,
+            entityType = ImageStoreResponse.class,
+            description = "the ID of the Image Store to update the parameter value for corresponding image store",
+            validations = ApiArgValidator.PositiveNumber)
+    private Long imageStoreId;
+
     /////////////////////////////////////////////////////
     /////////////////// Accessors ///////////////////////
     /////////////////////////////////////////////////////
@@ -107,6 +115,10 @@ public Long getAccountId() {
         return accountId;
     }
 
+    public Long getImageStoreId() {
+        return imageStoreId;
+    }
+
     /////////////////////////////////////////////////////
     /////////////// API Implementation///////////////////
     /////////////////////////////////////////////////////

engine/components-api/src/com/cloud/capacity/CapacityManager.java

@@ -73,6 +73,16 @@ public interface CapacityManager {
                     "If set to true, creates VMs as full clones on ESX hypervisor",
                     true,
                     ConfigKey.Scope.StoragePool);
+    static final ConfigKey<Integer> ImageStoreNFSVersion =
+            new ConfigKey<Integer>(
+                    Integer.class,
+                    "secstorage.nfs.version",
+                    "Advanced",
+                    null,
+                    "Enforces specific NFS version when mounting Secondary Storage. If NULL default selection is performed",
+                    true,
+                    ConfigKey.Scope.ImageStore,
+                    null);
 
     public boolean releaseVmCapacity(VirtualMachine vm, boolean moveFromReserved, boolean moveToReservered, Long hostId);
 

engine/schema/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml

@@ -176,8 +176,8 @@
   <bean id="hostTagsDaoImpl" class="com.cloud.host.dao.HostTagsDaoImpl" />
   <bean id="hostTransferMapDaoImpl" class="com.cloud.cluster.agentlb.dao.HostTransferMapDaoImpl" />
   <bean id="iPAddressDaoImpl" class="com.cloud.network.dao.IPAddressDaoImpl" />
-  <bean id="imageStoreDaoImpl" class="org.apache.cloudstack.storage.image.db.ImageStoreDaoImpl" />
-  <bean id="imageStoreDetailsDaoImpl" class="org.apache.cloudstack.storage.image.db.ImageStoreDetailsDaoImpl" />
+  <bean id="imageStoreDaoImpl" class="org.apache.cloudstack.storage.datastore.db.ImageStoreDaoImpl" />
+  <bean id="imageStoreDetailsDaoImpl" class="org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDaoImpl" />
   <bean id="imageStoreJoinDaoImpl" class="com.cloud.api.query.dao.ImageStoreJoinDaoImpl" />
   <bean id="snapshotDataStoreDaoImpl" class="org.apache.cloudstack.storage.image.db.SnapshotDataStoreDaoImpl" />
   <bean id="templateDataStoreDaoImpl" class="org.apache.cloudstack.storage.image.db.TemplateDataStoreDaoImpl" />

…/storage/image/db/ImageStoreDaoImpl.java …rage/datastore/db/ImageStoreDaoImpl.javaengine/storage/src/org/apache/cloudstack/storage/image/db/ImageStoreDaoImpl.java renamed to engine/schema/src/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java engine/storage/src/org/apache/cloudstack/storage/image/db/ImageStoreDaoImpl.java renamed to engine/schema/src/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java

@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.cloudstack.storage.image.db;
+package org.apache.cloudstack.storage.datastore.db;
 
 import java.util.List;
 import java.util.Map;
@@ -26,8 +26,6 @@
 import org.springframework.stereotype.Component;
 
 import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope;
-import org.apache.cloudstack.storage.datastore.db.ImageStoreDao;
-import org.apache.cloudstack.storage.datastore.db.ImageStoreVO;
 
 import com.cloud.storage.DataStoreRole;
 import com.cloud.storage.ScopeType;

engine/schema/src/org/apache/cloudstack/storage/datastore/db/ImageStoreDetailVO.java

@@ -23,61 +23,61 @@
 import javax.persistence.Id;
 import javax.persistence.Table;
 
-import org.apache.cloudstack.api.InternalIdentity;
+import org.apache.cloudstack.api.ResourceDetail;
 
 @Entity
 @Table(name = "image_store_details")
-public class ImageStoreDetailVO implements InternalIdentity {
+public class ImageStoreDetailVO implements ResourceDetail {
     @Id
     @GeneratedValue(strategy = GenerationType.IDENTITY)
     @Column(name = "id")
     long id;
 
     @Column(name = "store_id")
-    long storeId;
+    long resourceId;
 
     @Column(name = "name")
     String name;
 
     @Column(name = "value")
     String value;
 
+    @Column(name = "display")
+    private boolean display = true;
+
     public ImageStoreDetailVO() {
     }
 
-    public ImageStoreDetailVO(long storeId, String name, String value) {
-        this.storeId = storeId;
+    public ImageStoreDetailVO(long storeId, String name, String value, boolean display) {
+        this.resourceId = storeId;
         this.name = name;
         this.value = value;
+        this.display = display;
     }
 
     @Override
     public long getId() {
         return id;
     }
 
-    public long getStoreId() {
-        return storeId;
-    }
-
-    public void setStoreId(long storeId) {
-        this.storeId = storeId;
+    @Override
+    public long getResourceId() {
+        return resourceId;
     }
 
+    @Override
     public String getName() {
         return name;
     }
 
-    public void setName(String name) {
-        this.name = name;
-    }
-
+    @Override
     public String getValue() {
         return value;
     }
 
-    public void setValue(String value) {
-        this.value = value;
+    @Override
+    public boolean isDisplay() {
+        return display;
     }
 
 }

engine/schema/src/org/apache/cloudstack/storage/datastore/db/ImageStoreDetailsDao.java

@@ -18,9 +18,11 @@
 
 import java.util.Map;
 
+import org.apache.cloudstack.resourcedetail.ResourceDetailsDao;
+
 import com.cloud.utils.db.GenericDao;
 
-public interface ImageStoreDetailsDao extends GenericDao<ImageStoreDetailVO, Long> {
+public interface ImageStoreDetailsDao extends GenericDao<ImageStoreDetailVO, Long>, ResourceDetailsDao<ImageStoreDetailVO> {
 
     void update(long storeId, Map<String, String> details);
 

…e/image/db/ImageStoreDetailsDaoImpl.java …tastore/db/ImageStoreDetailsDaoImpl.javaengine/storage/src/org/apache/cloudstack/storage/image/db/ImageStoreDetailsDaoImpl.java renamed to engine/schema/src/org/apache/cloudstack/storage/datastore/db/ImageStoreDetailsDaoImpl.java engine/storage/src/org/apache/cloudstack/storage/image/db/ImageStoreDetailsDaoImpl.java renamed to engine/schema/src/org/apache/cloudstack/storage/datastore/db/ImageStoreDetailsDaoImpl.java

@@ -14,34 +14,36 @@
 // KIND, either express or implied.  See the License for the
 // specific language governing permissions and limitations
 // under the License.
-package org.apache.cloudstack.storage.image.db;
+package org.apache.cloudstack.storage.datastore.db;
 
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
-
 import org.springframework.stereotype.Component;
 
-import org.apache.cloudstack.api.ApiConstants;
-import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailVO;
-import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDao;
-
 import com.cloud.utils.crypt.DBEncryptionUtil;
-import com.cloud.utils.db.GenericDaoBase;
+import com.cloud.utils.db.QueryBuilder;
 import com.cloud.utils.db.SearchBuilder;
 import com.cloud.utils.db.SearchCriteria;
+import com.cloud.utils.db.SearchCriteria.Op;
 import com.cloud.utils.db.TransactionLegacy;
 
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.ConfigKey.Scope;
+import org.apache.cloudstack.framework.config.ScopedConfigStorage;
+import org.apache.cloudstack.resourcedetail.ResourceDetailsDaoBase;
+
 @Component
-public class ImageStoreDetailsDaoImpl extends GenericDaoBase<ImageStoreDetailVO, Long> implements ImageStoreDetailsDao {
+public class ImageStoreDetailsDaoImpl extends ResourceDetailsDaoBase<ImageStoreDetailVO> implements ImageStoreDetailsDao, ScopedConfigStorage {
 
     protected final SearchBuilder<ImageStoreDetailVO> storeSearch;
 
-    protected ImageStoreDetailsDaoImpl() {
+    public ImageStoreDetailsDaoImpl() {
         super();
         storeSearch = createSearchBuilder();
-        storeSearch.and("store", storeSearch.entity().getStoreId(), SearchCriteria.Op.EQ);
+        storeSearch.and("store", storeSearch.entity().getResourceId(), SearchCriteria.Op.EQ);
         storeSearch.done();
     }
 
@@ -54,7 +56,7 @@ public void update(long storeId, Map<String, String> details) {
         txn.start();
         expunge(sc);
         for (Map.Entry<String, String> entry : details.entrySet()) {
-            ImageStoreDetailVO detail = new ImageStoreDetailVO(storeId, entry.getKey(), entry.getValue());
+            ImageStoreDetailVO detail = new ImageStoreDetailVO(storeId, entry.getKey(), entry.getValue(), true);
             persist(detail);
         }
         txn.commit();
@@ -88,7 +90,30 @@ public void deleteDetails(long storeId) {
         for (ImageStoreDetailVO result : results) {
             remove(result.getId());
         }
+    }
+
+    @Override
+    public Scope getScope() {
+        return ConfigKey.Scope.ImageStore;
+    }
+
+    @Override
+    public ImageStoreDetailVO findDetail(long storeId, String name) {
+        QueryBuilder<ImageStoreDetailVO> sc = QueryBuilder.create(ImageStoreDetailVO.class);
+        sc.and(sc.entity().getResourceId(), Op.EQ, storeId);
+        sc.and(sc.entity().getName(), Op.EQ, name);
+        return sc.find();
+    }
 
+    @Override
+    public String getConfigValue(long id, ConfigKey<?> key) {
+        ImageStoreDetailVO vo = findDetail(id, key.key());
+        return vo == null ? null : vo.getValue();
+    }
+
+    @Override
+    public void addDetail(long resourceId, String key, String value, boolean display) {
+        super.addDetail(new ImageStoreDetailVO(resourceId, key, value, display));
     }
 
 }

engine/storage/integration-test/test/resource/fakeDriverTestContext.xml

@@ -35,8 +35,8 @@
         </property>
     </bean>
 
-    <bean id="imageStoreDaoImpl" class="org.apache.cloudstack.storage.image.db.ImageStoreDaoImpl" />
-    <bean id="imageStoreDetailsDaoImpl" class="org.apache.cloudstack.storage.image.db.ImageStoreDetailsDaoImpl" />
+    <bean id="imageStoreDaoImpl" class="org.apache.cloudstack.storage.datastore.db.ImageStoreDaoImpl" />
+    <bean id="imageStoreDetailsDaoImpl" class="org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDaoImpl" />
     <bean id="snapshotDataStoreDaoImpl" class="org.apache.cloudstack.storage.image.db.SnapshotDataStoreDaoImpl" />
     <bean id="templateDataStoreDaoImpl" class="org.apache.cloudstack.storage.image.db.TemplateDataStoreDaoImpl" />
     <bean id="volumeDataStoreDaoImpl" class="org.apache.cloudstack.storage.image.db.VolumeDataStoreDaoImpl" />

engine/storage/integration-test/test/resources/storageContext.xml

@@ -35,8 +35,8 @@
     </property>
   </bean>
 
-<bean id="imageStoreDaoImpl" class="org.apache.cloudstack.storage.image.db.ImageStoreDaoImpl" />
-  <bean id="imageStoreDetailsDaoImpl" class="org.apache.cloudstack.storage.image.db.ImageStoreDetailsDaoImpl" /> 
+<bean id="imageStoreDaoImpl" class="org.apache.cloudstack.storage.datastore.db.ImageStoreDaoImpl" />
+  <bean id="imageStoreDetailsDaoImpl" class="org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDaoImpl" /> 
   <bean id="snapshotDataStoreDaoImpl" class="org.apache.cloudstack.storage.image.db.SnapshotDataStoreDaoImpl" /> 
   <bean id="templateDataStoreDaoImpl" class="org.apache.cloudstack.storage.image.db.TemplateDataStoreDaoImpl" /> 
   <bean id="volumeDataStoreDaoImpl" class="org.apache.cloudstack.storage.image.db.VolumeDataStoreDaoImpl" />