CLOUDSTACK-10054:Volume download times out in 3600 seconds (apache#2244)

mrunalinikankariya · DaanHoogland · commit bb607d07a974 · 2018-02-27T13:33:00.000+01:00
diff --git a/core/src/main/java/com/cloud/storage/template/IsoProcessor.java b/core/src/main/java/com/cloud/storage/template/IsoProcessor.java
@@ -37,6 +37,11 @@ public class IsoProcessor extends AdapterBase implements Processor {
 
     @Override
     public FormatInfo process(String templatePath, ImageFormat format, String templateName) {
+      return process(templatePath, format, templateName, 0);
+    }
+
+   @Override
+    public FormatInfo process(String templatePath, ImageFormat format, String templateName, long processTimeout) {
         if (format != null) {
             s_logger.debug("We don't handle conversion from " + format + " to ISO.");
             return null;
diff --git a/core/src/main/java/com/cloud/storage/template/OVAProcessor.java b/core/src/main/java/com/cloud/storage/template/OVAProcessor.java
@@ -42,11 +42,15 @@
 
 public class OVAProcessor extends AdapterBase implements Processor {
     private static final Logger s_logger = Logger.getLogger(OVAProcessor.class);
-
     StorageLayer _storage;
 
     @Override
     public FormatInfo process(String templatePath, ImageFormat format, String templateName) throws InternalErrorException {
+        return process(templatePath, format, templateName, 0);
+    }
+
+    @Override
+    public FormatInfo process(String templatePath, ImageFormat format, String templateName, long processTimeout) throws InternalErrorException {
         if (format != null) {
             if (s_logger.isInfoEnabled()) {
                 s_logger.info("We currently don't handle conversion from " + format + " to OVA.");
@@ -66,8 +70,7 @@ public FormatInfo process(String templatePath, ImageFormat format, String templa
         s_logger.info("Template processing - untar OVA package. templatePath: " + templatePath + ", templateName: " + templateName);
         String templateFileFullPath = templatePath + File.separator + templateName + "." + ImageFormat.OVA.getFileExtension();
         File templateFile = new File(templateFileFullPath);
-
-        Script command = new Script("tar", 0, s_logger);
+        Script command = new Script("tar", processTimeout, s_logger);
         command.add("--no-same-owner");
         command.add("--no-same-permissions");
         command.add("-xf", templateFileFullPath);
diff --git a/core/src/main/java/com/cloud/storage/template/Processor.java b/core/src/main/java/com/cloud/storage/template/Processor.java
@@ -44,6 +44,8 @@ public interface Processor extends Adapter {
      */
     FormatInfo process(String templatePath, ImageFormat format, String templateName) throws InternalErrorException;
 
+    FormatInfo process(String templatePath, ImageFormat format, String templateName, long processTimeout) throws InternalErrorException;
+
     public static class FormatInfo {
         public ImageFormat format;
         public long size;
diff --git a/core/src/main/java/com/cloud/storage/template/QCOW2Processor.java b/core/src/main/java/com/cloud/storage/template/QCOW2Processor.java
@@ -40,8 +40,13 @@ public class QCOW2Processor extends AdapterBase implements Processor {
 
     private StorageLayer _storage;
 
+   @Override
+   public FormatInfo process(String templatePath, ImageFormat format, String templateName) throws InternalErrorException  {
+     return process(templatePath, format, templateName, 0);
+   }
+
     @Override
-    public FormatInfo process(String templatePath, ImageFormat format, String templateName) throws InternalErrorException {
+    public FormatInfo process(String templatePath, ImageFormat format, String templateName, long processTimeout) throws InternalErrorException {
         if (format != null) {
             s_logger.debug("We currently don't handle conversion from " + format + " to QCOW2.");
             return null;
diff --git a/core/src/main/java/com/cloud/storage/template/RawImageProcessor.java b/core/src/main/java/com/cloud/storage/template/RawImageProcessor.java
@@ -45,8 +45,13 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
         return true;
     }
 
+   @Override
+   public FormatInfo process(String templatePath, ImageFormat format, String templateName) throws InternalErrorException {
+      return process(templatePath, format, templateName, 0);
+   }
+
     @Override
-    public FormatInfo process(String templatePath, ImageFormat format, String templateName) throws InternalErrorException {
+    public FormatInfo process(String templatePath, ImageFormat format, String templateName, long processTimeout) throws InternalErrorException {
         if (format != null) {
             s_logger.debug("We currently don't handle conversion from " + format + " to raw image.");
             return null;
diff --git a/core/src/main/java/com/cloud/storage/template/TARProcessor.java b/core/src/main/java/com/cloud/storage/template/TARProcessor.java
@@ -35,6 +35,11 @@ public class TARProcessor extends AdapterBase implements Processor {
 
     @Override
     public FormatInfo process(String templatePath, ImageFormat format, String templateName) {
+       return process(templatePath, format, templateName, 0);
+    }
+
+    @Override
+    public FormatInfo process(String templatePath, ImageFormat format, String templateName, long processTimeout) {
         if (format != null) {
             s_logger.debug("We currently don't handle conversion from " + format + " to TAR.");
             return null;
diff --git a/core/src/main/java/com/cloud/storage/template/VhdProcessor.java b/core/src/main/java/com/cloud/storage/template/VhdProcessor.java
@@ -58,6 +58,11 @@ public class VhdProcessor extends AdapterBase implements Processor {
 
     @Override
     public FormatInfo process(String templatePath, ImageFormat format, String templateName) throws InternalErrorException {
+     return process(templatePath, format, templateName, 0);
+    }
+
+    @Override
+    public FormatInfo process(String templatePath, ImageFormat format, String templateName, long processTimeout) throws InternalErrorException {
         if (format != null) {
             s_logger.debug("We currently don't handle conversion from " + format + " to VHD.");
             return null;
diff --git a/core/src/main/java/com/cloud/storage/template/VmdkProcessor.java b/core/src/main/java/com/cloud/storage/template/VmdkProcessor.java
@@ -44,6 +44,11 @@ public class VmdkProcessor extends AdapterBase implements Processor {
 
     @Override
     public FormatInfo process(String templatePath, ImageFormat format, String templateName) throws InternalErrorException {
+     return process(templatePath, format, templateName, 0);
+    }
+
+    @Override
+    public FormatInfo process(String templatePath, ImageFormat format, String templateName, long processTimeout) throws InternalErrorException {
         if (format != null) {
             if (s_logger.isInfoEnabled()) {
                 s_logger.info("We currently don't handle conversion from " + format + " to VMDK.");
diff --git a/core/src/main/java/org/apache/cloudstack/storage/command/TemplateOrVolumePostUploadCommand.java b/core/src/main/java/org/apache/cloudstack/storage/command/TemplateOrVolumePostUploadCommand.java
@@ -51,6 +51,8 @@ public class TemplateOrVolumePostUploadCommand {
 
     private String defaultMaxAccountSecondaryStorage;
 
+    private long processTimeout;
+
     private long accountId;
 
     private Integer nfsVersion;
@@ -206,4 +208,12 @@ public Integer getNfsVersion() {
     public void setNfsVersion(Integer nfsVersion) {
         this.nfsVersion = nfsVersion;
     }
+
+    public void setProcessTimeout(long processTimeout) {
+        this.processTimeout = processTimeout;
+    }
+
+    public long getProcessTimeout() {
+        return processTimeout;
+    }
 }
diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java
@@ -32,6 +32,7 @@
 import org.apache.cloudstack.framework.config.ConfigKey;
 import org.apache.cloudstack.framework.config.Configurable;
 import org.apache.cloudstack.storage.command.CopyCommand;
+import org.apache.cloudstack.storage.command.DownloadCommand;
 import org.apache.cloudstack.storage.command.DeleteCommand;
 import org.apache.cloudstack.storage.command.StorageSubSystemCommand;
 import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
@@ -434,13 +435,14 @@ public int compare(NicTO arg0, NicTO arg1) {
     @DB
     public Pair<Boolean, Long> getCommandHostDelegation(long hostId, Command cmd) {
         boolean needDelegation = false;
-
         if (cmd instanceof StorageSubSystemCommand) {
             Boolean fullCloneEnabled = VmwareFullClone.value();
             StorageSubSystemCommand c = (StorageSubSystemCommand)cmd;
             c.setExecuteInSequence(fullCloneEnabled);
         }
-
+        if (cmd instanceof DownloadCommand) {
+          cmd.setContextParam(VmwareManager.s_vmwareOVAPackageTimeout.key(), String.valueOf(VmwareManager.s_vmwareOVAPackageTimeout.value()));
+        }
         //NOTE: the hostid can be a hypervisor host, or a ssvm agent. For copycommand, if it's for volume upload, the hypervisor
         //type is empty, so we need to check the format of volume at first.
         if (cmd instanceof CopyCommand) {
@@ -514,11 +516,11 @@ public Pair<Boolean, Long> getCommandHostDelegation(long hostId, Command cmd) {
             cmd.setContextParam("execid", String.valueOf(execLog.getId()));
             cmd.setContextParam("noderuninfo", String.format("%d-%d", _clusterMgr.getManagementNodeId(), _clusterMgr.getCurrentRunId()));
             cmd.setContextParam("vCenterSessionTimeout", String.valueOf(_vmwareMgr.getVcenterSessionTimeout()));
+            cmd.setContextParam(VmwareManager.s_vmwareOVAPackageTimeout.key(), String.valueOf(VmwareManager.s_vmwareOVAPackageTimeout.value()));
 
             if (cmd instanceof BackupSnapshotCommand || cmd instanceof CreatePrivateTemplateFromVolumeCommand ||
                 cmd instanceof CreatePrivateTemplateFromSnapshotCommand || cmd instanceof CopyVolumeCommand || cmd instanceof CopyCommand ||
                 cmd instanceof CreateVolumeOVACommand || cmd instanceof PrepareOVAPackingCommand || cmd instanceof CreateVolumeFromSnapshotCommand) {
-
                 String workerName = _vmwareMgr.composeWorkerName();
                 long checkPointId = 1;
                 // FIXME: Fix                    long checkPointId = _checkPointMgr.pushCheckPoint(new VmwareCleanupMaid(hostDetails.get("guid"), workerName));
diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManager.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManager.java
@@ -42,6 +42,9 @@ public interface VmwareManager {
     static final ConfigKey<String> s_vmwareSearchExcludeFolder = new ConfigKey<String>("Advanced", String.class, "vmware.search.exclude.folders", null,
             "Comma seperated list of Datastore Folders to exclude from VMWare search", true, ConfigKey.Scope.Global);
 
+    static final ConfigKey<Integer> s_vmwareOVAPackageTimeout = new ConfigKey<Integer>(Integer.class, "vmware.package.ova.timeout", "Advanced", "3600",
+            "Vmware script timeout for ova packaging process", true, ConfigKey.Scope.Global, 1000);
+
     String composeWorkerName();
 
     String getSystemVMIsoFileNameOnDatastore();
diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java
@@ -136,6 +136,7 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw
     private static final Logger s_logger = Logger.getLogger(VmwareManagerImpl.class);
 
     private static final long SECONDS_PER_MINUTE = 60;
+
     private int _timeout;
 
     private String _instance;
@@ -204,7 +205,6 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw
     private int _additionalPortRangeSize;
     private int _routerExtraPublicNics = 2;
     private int _vCenterSessionTimeout = 1200000; // Timeout in milliseconds
-
     private String _rootDiskController = DiskControllerType.ide.toString();
 
     private final String _dataDiskController = DiskControllerType.osdefault.toString();
@@ -229,9 +229,8 @@ public String getConfigComponentName() {
 
     @Override
     public ConfigKey<?>[] getConfigKeys() {
-        return new ConfigKey<?>[] {s_vmwareNicHotplugWaitTimeout, s_vmwareCleanOldWorderVMs, templateCleanupInterval, s_vmwareSearchExcludeFolder};
+        return new ConfigKey<?>[] {s_vmwareNicHotplugWaitTimeout, s_vmwareCleanOldWorderVMs, templateCleanupInterval, s_vmwareSearchExcludeFolder, s_vmwareOVAPackageTimeout};
     }
-
     @Override
     public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
         s_logger.info("Configure VmwareManagerImpl, manager name: " + name);
diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManager.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManager.java
@@ -51,7 +51,7 @@ public interface VmwareStorageManager {
 
     boolean execute(VmwareHostService hostService, CreateEntityDownloadURLCommand cmd);
 
-    public void createOva(String path, String name);
+    public void createOva(String path, String name, int archiveTimeout);
 
-    public String createOvaForTemplate(TemplateObjectTO template);
+    public String createOvaForTemplate(TemplateObjectTO template, int archiveTimeout);
 }
diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java
@@ -98,28 +98,32 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
     @Override
     public boolean execute(VmwareHostService hostService, CreateEntityDownloadURLCommand cmd) {
         DataTO data = cmd.getData();
+        int timeout = NumbersUtil.parseInt(cmd.getContextParam(VmwareManager.s_vmwareOVAPackageTimeout.key()),
+                    Integer.valueOf(VmwareManager.s_vmwareOVAPackageTimeout.defaultValue()) * VmwareManager.s_vmwareOVAPackageTimeout.multiplier());
         if (data == null) {
             return false;
         }
 
         String newPath = null;
         if (data.getObjectType() == DataObjectType.VOLUME) {
-            newPath = createOvaForVolume((VolumeObjectTO)data);
+            newPath = createOvaForVolume((VolumeObjectTO)data, timeout);
         } else if (data.getObjectType() == DataObjectType.TEMPLATE) {
-            newPath = createOvaForTemplate((TemplateObjectTO)data);
+            newPath = createOvaForTemplate((TemplateObjectTO)data, timeout);
         }
         if (newPath != null) {
             cmd.setInstallPath(newPath);
+            return true;
         }
-        return true;
+        return false;
+
     }
 
     @Override
-    public void createOva(String path, String name) {
+    public void createOva(String path, String name, int archiveTimeout) {
         Script commandSync = new Script(true, "sync", 0, s_logger);
         commandSync.execute();
 
-        Script command = new Script(false, "tar", 0, s_logger);
+        Script command = new Script(false, "tar", archiveTimeout, s_logger);
         command.setWorkDir(path);
         command.add("-cf", name + ".ova");
         command.add(name + ".ovf");        // OVF file should be the first file in OVA archive
@@ -155,7 +159,7 @@ public void configure(Map<String, Object> params) {
     }
 
     @Override
-    public String createOvaForTemplate(TemplateObjectTO template) {
+    public String createOvaForTemplate(TemplateObjectTO template, int archiveTimeout) {
         DataStoreTO storeTO = template.getDataStore();
         if (!(storeTO instanceof NfsTO)) {
             s_logger.debug("Can only handle NFS storage, while creating OVA from template");
@@ -173,7 +177,7 @@ public String createOvaForTemplate(TemplateObjectTO template) {
                     s_logger.debug("OVA file found at: " + installFullPath);
                 } else {
                     if (new File(installFullPath + ".meta").exists()) {
-                        createOVAFromMetafile(installFullPath + ".meta");
+                        createOVAFromMetafile(installFullPath + ".meta", archiveTimeout);
                     } else {
                         String msg = "Unable to find OVA or OVA MetaFile to prepare template.";
                         s_logger.error(msg);
@@ -190,7 +194,7 @@ public String createOvaForTemplate(TemplateObjectTO template) {
 
     //Fang: new command added;
     // Important! we need to sync file system before we can safely use tar to work around a linux kernal bug(or feature)
-    public String createOvaForVolume(VolumeObjectTO volume) {
+    public String createOvaForVolume(VolumeObjectTO volume, int archiveTimeout) {
         DataStoreTO storeTO = volume.getDataStore();
         if (!(storeTO instanceof NfsTO)) {
             s_logger.debug("can only handle nfs storage, when create ova from volume");
@@ -215,15 +219,17 @@ public String createOvaForVolume(VolumeObjectTO volume) {
             } else {
                 Script commandSync = new Script(true, "sync", 0, s_logger);
                 commandSync.execute();
-
-                Script command = new Script(false, "tar", 0, s_logger);
+                Script command = new Script(false, "tar", archiveTimeout, s_logger);
                 command.setWorkDir(installFullPath);
                 command.add("-cf", volumeUuid + ".ova");
                 command.add(volumeUuid + ".ovf");        // OVF file should be the first file in OVA archive
                 command.add(volumeUuid + "-disk0.vmdk");
 
-                command.execute();
-                return volumePath;
+                String result = command.execute();
+                if (result != Script.ERR_TIMEOUT) {
+                    return volumePath;
+                }
+
             }
         } catch (Throwable e) {
             s_logger.info("Exception for createVolumeOVA");
@@ -1046,7 +1052,7 @@ private Pair<String, String> copyVolumeFromSecStorage(VmwareHypervisorHost hyper
 
     // here we use a method to return the ovf and vmdk file names; Another way to do it:
     // create a new class, and like TemplateLocation.java and create templateOvfInfo.java to handle it;
-    private String createOVAFromMetafile(String metafileName) throws Exception {
+    private String createOVAFromMetafile(String metafileName, int archiveTimeout) throws Exception {
         File ova_metafile = new File(metafileName);
         Properties props = null;
         String ovaFileName = "";
@@ -1080,7 +1086,7 @@ private String createOVAFromMetafile(String metafileName) throws Exception {
             s_logger.info("ova: " + ovaFileName + ", ovf:" + ovfFileName + ", vmdk:" + disks[0] + ".");
             Script commandSync = new Script(true, "sync", 0, s_logger);
             commandSync.execute();
-            Script command = new Script(false, "tar", 0, s_logger);
+            Script command = new Script(false, "tar", archiveTimeout, s_logger);
             command.setWorkDir(exportDir); // Fang: pass this in to the method?
             command.add("-cf", ovaFileName);
             command.add(ovfFileName); // OVF file should be the first file in OVA archive
diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareSecondaryStorageResourceHandler.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareSecondaryStorageResourceHandler.java
@@ -145,8 +145,11 @@ public Answer executeRequest(Command cmd) {
     }
 
     protected Answer execute(CreateEntityDownloadURLCommand cmd) {
-        _storageMgr.execute(this, cmd);
-        return _resource.defaultAction(cmd);
+        boolean success = _storageMgr.execute(this, cmd);
+        if (success) {
+            return _resource.defaultAction(cmd);
+        }
+        return new Answer(cmd, false, "Failed to download");
     }
 
     private Answer execute(PrimaryStorageDownloadCommand cmd) {
diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageSubsystemCommandHandler.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageSubsystemCommandHandler.java
diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
diff --git a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/HttpUploadServerHandler.java b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/HttpUploadServerHandler.java
diff --git a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
diff --git a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/DownloadManagerImpl.java b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/DownloadManagerImpl.java
diff --git a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/UploadEntity.java b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/UploadEntity.java
diff --git a/utils/src/main/java/com/cloud/utils/script/Script.java b/utils/src/main/java/com/cloud/utils/script/Script.java
