vmware: Fix template upload from local (#4555)

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 <daan.hoogland@gmail.com>
Co-authored-by: dahn <daan.hoogland@gmail.com>

api/src/main/java/org/apache/cloudstack/api/command/user/template/GetUploadParamsForTemplateCmd.java

@@ -22,6 +22,7 @@ import java.net.MalformedURLException;
 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 @@ public class GetUploadParamsForTemplateCmd extends AbstractGetUploadParamsCmd {
         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

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 class UploadStatusAnswer extends Answer {
     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 class UploadStatusAnswer extends Answer {
     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;
+    }
 }

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.hypervisor.Hypervisor;
 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.Gson;
 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 class DeployAsIsHelperImpl implements DeployAsIsHelper {
     }
 
     /**
-     * Handle the guest OS read from the OVF and try to match it to an existing guest OS in DB.
+     * Returns the mapped guest OS from the OVF file of the template to the CloudStack database OS ID
-     * 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
      */
-    private void handleGuestOsFromOVFDescriptor(long templateId, String guestOsType, String guestOsDescription,
+    public Long retrieveTemplateGuestOsIdFromGuestOsInfo(long templateId, String guestOsType, String guestOsDescription,
-                                                String minimumHardwareVersion) {
+                                                         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 @@ public class DeployAsIsHelperImpl implements DeployAsIsHelper {
 
         if (CollectionUtils.isNotEmpty(guestOsMappings)) {
             GuestOSHypervisorVO mapping = guestOsMappings.get(0);
-            long guestOsId = mapping.getGuestOsId();
+            return mapping.getGuestOsId();
-            LOGGER.info("Updating deploy-as-is template guest OS to " + guestOsType);
-            updateTemplateGuestOsId(template, guestOsId);
         } 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 class DeployAsIsHelperImpl implements DeployAsIsHelper {
         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<>();

server/src/main/java/com/cloud/storage/ImageStoreUploadMonitorImpl.java

@@ -25,6 +25,8 @@ import java.util.concurrent.TimeUnit;
 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.TemplateDataStoreDao;
 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 class ImageStoreUploadMonitorImpl extends ManagerBase implements ImageSto
 
                             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)) {

server/src/main/java/com/cloud/template/TemplateAdapterBase.java

@@ -171,9 +171,8 @@ public abstract class TemplateAdapterBase extends AdapterBase implements Templat
                 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 @@ public abstract class TemplateAdapterBase extends AdapterBase implements Templat
                 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()));

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);
 }

services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java

@@ -2278,6 +2278,8 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
                 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 class NfsSecondaryStorageResource extends ServerResourceBase implements S
                 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;
             }
         }

services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/UploadEntity.java

@@ -19,6 +19,7 @@ package org.apache.cloudstack.storage.template;
 
 
 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 class UploadEntity {
     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;
+    }
 }