You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ni...@apache.org on 2014/05/30 19:49:16 UTC

git commit: updated refs/heads/master to 5393387

Repository: cloudstack
Updated Branches:
  refs/heads/master bdde5335f -> 5393387bb


CLOUDSTACK-6599:
1. Adding the missing Template/Volume URLs expiration functionality
2. Improvement - While deleting the volume during expiration use rm -rf as vmware now contains directoy
3. Improvement - Use standard Answer so that the error gets logged in case deletion of expiration link didnt work fine.
4. Improvement - In case of domain change, expire the old urls


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/5393387b
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/5393387b
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/5393387b

Branch: refs/heads/master
Commit: 5393387bbd4e2d3659fb0c7171e6ff347ad6a07b
Parents: bdde533
Author: Nitin Mehta <ni...@citrix.com>
Authored: Fri May 30 10:23:50 2014 -0700
Committer: Nitin Mehta <ni...@citrix.com>
Committed: Fri May 30 10:48:42 2014 -0700

----------------------------------------------------------------------
 .../storage/DeleteEntityDownloadURLAnswer.java  | 37 ---------
 .../image/datastore/ImageStoreEntity.java       |  3 +
 .../datastore/db/TemplateDataStoreDao.java      |  2 +
 .../datastore/db/TemplateDataStoreVO.java       | 23 ++++++
 .../datastore/db/VolumeDataStoreDao.java        |  2 +
 .../storage/datastore/db/VolumeDataStoreVO.java | 12 +++
 .../storage/image/store/ImageStoreImpl.java     |  7 ++
 .../storage/image/BaseImageStoreDriverImpl.java |  5 ++
 .../storage/image/ImageStoreDriver.java         |  3 +
 .../image/db/TemplateDataStoreDaoImpl.java      | 13 +++
 .../image/db/VolumeDataStoreDaoImpl.java        | 14 ++++
 .../driver/CloudStackImageStoreDriverImpl.java  | 26 ++++++
 .../configuration/ConfigurationManagerImpl.java | 25 ++++++
 .../src/com/cloud/storage/StorageManager.java   |  2 +
 .../com/cloud/storage/StorageManagerImpl.java   | 87 ++++++++++++++++++++
 .../com/cloud/storage/VolumeApiServiceImpl.java |  2 +
 .../com/cloud/template/TemplateManagerImpl.java | 13 ++-
 .../networkoffering/ChildTestConfiguration.java |  9 +-
 .../storage/template/UploadManager.java         |  4 +-
 .../storage/template/UploadManagerImpl.java     | 12 +--
 setup/db/db/schema-430to440.sql                 |  5 ++
 utils/src/com/cloud/utils/DateUtil.java         | 11 +++
 22 files changed, 269 insertions(+), 48 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5393387b/core/src/com/cloud/agent/api/storage/DeleteEntityDownloadURLAnswer.java
----------------------------------------------------------------------
diff --git a/core/src/com/cloud/agent/api/storage/DeleteEntityDownloadURLAnswer.java b/core/src/com/cloud/agent/api/storage/DeleteEntityDownloadURLAnswer.java
deleted file mode 100644
index 7b8da59..0000000
--- a/core/src/com/cloud/agent/api/storage/DeleteEntityDownloadURLAnswer.java
+++ /dev/null
@@ -1,37 +0,0 @@
-// 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.api.storage;
-
-import com.cloud.agent.api.Answer;
-
-public class DeleteEntityDownloadURLAnswer extends Answer {
-
-    String resultString;
-    short resultCode;
-    public static final short RESULT_SUCCESS = 1;
-    public static final short RESULT_FAILURE = 0;
-
-    public DeleteEntityDownloadURLAnswer(String resultString, short resultCode) {
-        super();
-        this.resultString = resultString;
-        this.resultCode = resultCode;
-    }
-
-    public DeleteEntityDownloadURLAnswer() {
-    }
-
-}

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5393387b/engine/api/src/org/apache/cloudstack/storage/image/datastore/ImageStoreEntity.java
----------------------------------------------------------------------
diff --git a/engine/api/src/org/apache/cloudstack/storage/image/datastore/ImageStoreEntity.java b/engine/api/src/org/apache/cloudstack/storage/image/datastore/ImageStoreEntity.java
index 43a0f75..461bd50 100644
--- a/engine/api/src/org/apache/cloudstack/storage/image/datastore/ImageStoreEntity.java
+++ b/engine/api/src/org/apache/cloudstack/storage/image/datastore/ImageStoreEntity.java
@@ -20,6 +20,7 @@ package org.apache.cloudstack.storage.image.datastore;
 
 import java.util.Set;
 
+import com.cloud.storage.Upload;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
 import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
@@ -43,4 +44,6 @@ public interface ImageStoreEntity extends DataStore, ImageStore {
     String getMountPoint(); // get the mount point on ssvm.
 
     String createEntityExtractUrl(String installPath, ImageFormat format, DataObject dataObject);  // get the entity download URL
+
+    void deleteExtractUrl(String installPath, String url, Upload.Type volume);
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5393387b/engine/schema/src/org/apache/cloudstack/storage/datastore/db/TemplateDataStoreDao.java
----------------------------------------------------------------------
diff --git a/engine/schema/src/org/apache/cloudstack/storage/datastore/db/TemplateDataStoreDao.java b/engine/schema/src/org/apache/cloudstack/storage/datastore/db/TemplateDataStoreDao.java
index 271bbca..cb15949 100644
--- a/engine/schema/src/org/apache/cloudstack/storage/datastore/db/TemplateDataStoreDao.java
+++ b/engine/schema/src/org/apache/cloudstack/storage/datastore/db/TemplateDataStoreDao.java
@@ -74,4 +74,6 @@ public interface TemplateDataStoreDao extends GenericDao<TemplateDataStoreVO, Lo
     List<TemplateDataStoreVO> listOnCache(long templateId);
 
     void updateStoreRoleToCachce(long storeId);
+
+    List<TemplateDataStoreVO> listTemplateDownloadUrls();
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5393387b/engine/schema/src/org/apache/cloudstack/storage/datastore/db/TemplateDataStoreVO.java
----------------------------------------------------------------------
diff --git a/engine/schema/src/org/apache/cloudstack/storage/datastore/db/TemplateDataStoreVO.java b/engine/schema/src/org/apache/cloudstack/storage/datastore/db/TemplateDataStoreVO.java
index bb40bf5..bb05300 100755
--- a/engine/schema/src/org/apache/cloudstack/storage/datastore/db/TemplateDataStoreVO.java
+++ b/engine/schema/src/org/apache/cloudstack/storage/datastore/db/TemplateDataStoreVO.java
@@ -98,6 +98,13 @@ public class TemplateDataStoreVO implements StateObject<ObjectInDataStoreStateMa
     @Column(name = "url")
     private String downloadUrl;
 
+    @Column(name = "download_url")
+    private String extractUrl;
+
+    @Column(name = "download_url_created")
+    @Temporal(value = TemporalType.TIMESTAMP)
+    private Date extractUrlCreated = null;
+
     @Column(name = "is_copy")
     private boolean isCopy = false;
 
@@ -379,4 +386,20 @@ public class TemplateDataStoreVO implements StateObject<ObjectInDataStoreStateMa
         }
     }
 
+    public String getExtractUrl() {
+        return extractUrl;
+    }
+
+    public void setExtractUrl(String extractUrl) {
+        this.extractUrl = extractUrl;
+    }
+
+    public Date getExtractUrlCreated() {
+        return extractUrlCreated;
+    }
+
+    public void setExtractUrlCreated(Date extractUrlCreated) {
+        this.extractUrlCreated = extractUrlCreated;
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5393387b/engine/schema/src/org/apache/cloudstack/storage/datastore/db/VolumeDataStoreDao.java
----------------------------------------------------------------------
diff --git a/engine/schema/src/org/apache/cloudstack/storage/datastore/db/VolumeDataStoreDao.java b/engine/schema/src/org/apache/cloudstack/storage/datastore/db/VolumeDataStoreDao.java
index f567f26..0016185 100644
--- a/engine/schema/src/org/apache/cloudstack/storage/datastore/db/VolumeDataStoreDao.java
+++ b/engine/schema/src/org/apache/cloudstack/storage/datastore/db/VolumeDataStoreDao.java
@@ -42,4 +42,6 @@ public interface VolumeDataStoreDao extends GenericDao<VolumeDataStoreVO, Long>,
     List<VolumeDataStoreVO> listDestroyed(long storeId);
 
     void duplicateCacheRecordsOnRegionStore(long storeId);
+
+    List<VolumeDataStoreVO> listVolumeDownloadUrls();
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5393387b/engine/schema/src/org/apache/cloudstack/storage/datastore/db/VolumeDataStoreVO.java
----------------------------------------------------------------------
diff --git a/engine/schema/src/org/apache/cloudstack/storage/datastore/db/VolumeDataStoreVO.java b/engine/schema/src/org/apache/cloudstack/storage/datastore/db/VolumeDataStoreVO.java
index 588eae8..aa57e74 100755
--- a/engine/schema/src/org/apache/cloudstack/storage/datastore/db/VolumeDataStoreVO.java
+++ b/engine/schema/src/org/apache/cloudstack/storage/datastore/db/VolumeDataStoreVO.java
@@ -102,6 +102,10 @@ public class VolumeDataStoreVO implements StateObject<ObjectInDataStoreStateMach
     @Column(name = "download_url")
     private String extractUrl;
 
+    @Column(name = "download_url_created")
+    @Temporal(value = TemporalType.TIMESTAMP)
+    private Date extractUrlCreated = null;
+
     @Column(name = "destroyed")
     boolean destroyed = false;
 
@@ -369,4 +373,12 @@ public class VolumeDataStoreVO implements StateObject<ObjectInDataStoreStateMach
     public void setExtractUrl(String extractUrl) {
         this.extractUrl = extractUrl;
     }
+
+    public Date getExtractUrlCreated() {
+        return extractUrlCreated;
+    }
+
+    public void setExtractUrlCreated(Date extractUrlCreated) {
+        this.extractUrlCreated = extractUrlCreated;
+    }
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5393387b/engine/storage/image/src/org/apache/cloudstack/storage/image/store/ImageStoreImpl.java
----------------------------------------------------------------------
diff --git a/engine/storage/image/src/org/apache/cloudstack/storage/image/store/ImageStoreImpl.java b/engine/storage/image/src/org/apache/cloudstack/storage/image/store/ImageStoreImpl.java
index 8da7eb7..182a8ec 100644
--- a/engine/storage/image/src/org/apache/cloudstack/storage/image/store/ImageStoreImpl.java
+++ b/engine/storage/image/src/org/apache/cloudstack/storage/image/store/ImageStoreImpl.java
@@ -24,6 +24,7 @@ import java.util.concurrent.ExecutionException;
 
 import javax.inject.Inject;
 
+import com.cloud.storage.Upload;
 import org.apache.log4j.Logger;
 
 import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
@@ -203,4 +204,10 @@ public class ImageStoreImpl implements ImageStoreEntity {
         return driver.createEntityExtractUrl(this, installPath, format, dataObject);
     }
 
+    @Override
+    public void deleteExtractUrl(String installPath, String url, Upload.Type entityType) {
+        driver.deleteEntityExtractUrl(this, installPath, url, entityType);
+    }
+
+
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5393387b/engine/storage/src/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java
----------------------------------------------------------------------
diff --git a/engine/storage/src/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java b/engine/storage/src/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java
index 7ed11ec..25aa8e8 100644
--- a/engine/storage/src/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java
+++ b/engine/storage/src/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java
@@ -25,6 +25,7 @@ import java.util.Map;
 
 import javax.inject.Inject;
 
+import com.cloud.storage.Upload;
 import org.apache.log4j.Logger;
 
 import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult;
@@ -271,4 +272,8 @@ public abstract class BaseImageStoreDriverImpl implements ImageStoreDriver {
     @Override
     public void resize(DataObject data, AsyncCompletionCallback<CreateCmdResult> callback) {
     }
+
+    @Override
+    public void deleteEntityExtractUrl(DataStore store, String installPath, String url, Upload.Type entityType){
+    }
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5393387b/engine/storage/src/org/apache/cloudstack/storage/image/ImageStoreDriver.java
----------------------------------------------------------------------
diff --git a/engine/storage/src/org/apache/cloudstack/storage/image/ImageStoreDriver.java b/engine/storage/src/org/apache/cloudstack/storage/image/ImageStoreDriver.java
index fa7ea37..e71529e 100644
--- a/engine/storage/src/org/apache/cloudstack/storage/image/ImageStoreDriver.java
+++ b/engine/storage/src/org/apache/cloudstack/storage/image/ImageStoreDriver.java
@@ -18,6 +18,7 @@
  */
 package org.apache.cloudstack.storage.image;
 
+import com.cloud.storage.Upload;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreDriver;
@@ -26,4 +27,6 @@ import com.cloud.storage.Storage.ImageFormat;
 
 public interface ImageStoreDriver extends DataStoreDriver {
     String createEntityExtractUrl(DataStore store, String installPath, ImageFormat format, DataObject dataObject);
+
+    void deleteEntityExtractUrl(DataStore store, String installPath, String url, Upload.Type entityType);
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5393387b/engine/storage/src/org/apache/cloudstack/storage/image/db/TemplateDataStoreDaoImpl.java
----------------------------------------------------------------------
diff --git a/engine/storage/src/org/apache/cloudstack/storage/image/db/TemplateDataStoreDaoImpl.java b/engine/storage/src/org/apache/cloudstack/storage/image/db/TemplateDataStoreDaoImpl.java
index cce7f08..50334f4 100644
--- a/engine/storage/src/org/apache/cloudstack/storage/image/db/TemplateDataStoreDaoImpl.java
+++ b/engine/storage/src/org/apache/cloudstack/storage/image/db/TemplateDataStoreDaoImpl.java
@@ -63,6 +63,7 @@ public class TemplateDataStoreDaoImpl extends GenericDaoBase<TemplateDataStoreVO
     private SearchBuilder<TemplateDataStoreVO> storeTemplateSearch;
     private SearchBuilder<TemplateDataStoreVO> storeTemplateStateSearch;
     private SearchBuilder<TemplateDataStoreVO> storeTemplateDownloadStatusSearch;
+    private SearchBuilder<TemplateDataStoreVO> downloadTemplateSearch;
 
     @Inject
     private DataStoreManager _storeMgr;
@@ -131,6 +132,11 @@ public class TemplateDataStoreDaoImpl extends GenericDaoBase<TemplateDataStoreVO
         storeTemplateSearch.and("destroyed", storeTemplateSearch.entity().getDestroyed(), SearchCriteria.Op.EQ);
         storeTemplateSearch.done();
 
+        downloadTemplateSearch = createSearchBuilder();
+        downloadTemplateSearch.and("download_url", downloadTemplateSearch.entity().getExtractUrl(), Op.NNULL);
+        downloadTemplateSearch.and("destroyed", downloadTemplateSearch.entity().getDestroyed(), SearchCriteria.Op.EQ);
+        downloadTemplateSearch.done();
+
         return true;
     }
 
@@ -488,4 +494,11 @@ public class TemplateDataStoreDaoImpl extends GenericDaoBase<TemplateDataStoreVO
 
     }
 
+    @Override
+    public List<TemplateDataStoreVO> listTemplateDownloadUrls() {
+        SearchCriteria<TemplateDataStoreVO> sc = downloadTemplateSearch.create();
+        sc.setParameters("destroyed", false);
+        return listBy(sc);
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5393387b/engine/storage/src/org/apache/cloudstack/storage/image/db/VolumeDataStoreDaoImpl.java
----------------------------------------------------------------------
diff --git a/engine/storage/src/org/apache/cloudstack/storage/image/db/VolumeDataStoreDaoImpl.java b/engine/storage/src/org/apache/cloudstack/storage/image/db/VolumeDataStoreDaoImpl.java
index e906d95..9309f6e 100644
--- a/engine/storage/src/org/apache/cloudstack/storage/image/db/VolumeDataStoreDaoImpl.java
+++ b/engine/storage/src/org/apache/cloudstack/storage/image/db/VolumeDataStoreDaoImpl.java
@@ -50,6 +50,7 @@ public class VolumeDataStoreDaoImpl extends GenericDaoBase<VolumeDataStoreVO, Lo
     private SearchBuilder<VolumeDataStoreVO> storeSearch;
     private SearchBuilder<VolumeDataStoreVO> cacheSearch;
     private SearchBuilder<VolumeDataStoreVO> storeVolumeSearch;
+    private SearchBuilder<VolumeDataStoreVO> downloadVolumeSearch;
 
     @Inject
     DataStoreManager storeMgr;
@@ -85,6 +86,12 @@ public class VolumeDataStoreDaoImpl extends GenericDaoBase<VolumeDataStoreVO, Lo
         updateStateSearch.and("state", updateStateSearch.entity().getState(), Op.EQ);
         updateStateSearch.and("updatedCount", updateStateSearch.entity().getUpdatedCount(), Op.EQ);
         updateStateSearch.done();
+
+        downloadVolumeSearch = createSearchBuilder();
+        downloadVolumeSearch.and("download_url", downloadVolumeSearch.entity().getExtractUrl(), Op.NNULL);
+        downloadVolumeSearch.and("destroyed", downloadVolumeSearch.entity().getDestroyed(), SearchCriteria.Op.EQ);
+        downloadVolumeSearch.done();
+
         return true;
     }
 
@@ -253,4 +260,11 @@ public class VolumeDataStoreDaoImpl extends GenericDaoBase<VolumeDataStoreVO, Lo
         }
 
     }
+
+    @Override
+    public List<VolumeDataStoreVO> listVolumeDownloadUrls() {
+        SearchCriteria<VolumeDataStoreVO> sc = downloadVolumeSearch.create();
+        sc.setParameters("destroyed", false);
+        return listBy(sc);
+    }
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5393387b/plugins/storage/image/default/src/org/apache/cloudstack/storage/datastore/driver/CloudStackImageStoreDriverImpl.java
----------------------------------------------------------------------
diff --git a/plugins/storage/image/default/src/org/apache/cloudstack/storage/datastore/driver/CloudStackImageStoreDriverImpl.java b/plugins/storage/image/default/src/org/apache/cloudstack/storage/datastore/driver/CloudStackImageStoreDriverImpl.java
index c2e26d5..c25e99f 100644
--- a/plugins/storage/image/default/src/org/apache/cloudstack/storage/datastore/driver/CloudStackImageStoreDriverImpl.java
+++ b/plugins/storage/image/default/src/org/apache/cloudstack/storage/datastore/driver/CloudStackImageStoreDriverImpl.java
@@ -22,6 +22,8 @@ import java.util.UUID;
 
 import javax.inject.Inject;
 
+import com.cloud.agent.api.storage.DeleteEntityDownloadURLCommand;
+import com.cloud.storage.Upload;
 import org.apache.log4j.Logger;
 
 import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
@@ -109,4 +111,28 @@ public class CloudStackImageStoreDriverImpl extends BaseImageStoreDriverImpl {
         return scheme + "://" + hostname + "/userdata/" + uuid;
     }
 
+    @Override
+    public void deleteEntityExtractUrl(DataStore store, String installPath, String downloadUrl, Upload.Type entityType) {
+        // find an endpoint to send command
+        EndPoint ep = _epSelector.select(store);
+
+        // Delete Symlink at ssvm. In case of volume also delete the volume.
+        DeleteEntityDownloadURLCommand cmd = new DeleteEntityDownloadURLCommand(installPath, entityType, downloadUrl, ((ImageStoreEntity) store).getMountPoint());
+
+        Answer ans = null;
+        if (ep == null) {
+            String errMsg = "No remote endpoint to send command, check if host or ssvm is down?";
+            s_logger.error(errMsg);
+            ans = new Answer(cmd, false, errMsg);
+        } else {
+            ans = ep.sendMessage(cmd);
+        }
+        if (ans == null || !ans.getResult()) {
+            String errorString = "Unable to delete the url " + downloadUrl + " for path " + installPath + " on ssvm, " + ans.getDetails();
+            s_logger.error(errorString);
+            throw new CloudRuntimeException(errorString);
+        }
+
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5393387b/server/src/com/cloud/configuration/ConfigurationManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/com/cloud/configuration/ConfigurationManagerImpl.java
index 936f4cd..2310de6 100755
--- a/server/src/com/cloud/configuration/ConfigurationManagerImpl.java
+++ b/server/src/com/cloud/configuration/ConfigurationManagerImpl.java
@@ -17,6 +17,7 @@
 package com.cloud.configuration;
 
 import java.net.URI;
+import java.sql.Date;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
@@ -36,6 +37,7 @@ import javax.ejb.Local;
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
+import com.cloud.storage.StorageManager;
 import org.apache.log4j.Logger;
 
 import org.apache.cloudstack.acl.SecurityChecker;
@@ -315,6 +317,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
     AffinityGroupDao _affinityGroupDao;
     @Inject
     AffinityGroupService _affinityGroupService;
+    @Inject
+    StorageManager _storageManager;
 
     // FIXME - why don't we have interface for DataCenterLinkLocalIpAddressDao?
     @Inject
@@ -585,6 +589,27 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
                     throw new CloudRuntimeException("Failed to update SecondaryStorage offering's use_local_storage option to value:" + useLocalStorage);
                 }
             }
+        }else if (Config.SecStorageSecureCopyCert.key().equalsIgnoreCase(name)) {
+            //FIXME - Ideally there should be a listener model to listen to global config changes and be able to take action gracefully.
+            //Expire the download urls
+            String sqlTemplate = "update template_store_ref set download_url_created=?";
+            String sqlVolume = "update volume_store_ref set download_url_created=?";
+            try {
+                // Change for templates
+                pstmt = txn.prepareAutoCloseStatement(sqlTemplate);
+                pstmt.setDate(1, new Date(-1l));// Set the time before the epoch time.
+                pstmt.executeUpdate();
+                // Change for volumes
+                pstmt = txn.prepareAutoCloseStatement(sqlVolume);
+                pstmt.setDate(1, new Date(-1l));// Set the time before the epoch time.
+                pstmt.executeUpdate();
+                // Cleanup the download urls
+                _storageManager.cleanupDownloadUrls();
+            } catch (Throwable e) {
+                throw new CloudRuntimeException("Failed to clean up download URLs in template_store_ref or volume_store_ref due to exception ", e);
+            }
+
+
         }
 
         txn.commit();

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5393387b/server/src/com/cloud/storage/StorageManager.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/storage/StorageManager.java b/server/src/com/cloud/storage/StorageManager.java
index c8a7051..eebbf7f 100755
--- a/server/src/com/cloud/storage/StorageManager.java
+++ b/server/src/com/cloud/storage/StorageManager.java
@@ -114,4 +114,6 @@ public interface StorageManager extends StorageService {
     Long getDiskIopsReadRate(ServiceOfferingVO offering, DiskOfferingVO diskOffering);
 
     Long getDiskIopsWriteRate(ServiceOfferingVO offering, DiskOfferingVO diskOffering);
+
+    void cleanupDownloadUrls();
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5393387b/server/src/com/cloud/storage/StorageManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/storage/StorageManagerImpl.java b/server/src/com/cloud/storage/StorageManagerImpl.java
index 95e037c..3d8b2c1 100755
--- a/server/src/com/cloud/storage/StorageManagerImpl.java
+++ b/server/src/com/cloud/storage/StorageManagerImpl.java
@@ -41,6 +41,8 @@ import javax.ejb.Local;
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
+import com.cloud.utils.DateUtil;
+import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity;
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
 
@@ -284,6 +286,8 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
     boolean _templateCleanupEnabled = true;
     int _storageCleanupInterval;
     int _storagePoolAcquisitionWaitSeconds = 1800; // 30 minutes
+    int _downloadUrlCleanupInterval;
+    int _downloadUrlExpirationInterval;
     // protected BigDecimal _overProvisioningFactor = new BigDecimal(1);
     private long _serverId;
 
@@ -455,6 +459,12 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
         s_logger.info("Storage cleanup enabled: " + _storageCleanupEnabled + ", interval: " + _storageCleanupInterval + ", template cleanup enabled: " +
             _templateCleanupEnabled);
 
+        String cleanupInterval = configs.get("extract.url.cleanup.interval");
+        _downloadUrlCleanupInterval = NumbersUtil.parseInt(cleanupInterval, 7200);
+
+        String urlExpirationInterval = configs.get("extract.url.expiration.interval");
+        _downloadUrlExpirationInterval = NumbersUtil.parseInt(urlExpirationInterval, 14400);
+
         String workers = configs.get("expunge.workers");
         int wrks = NumbersUtil.parseInt(workers, 10);
         _executor = Executors.newScheduledThreadPool(wrks, new NamedThreadFactory("StorageManager-Scavenger"));
@@ -507,6 +517,9 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
         } else {
             s_logger.debug("Storage cleanup is not enabled, so the storage cleanup thread is not being scheduled.");
         }
+
+        _executor.scheduleWithFixedDelay(new DownloadURLGarbageCollector(), _downloadUrlCleanupInterval, _downloadUrlCleanupInterval, TimeUnit.SECONDS);
+
         return true;
     }
 
@@ -1962,6 +1975,80 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
         return true;
     }
 
+    protected class DownloadURLGarbageCollector implements Runnable {
+
+        public DownloadURLGarbageCollector() {
+        }
+
+        @Override
+        public void run() {
+            try {
+                s_logger.trace("Download URL Garbage Collection Thread is running.");
+
+                cleanupDownloadUrls();
+
+            } catch (Exception e) {
+                s_logger.error("Caught the following Exception", e);
+            }
+        }
+    }
+
+    @Override
+    public void cleanupDownloadUrls(){
+
+        // Cleanup expired volume URLs
+        List<VolumeDataStoreVO> volumesOnImageStoreList = _volumeStoreDao.listVolumeDownloadUrls();
+        for(VolumeDataStoreVO volumeOnImageStore : volumesOnImageStoreList){
+
+            try {
+                long downloadUrlCurrentAgeInSecs = DateUtil.getTimeDifference(DateUtil.now(), volumeOnImageStore.getExtractUrlCreated());
+                if(downloadUrlCurrentAgeInSecs < _downloadUrlExpirationInterval){  // URL hasnt expired yet
+                    continue;
+                }
+
+                s_logger.debug("Removing download url " + volumeOnImageStore.getExtractUrl() + " for volume id " + volumeOnImageStore.getVolumeId());
+
+                // Remove it from image store
+                ImageStoreEntity secStore = (ImageStoreEntity) _dataStoreMgr.getDataStore(volumeOnImageStore.getDataStoreId(), DataStoreRole.Image);
+                secStore.deleteExtractUrl(volumeOnImageStore.getInstallPath(), volumeOnImageStore.getExtractUrl(), Upload.Type.VOLUME);
+
+                // Now expunge it from DB since this entry was created only for download purpose
+                _volumeStoreDao.expunge(volumeOnImageStore.getId());
+            }catch(Throwable th){
+                s_logger.warn("Caught exception while deleting download url " +volumeOnImageStore.getExtractUrl() +
+                        " for volume id " + volumeOnImageStore.getVolumeId(), th);
+            }
+        }
+
+        // Cleanup expired template URLs
+        List<TemplateDataStoreVO> templatesOnImageStoreList = _templateStoreDao.listTemplateDownloadUrls();
+        for(TemplateDataStoreVO templateOnImageStore : templatesOnImageStoreList){
+
+            try {
+                long downloadUrlCurrentAgeInSecs = DateUtil.getTimeDifference(DateUtil.now(), templateOnImageStore.getExtractUrlCreated());
+                if(downloadUrlCurrentAgeInSecs < _downloadUrlExpirationInterval){  // URL hasnt expired yet
+                    continue;
+                }
+
+                s_logger.debug("Removing download url " + templateOnImageStore.getExtractUrl() + " for template id " + templateOnImageStore.getTemplateId());
+
+                // Remove it from image store
+                ImageStoreEntity secStore = (ImageStoreEntity) _dataStoreMgr.getDataStore(templateOnImageStore.getDataStoreId(), DataStoreRole.Image);
+                secStore.deleteExtractUrl(templateOnImageStore.getInstallPath(), templateOnImageStore.getExtractUrl(), Upload.Type.TEMPLATE);
+
+                // Now remove download details from DB.
+                templateOnImageStore.setExtractUrl(null);
+                templateOnImageStore.setExtractUrlCreated(null);
+                _templateStoreDao.update(templateOnImageStore.getId(), templateOnImageStore);
+            }catch(Throwable th){
+                s_logger.warn("caught exception while deleting download url " +templateOnImageStore.getExtractUrl() +
+                        " for template id " +templateOnImageStore.getTemplateId(), th);
+            }
+        }
+
+
+    }
+
     // get bytesReadRate from service_offering, disk_offering and vm.disk.throttling.bytes_read_rate
     @Override
     public Long getDiskBytesReadRate(ServiceOfferingVO offering, DiskOfferingVO diskOffering) {

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5393387b/server/src/com/cloud/storage/VolumeApiServiceImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/com/cloud/storage/VolumeApiServiceImpl.java
index 150daec..9b034e9 100644
--- a/server/src/com/cloud/storage/VolumeApiServiceImpl.java
+++ b/server/src/com/cloud/storage/VolumeApiServiceImpl.java
@@ -26,6 +26,7 @@ import java.util.concurrent.ExecutionException;
 
 import javax.inject.Inject;
 
+import com.cloud.utils.DateUtil;
 import org.apache.log4j.Logger;
 
 import org.apache.cloudstack.api.command.user.volume.AttachVolumeCmd;
@@ -1910,6 +1911,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
         String extractUrl = secStore.createEntityExtractUrl(vol.getPath(), vol.getFormat(), vol);
         volumeStoreRef = _volumeStoreDao.findByVolume(volumeId);
         volumeStoreRef.setExtractUrl(extractUrl);
+        volumeStoreRef.setExtractUrlCreated(DateUtil.now());
         _volumeStoreDao.update(volumeStoreRef.getId(), volumeStoreRef);
 
         return extractUrl;

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5393387b/server/src/com/cloud/template/TemplateManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/template/TemplateManagerImpl.java b/server/src/com/cloud/template/TemplateManagerImpl.java
index 33a6df8..51d09ef 100755
--- a/server/src/com/cloud/template/TemplateManagerImpl.java
+++ b/server/src/com/cloud/template/TemplateManagerImpl.java
@@ -31,6 +31,7 @@ import javax.ejb.Local;
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
+import com.cloud.utils.DateUtil;
 import org.apache.log4j.Logger;
 
 import org.apache.cloudstack.acl.SecurityChecker.AccessType;
@@ -437,12 +438,20 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager,
             throw new InvalidParameterValueException("The " + desc + " has not been downloaded ");
         }
 
+        // Check if the url already exists
+        if(tmpltStoreRef.getExtractUrl() != null){
+            return tmpltStoreRef.getExtractUrl();
+        }
+
         // Handle NFS to S3 object store migration case, we trigger template sync from NFS to S3 during extract template or copy template
         _tmpltSvr.syncTemplateToRegionStore(templateId, tmpltStore);
 
         TemplateInfo templateObject = _tmplFactory.getTemplate(templateId, tmpltStore);
-
-        return tmpltStore.createEntityExtractUrl(templateObject.getInstallPath(), template.getFormat(), templateObject);
+        String extractUrl = tmpltStore.createEntityExtractUrl(tmpltStoreRef.getInstallPath(), template.getFormat(), templateObject);
+        tmpltStoreRef.setExtractUrl(extractUrl);
+        tmpltStoreRef.setExtractUrlCreated(DateUtil.now());
+        _tmplStoreDao.update(tmpltStoreRef.getId(), tmpltStoreRef);
+        return extractUrl;
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5393387b/server/test/org/apache/cloudstack/networkoffering/ChildTestConfiguration.java
----------------------------------------------------------------------
diff --git a/server/test/org/apache/cloudstack/networkoffering/ChildTestConfiguration.java b/server/test/org/apache/cloudstack/networkoffering/ChildTestConfiguration.java
index 22516c0..4760144 100644
--- a/server/test/org/apache/cloudstack/networkoffering/ChildTestConfiguration.java
+++ b/server/test/org/apache/cloudstack/networkoffering/ChildTestConfiguration.java
@@ -19,6 +19,7 @@ package org.apache.cloudstack.networkoffering;
 
 import java.io.IOException;
 
+import com.cloud.storage.StorageManager;
 import org.mockito.Mockito;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.ComponentScan;
@@ -132,7 +133,8 @@ import com.cloud.vm.dao.VMInstanceDaoImpl;
     PortableIpRangeDaoImpl.class, RegionDaoImpl.class, PortableIpDaoImpl.class, AccountGuestVlanMapDaoImpl.class},
                includeFilters = {@Filter(value = ChildTestConfiguration.Library.class, type = FilterType.CUSTOM)},
                useDefaultFilters = false)
-public class ChildTestConfiguration {
+public class
+        ChildTestConfiguration {
 
     @Bean
     public ManagementService managementService() {
@@ -329,6 +331,11 @@ public class ChildTestConfiguration {
         return Mockito.mock(AffinityGroupService.class);
     }
 
+    @Bean
+    public StorageManager storageManager() {
+        return Mockito.mock(StorageManager.class);
+    }
+
     public static class Library implements TypeFilter {
 
         @Override

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5393387b/services/secondary-storage/server/src/org/apache/cloudstack/storage/template/UploadManager.java
----------------------------------------------------------------------
diff --git a/services/secondary-storage/server/src/org/apache/cloudstack/storage/template/UploadManager.java b/services/secondary-storage/server/src/org/apache/cloudstack/storage/template/UploadManager.java
index be99fea..81168b8 100755
--- a/services/secondary-storage/server/src/org/apache/cloudstack/storage/template/UploadManager.java
+++ b/services/secondary-storage/server/src/org/apache/cloudstack/storage/template/UploadManager.java
@@ -16,11 +16,11 @@
 // under the License.
 package org.apache.cloudstack.storage.template;
 
+import com.cloud.agent.api.Answer;
 import org.apache.cloudstack.storage.resource.SecondaryStorageResource;
 
 import com.cloud.agent.api.storage.CreateEntityDownloadURLAnswer;
 import com.cloud.agent.api.storage.CreateEntityDownloadURLCommand;
-import com.cloud.agent.api.storage.DeleteEntityDownloadURLAnswer;
 import com.cloud.agent.api.storage.DeleteEntityDownloadURLCommand;
 import com.cloud.agent.api.storage.UploadAnswer;
 import com.cloud.agent.api.storage.UploadCommand;
@@ -77,6 +77,6 @@ public interface UploadManager extends Manager {
 
     CreateEntityDownloadURLAnswer handleCreateEntityURLCommand(CreateEntityDownloadURLCommand cmd);
 
-    DeleteEntityDownloadURLAnswer handleDeleteEntityDownloadURLCommand(DeleteEntityDownloadURLCommand cmd);
+    Answer handleDeleteEntityDownloadURLCommand(DeleteEntityDownloadURLCommand cmd);
 
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5393387b/services/secondary-storage/server/src/org/apache/cloudstack/storage/template/UploadManagerImpl.java
----------------------------------------------------------------------
diff --git a/services/secondary-storage/server/src/org/apache/cloudstack/storage/template/UploadManagerImpl.java b/services/secondary-storage/server/src/org/apache/cloudstack/storage/template/UploadManagerImpl.java
index cdbc52d..d046eb9 100755
--- a/services/secondary-storage/server/src/org/apache/cloudstack/storage/template/UploadManagerImpl.java
+++ b/services/secondary-storage/server/src/org/apache/cloudstack/storage/template/UploadManagerImpl.java
@@ -29,13 +29,13 @@ import java.util.concurrent.Executors;
 
 import javax.naming.ConfigurationException;
 
+import com.cloud.agent.api.Answer;
 import org.apache.log4j.Logger;
 
 import org.apache.cloudstack.storage.resource.SecondaryStorageResource;
 
 import com.cloud.agent.api.storage.CreateEntityDownloadURLAnswer;
 import com.cloud.agent.api.storage.CreateEntityDownloadURLCommand;
-import com.cloud.agent.api.storage.DeleteEntityDownloadURLAnswer;
 import com.cloud.agent.api.storage.DeleteEntityDownloadURLCommand;
 import com.cloud.agent.api.storage.UploadAnswer;
 import com.cloud.agent.api.storage.UploadCommand;
@@ -303,7 +303,7 @@ public class UploadManagerImpl extends ManagerBase implements UploadManager {
     }
 
     @Override
-    public DeleteEntityDownloadURLAnswer handleDeleteEntityDownloadURLCommand(DeleteEntityDownloadURLCommand cmd) {
+    public Answer handleDeleteEntityDownloadURLCommand(DeleteEntityDownloadURLCommand cmd) {
 
         //Delete the soft link. Example path = volumes/8/74eeb2c6-8ab1-4357-841f-2e9d06d1f360.vhd
         s_logger.warn("handleDeleteEntityDownloadURLCommand Path:" + cmd.getPath() + " Type:" + cmd.getType().toString());
@@ -318,24 +318,24 @@ public class UploadManagerImpl extends ManagerBase implements UploadManager {
         if (result != null) {
             String errorString = "Error in deleting =" + result;
             s_logger.warn(errorString);
-            return new DeleteEntityDownloadURLAnswer(errorString, CreateEntityDownloadURLAnswer.RESULT_FAILURE);
+            return new Answer(cmd, false, errorString);
         }
 
         // If its a volume also delete the Hard link since it was created only for the purpose of download.
         if (cmd.getType() == Upload.Type.VOLUME) {
             command = new Script("/bin/bash", s_logger);
             command.add("-c");
-            command.add("rm -f /mnt/SecStorage/" + cmd.getParentPath() + File.separator + path);
+            command.add("rm -rf /mnt/SecStorage/" + cmd.getParentPath() + File.separator + path);
             s_logger.warn(" " + parentDir + File.separator + path);
             result = command.execute();
             if (result != null) {
                 String errorString = "Error in linking  err=" + result;
                 s_logger.warn(errorString);
-                return new DeleteEntityDownloadURLAnswer(errorString, CreateEntityDownloadURLAnswer.RESULT_FAILURE);
+                return new Answer(cmd, false, errorString);
             }
         }
 
-        return new DeleteEntityDownloadURLAnswer("", CreateEntityDownloadURLAnswer.RESULT_SUCCESS);
+        return new Answer(cmd, true, "");
     }
 
     private String getInstallPath(String jobId) {

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5393387b/setup/db/db/schema-430to440.sql
----------------------------------------------------------------------
diff --git a/setup/db/db/schema-430to440.sql b/setup/db/db/schema-430to440.sql
index 7cd01d7..1d0a79a 100644
--- a/setup/db/db/schema-430to440.sql
+++ b/setup/db/db/schema-430to440.sql
@@ -1685,3 +1685,8 @@ alter table `cloud`.`vlan` add column created datetime NULL COMMENT 'date create
 
 alter table `cloud`.`user_ip_address` drop key public_ip_address;
 alter table `cloud`.`user_ip_address` add UNIQUE KEY public_ip_address (public_ip_address,source_network_id, removed);
+
+ALTER TABLE `cloud`.`volume_store_ref` ADD `download_url_created` datetime;
+ALTER TABLE `cloud`.`template_store_ref` ADD `download_url_created` datetime;
+ALTER TABLE `cloud`.`template_store_ref` ADD `download_url` varchar(255);
+

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5393387b/utils/src/com/cloud/utils/DateUtil.java
----------------------------------------------------------------------
diff --git a/utils/src/com/cloud/utils/DateUtil.java b/utils/src/com/cloud/utils/DateUtil.java
index fc7fd01..be18fa2 100644
--- a/utils/src/com/cloud/utils/DateUtil.java
+++ b/utils/src/com/cloud/utils/DateUtil.java
@@ -261,6 +261,17 @@ public class DateUtil {
         }
     }
 
+    public static long getTimeDifference(Date date1, Date date2){
+
+        Calendar dateCalendar1 = Calendar.getInstance();
+        dateCalendar1.setTime(date1);
+        Calendar dateCalendar2 = Calendar.getInstance();
+        dateCalendar2.setTime(date2);
+
+        return (dateCalendar1.getTimeInMillis() - dateCalendar2.getTimeInMillis() )/1000;
+
+    }
+
     // test only
     public static void main(String[] args) {
         TimeZone localTimezone = Calendar.getInstance().getTimeZone();