vmware: Fix template upload from local (#4555)

nvazquez · DaanHoogland · web-flow · commit 4617be458387 · 2020-12-23T15:13:39.000+05:30
Update the guest OS from the OVF file after upload is completed
This PR fixes the template upload from local on VMware

Co-authored-by: dahn &lt;daan.hoogland@gmail.com&gt;
Co-authored-by: dahn &lt;daan.hoogland@gmail.com&gt;
diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/template/GetUploadParamsForTemplateCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/template/GetUploadParamsForTemplateCmd.java
@@ -22,6 +22,7 @@
 import java.util.Collection;
 import java.util.Map;
 
+import com.cloud.hypervisor.Hypervisor;
 import org.apache.cloudstack.acl.RoleType;
 import org.apache.cloudstack.api.APICommand;
 import org.apache.cloudstack.api.AbstractGetUploadParamsCmd;
@@ -51,7 +52,8 @@ public class GetUploadParamsForTemplateCmd extends AbstractGetUploadParamsCmd {
     @Parameter(name = ApiConstants.HYPERVISOR, type = CommandType.STRING, required = true, description = "the target hypervisor for the template")
     private String hypervisor;
 
-    @Parameter(name = ApiConstants.OS_TYPE_ID, type = CommandType.UUID, entityType = GuestOSResponse.class, required = true, description = "the ID of the OS Type that best represents the OS of this template.")
+    @Parameter(name = ApiConstants.OS_TYPE_ID, type = CommandType.UUID, entityType = GuestOSResponse.class, required = false,
+            description = "the ID of the OS Type that best represents the OS of this template. Not required for VMware as the guest OS is obtained from the OVF file.")
     private Long osTypeId;
 
     @Parameter(name = ApiConstants.BITS, type = CommandType.INTEGER, description = "32 or 64 bits support. 64 by default")
@@ -168,6 +170,12 @@ private void validateRequest() {
         if (getZoneId() <= 0) {
             throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "invalid zoneid");
         }
+        if (!hypervisor.equalsIgnoreCase(Hypervisor.HypervisorType.VMware.toString()) && osTypeId == null) {
+            throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Missing parameter ostypeid");
+        }
+        if (hypervisor.equalsIgnoreCase(Hypervisor.HypervisorType.VMware.toString()) && osTypeId != null) {
+            throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Invalid parameter ostypeid, not applicable for VMware");
+        }
     }
 
     @Override
diff --git a/core/src/main/java/org/apache/cloudstack/storage/command/UploadStatusAnswer.java b/core/src/main/java/org/apache/cloudstack/storage/command/UploadStatusAnswer.java
@@ -20,6 +20,7 @@
 package org.apache.cloudstack.storage.command;
 
 import com.cloud.agent.api.Answer;
+import com.cloud.utils.Pair;
 
 public class UploadStatusAnswer extends Answer {
     public static enum UploadStatus {
@@ -31,6 +32,8 @@ public static enum UploadStatus {
     private long physicalSize = 0;
     private String installPath = null;
     private int downloadPercent = 0;
+    private Pair<String, String> guestOsInfo;
+    private String minimumHardwareVersion;
 
     protected UploadStatusAnswer() {
     }
@@ -85,4 +88,20 @@ public int getDownloadPercent() {
     public void setDownloadPercent(int downloadPercent) {
         this.downloadPercent = downloadPercent;
     }
+
+    public Pair<String, String> getGuestOsInfo() {
+        return guestOsInfo;
+    }
+
+    public void setGuestOsInfo(Pair<String, String> guestOsInfo) {
+        this.guestOsInfo = guestOsInfo;
+    }
+
+    public void setMinimumHardwareVersion(String minimumHardwareVersion) {
+        this.minimumHardwareVersion = minimumHardwareVersion;
+    }
+
+    public String getMinimumHardwareVersion() {
+        return minimumHardwareVersion;
+    }
 }
diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/deployasis/DeployAsIsHelperImpl.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/image/deployasis/DeployAsIsHelperImpl.java
@@ -19,9 +19,6 @@
 package org.apache.cloudstack.storage.image.deployasis;
 
 import com.cloud.agent.api.storage.DownloadAnswer;
-import com.cloud.agent.api.to.DataStoreTO;
-import com.cloud.agent.api.to.DataTO;
-import com.cloud.agent.api.to.DiskTO;
 import com.cloud.agent.api.to.NicTO;
 import com.cloud.agent.api.to.OVFInformationTO;
 import com.cloud.agent.api.to.deployasis.OVFConfigurationTO;
@@ -39,10 +36,8 @@
 import com.cloud.storage.GuestOSCategoryVO;
 import com.cloud.storage.GuestOSHypervisorVO;
 import com.cloud.storage.GuestOSVO;
-import com.cloud.storage.VMTemplateStoragePoolVO;
 import com.cloud.storage.VMTemplateStorageResourceAssoc;
 import com.cloud.storage.VMTemplateVO;
-import com.cloud.storage.Volume;
 import com.cloud.storage.dao.GuestOSCategoryDao;
 import com.cloud.storage.dao.GuestOSDao;
 import com.cloud.storage.dao.GuestOSHypervisorDao;
@@ -57,7 +52,6 @@
 import com.google.gson.GsonBuilder;
 import com.cloud.agent.api.to.deployasis.OVFNetworkTO;
 import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
 import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
 import org.apache.commons.collections.CollectionUtils;
@@ -155,17 +149,16 @@ public boolean persistTemplateDeployAsIsDetails(long templateId, DownloadAnswer
     }
 
     /**
-     * Handle the guest OS read from the OVF and try to match it to an existing guest OS in DB.
-     * If the guest OS cannot be mapped to an existing guest OS in DB, then create it and create support for hypervisor versions.
-     * Roll back actions in case of unexpected erros
+     * Returns the mapped guest OS from the OVF file of the template to the CloudStack database OS ID
      */
-    private void handleGuestOsFromOVFDescriptor(long templateId, String guestOsType, String guestOsDescription,
-                                                String minimumHardwareVersion) {
+    public Long retrieveTemplateGuestOsIdFromGuestOsInfo(long templateId, String guestOsType, String guestOsDescription,
+                                                         String minimumHardwareVersion) {
         VMTemplateVO template = templateDao.findById(templateId);
         Hypervisor.HypervisorType hypervisor = template.getHypervisorType();
         if (hypervisor != Hypervisor.HypervisorType.VMware) {
-            return;
+            return null;
         }
+
         String minimumHypervisorVersion = getMinimumSupportedHypervisorVersionForHardwareVersion(minimumHardwareVersion);
         LOGGER.info("Minimum hardware version " + minimumHardwareVersion + " matched to hypervisor version " + minimumHypervisorVersion + ". " +
                 "Checking guest OS supporting this version");
@@ -175,14 +168,27 @@ private void handleGuestOsFromOVFDescriptor(long templateId, String guestOsType,
 
         if (CollectionUtils.isNotEmpty(guestOsMappings)) {
             GuestOSHypervisorVO mapping = guestOsMappings.get(0);
-            long guestOsId = mapping.getGuestOsId();
-            LOGGER.info("Updating deploy-as-is template guest OS to " + guestOsType);
-            updateTemplateGuestOsId(template, guestOsId);
+            return mapping.getGuestOsId();
         } else {
             throw new CloudRuntimeException("Did not find a guest OS with type " + guestOsType);
         }
     }
 
+    /**
+     * Handle the guest OS read from the OVF and try to match it to an existing guest OS in DB.
+     * If the guest OS cannot be mapped to an existing guest OS in DB, then create it and create support for hypervisor versions.
+     * Roll back actions in case of unexpected errors
+     */
+    private void handleGuestOsFromOVFDescriptor(long templateId, String guestOsType, String guestOsDescription,
+                                                String minimumHardwareVersion) {
+        Long guestOsId = retrieveTemplateGuestOsIdFromGuestOsInfo(templateId, guestOsType, guestOsDescription, minimumHardwareVersion);
+        if (guestOsId != null) {
+            LOGGER.info("Updating deploy-as-is template guest OS to " + guestOsType);
+            VMTemplateVO template = templateDao.findById(templateId);
+            updateTemplateGuestOsId(template, guestOsId);
+        }
+    }
+
     /**
      * Updates the deploy-as-is template guest OS doing:
      * - Create a new guest OS with the guest OS description parsed from the OVF
@@ -268,38 +274,6 @@ public Map<String, String> getVirtualMachineDeployAsIsProperties(VirtualMachineP
         return map;
     }
 
-    @Override
-    public String getAllocatedVirtualMachineTemplatePath(VirtualMachineProfile vm, String configuration, String destStoragePool) {
-        StoragePoolVO storagePoolVO = storagePoolDao.findByUuid(destStoragePool);
-        VMTemplateStoragePoolVO tmplRef = templateStoragePoolDao.findByPoolTemplate(storagePoolVO.getId(),
-                vm.getTemplate().getId(), configuration);
-        if (tmplRef != null) {
-            return tmplRef.getInstallPath();
-        }
-        return null;
-    }
-
-    @Override
-    public String getAllocatedVirtualMachineDestinationStoragePool(VirtualMachineProfile vm) {
-        if (vm != null) {
-            if (CollectionUtils.isNotEmpty(vm.getDisks())) {
-                for (DiskTO disk : vm.getDisks()) {
-                    if (disk.getType() == Volume.Type.ISO) {
-                        continue;
-                    }
-                    DataTO data = disk.getData();
-                    if (data != null) {
-                        DataStoreTO dataStore = data.getDataStore();
-                        if (dataStore != null) {
-                            return dataStore.getUuid();
-                        }
-                    }
-                }
-            }
-        }
-        return null;
-    }
-
     @Override
     public Map<Integer, String> getAllocatedVirtualMachineNicsAdapterMapping(VirtualMachineProfile vm, NicTO[] nics) {
         Map<Integer, String> map = new HashMap<>();
diff --git a/server/src/main/java/com/cloud/storage/ImageStoreUploadMonitorImpl.java b/server/src/main/java/com/cloud/storage/ImageStoreUploadMonitorImpl.java
@@ -25,6 +25,8 @@
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
+import com.cloud.hypervisor.Hypervisor;
+import com.cloud.utils.Pair;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
 import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
@@ -44,6 +46,7 @@
 import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
 import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreVO;
+import org.apache.cloudstack.storage.image.deployasis.DeployAsIsHelper;
 import org.apache.cloudstack.utils.identity.ManagementServerNode;
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
@@ -115,6 +118,8 @@ public class ImageStoreUploadMonitorImpl extends ManagerBase implements ImageSto
     private TemplateService templateService;
     @Inject
     private TemplateJoinDao templateJoinDao;
+    @Inject
+    private DeployAsIsHelper deployAsIsHelper;
 
     private long _nodeId;
     private ScheduledExecutorService _executor = null;
@@ -407,6 +412,20 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
 
                             VMTemplateVO templateUpdate = _templateDao.createForUpdate();
                             templateUpdate.setSize(answer.getVirtualSize());
+                            if (template.getHypervisorType() == Hypervisor.HypervisorType.VMware) {
+                                Pair<String, String> guestOsInfo = answer.getGuestOsInfo();
+                                String minimumHardwareVersion = answer.getMinimumHardwareVersion();
+                                String osType = guestOsInfo.first();
+                                String osDescription = guestOsInfo.second();
+                                s_logger.info("Guest OS information retrieved from the template: " + osType + " - " + osDescription);
+                                try {
+                                    Long guestOsId = deployAsIsHelper.retrieveTemplateGuestOsIdFromGuestOsInfo(template.getId(),
+                                            osType, osDescription, minimumHardwareVersion);
+                                    templateUpdate.setGuestOSId(guestOsId);
+                                } catch (CloudRuntimeException e) {
+                                    s_logger.error("Could not map the guest OS to a CloudStack guest OS", e);
+                                }
+                            }
                             _templateDao.update(tmpTemplate.getId(), templateUpdate);
                             // For multi-disk OVA, check and create data disk templates or root disks as details
                             if (tmpTemplate.getFormat().equals(Storage.ImageFormat.OVA)) {
diff --git a/server/src/main/java/com/cloud/template/TemplateAdapterBase.java b/server/src/main/java/com/cloud/template/TemplateAdapterBase.java
@@ -171,9 +171,8 @@ public TemplateProfile prepare(boolean isIso, long userId, String name, String d
                 requiresHVM = true;
             }
             if (deployAsIs) {
-                GuestOS deployAsIsGuestOs = ApiDBUtils.findGuestOSByDisplayName(DeployAsIsConstants.DEFAULT_GUEST_OS_DEPLOY_AS_IS);
                 s_logger.info("Setting default guest OS for deploy-as-is template while the template registration is not completed");
-                guestOSId = deployAsIsGuestOs.getId();
+                guestOSId = getDefaultDeployAsIsGuestOsId();
             }
         }
 
@@ -341,12 +340,22 @@ private TemplateProfile prepareUploadParamsInternal(UploadParams params) throws
                 params.isDynamicallyScalable(), params.isRoutingType() ? TemplateType.ROUTING : TemplateType.USER, params.isDirectDownload(), false);
     }
 
+    private Long getDefaultDeployAsIsGuestOsId() {
+        GuestOS deployAsIsGuestOs = ApiDBUtils.findGuestOSByDisplayName(DeployAsIsConstants.DEFAULT_GUEST_OS_DEPLOY_AS_IS);
+        return deployAsIsGuestOs.getId();
+    }
+
     @Override
     public TemplateProfile prepare(GetUploadParamsForTemplateCmd cmd) throws ResourceAllocationException {
+        Long osTypeId = cmd.getOsTypeId();
+        if (osTypeId == null) {
+            s_logger.info("Setting the default guest OS for deploy-as-is templates while the template upload is not completed");
+            osTypeId = getDefaultDeployAsIsGuestOsId();
+        }
         UploadParams params = new TemplateUploadParams(CallContext.current().getCallingUserId(), cmd.getName(),
                 cmd.getDisplayText(), cmd.getBits(), BooleanUtils.toBoolean(cmd.isPasswordEnabled()),
                 BooleanUtils.toBoolean(cmd.getRequiresHvm()), BooleanUtils.toBoolean(cmd.isPublic()),
-                BooleanUtils.toBoolean(cmd.isFeatured()), BooleanUtils.toBoolean(cmd.isExtractable()), cmd.getFormat(), cmd.getOsTypeId(),
+                BooleanUtils.toBoolean(cmd.isFeatured()), BooleanUtils.toBoolean(cmd.isExtractable()), cmd.getFormat(), osTypeId,
                 cmd.getZoneId(), HypervisorType.getType(cmd.getHypervisor()), cmd.getChecksum(),
                 cmd.getTemplateTag(), cmd.getEntityOwnerId(), cmd.getDetails(), BooleanUtils.toBoolean(cmd.isSshKeyEnabled()),
                 BooleanUtils.toBoolean(cmd.isDynamicallyScalable()), BooleanUtils.toBoolean(cmd.isRoutingType()));
diff --git a/server/src/main/java/org/apache/cloudstack/storage/image/deployasis/DeployAsIsHelper.java b/server/src/main/java/org/apache/cloudstack/storage/image/deployasis/DeployAsIsHelper.java
@@ -27,9 +27,7 @@ public interface DeployAsIsHelper {
 
     boolean persistTemplateDeployAsIsDetails(long templateId, DownloadAnswer answer, TemplateDataStoreVO tmpltStoreVO);
     Map<String, String> getVirtualMachineDeployAsIsProperties(VirtualMachineProfile vmId);
-
-    String getAllocatedVirtualMachineTemplatePath(VirtualMachineProfile vm, String configuration, String destStoragePool);
-    String getAllocatedVirtualMachineDestinationStoragePool(VirtualMachineProfile vm);
-
     Map<Integer, String> getAllocatedVirtualMachineNicsAdapterMapping(VirtualMachineProfile vm, NicTO[] nics);
+    Long retrieveTemplateGuestOsIdFromGuestOsInfo(long templateId, String guestOsType, String guestOsDescription,
+                                                  String minimumHardwareVersion);
 }
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
@@ -2278,6 +2278,8 @@ private UploadStatusAnswer execute(UploadStatusCommand cmd) {
                 answer.setInstallPath(uploadEntity.getTmpltPath());
                 answer.setPhysicalSize(uploadEntity.getPhysicalSize());
                 answer.setDownloadPercent(100);
+                answer.setGuestOsInfo(uploadEntity.getGuestOsInfo());
+                answer.setMinimumHardwareVersion(uploadEntity.getMinimumHardwareVersion());
                 uploadEntityStateMap.remove(entityUuid);
                 return answer;
             } else if (uploadEntity.getUploadState() == UploadEntity.Status.IN_PROGRESS) {
@@ -3413,6 +3415,14 @@ public String postUpload(String uuid, String filename, long processTimeout) {
                 loc.addFormat(info);
                 uploadEntity.setVirtualSize(info.virtualSize);
                 uploadEntity.setPhysicalSize(info.size);
+                if (info.ovfInformationTO != null) {
+                    if (info.ovfInformationTO.getGuestOsInfo() != null) {
+                        uploadEntity.setGuestOsInfo(info.ovfInformationTO.getGuestOsInfo());
+                    }
+                    if (info.ovfInformationTO.getHardwareSection() != null) {
+                        uploadEntity.setMinimumHardwareVersion(info.ovfInformationTO.getHardwareSection().getMinimiumHardwareVersion());
+                    }
+                }
                 break;
             }
         }
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
@@ -19,6 +19,7 @@
 
 
 import com.cloud.storage.Storage;
+import com.cloud.utils.Pair;
 
 public class UploadEntity {
     private long downloadedsize;
@@ -35,6 +36,8 @@ public class UploadEntity {
     private String description;
     private long contentLength;
     private long processTimeout;
+    private Pair<String, String> guestOsInfo;
+    private String minimumHardwareVersion;
 
     public static enum ResourceType {
         VOLUME, TEMPLATE
@@ -207,4 +210,20 @@ public long getContentLength() {
     public void setContentLength(long contentLength) {
         this.contentLength = contentLength;
     }
+
+    public Pair<String, String> getGuestOsInfo() {
+        return guestOsInfo;
+    }
+
+    public void setGuestOsInfo(Pair<String, String> guestOsInfo) {
+        this.guestOsInfo = guestOsInfo;
+    }
+
+    public void setMinimumHardwareVersion(String minimumHardwareVersion) {
+        this.minimumHardwareVersion = minimumHardwareVersion;
+    }
+
+    public String getMinimumHardwareVersion() {
+        return minimumHardwareVersion;
+    }
 }
