From b0d3168e0b78085036c05245c638b3a0d7eb7750 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 30 Sep 2020 00:54:31 -0300 Subject: [PATCH] Fail template registration when guest OS not found --- .../image/BaseImageStoreDriverImpl.java | 10 +- .../image/deployasis/DeployAsIsHelper.java | 3 +- .../deployasis/DeployAsIsHelperImpl.java | 106 +++++++++--------- 3 files changed, 63 insertions(+), 56 deletions(-) diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java index 58c24537d57..37a7985ce6f 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java @@ -204,13 +204,18 @@ public abstract class BaseImageStoreDriverImpl implements ImageStoreDriver { if (tmpltStoreVO != null) { if (tmpltStoreVO.getDownloadState() == VMTemplateStorageResourceAssoc.Status.DOWNLOADED) { if (template.isDeployAsIs()) { - deployAsIsHelper.persistTemplateDeployAsIsDetails(template.getId(), answer); + boolean persistDeployAsIs = deployAsIsHelper.persistTemplateDeployAsIsDetails(template.getId(), answer, tmpltStoreVO); + if (!persistDeployAsIs) { + LOGGER.info("Failed persisting deploy-as-is template details for template " + template.getName()); + return null; + } } if (LOGGER.isDebugEnabled()) { LOGGER.debug("Template is already in DOWNLOADED state, ignore further incoming DownloadAnswer"); } return null; } + LOGGER.info("Updating store ref entry for template " + template.getName()); TemplateDataStoreVO updateBuilder = _templateStoreDao.createForUpdate(); updateBuilder.setDownloadPercent(answer.getDownloadPct()); updateBuilder.setDownloadState(answer.getDownloadStatus()); @@ -245,9 +250,6 @@ public abstract class BaseImageStoreDriverImpl implements ImageStoreDriver { templateDaoBuilder.setChecksum(answer.getCheckSum()); _templateDao.update(obj.getId(), templateDaoBuilder); } - if (template.isDeployAsIs()) { - deployAsIsHelper.persistTemplateDeployAsIsDetails(template.getId(), answer); - } CreateCmdResult result = new CreateCmdResult(null, null); caller.complete(result); diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/deployasis/DeployAsIsHelper.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/image/deployasis/DeployAsIsHelper.java index 303161c0c59..4f7277e31cd 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/deployasis/DeployAsIsHelper.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/image/deployasis/DeployAsIsHelper.java @@ -19,12 +19,13 @@ package org.apache.cloudstack.storage.image.deployasis; import com.cloud.agent.api.storage.DownloadAnswer; import com.cloud.agent.api.to.NicTO; import com.cloud.vm.VirtualMachineProfile; +import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO; import java.util.Map; public interface DeployAsIsHelper { - void persistTemplateDeployAsIsDetails(long templateId, DownloadAnswer answer); + boolean persistTemplateDeployAsIsDetails(long templateId, DownloadAnswer answer, TemplateDataStoreVO tmpltStoreVO); Map getVirtualMachineDeployAsIsProperties(VirtualMachineProfile vmId); String getAllocatedVirtualMachineTemplatePath(VirtualMachineProfile vm, String configuration, String destStoragePool); 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 index 9dde8959152..0d52f57ac98 100644 --- 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 @@ -40,6 +40,7 @@ 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; @@ -50,15 +51,15 @@ import com.cloud.storage.dao.VMTemplatePoolDao; import com.cloud.utils.Pair; import com.cloud.utils.compression.CompressionUtil; import com.cloud.utils.crypt.DBEncryptionUtil; -import com.cloud.utils.db.Transaction; -import com.cloud.utils.db.TransactionCallbackNoReturn; -import com.cloud.utils.db.TransactionStatus; +import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.VirtualMachineProfile; 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; import org.apache.commons.lang.ArrayUtils; import org.apache.commons.lang.StringUtils; @@ -72,6 +73,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import static org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.State.Failed; + @Component public class DeployAsIsHelperImpl implements DeployAsIsHelper { @@ -94,6 +97,8 @@ public class DeployAsIsHelperImpl implements DeployAsIsHelper { private GuestOSHypervisorDao guestOSHypervisorDao; @Inject private GuestOSCategoryDao guestOSCategoryDao; + @Inject + private TemplateDataStoreDao templateDataStoreDao; static { GsonBuilder builder = new GsonBuilder(); @@ -101,45 +106,52 @@ public class DeployAsIsHelperImpl implements DeployAsIsHelper { gson = builder.create(); } - public void persistTemplateDeployAsIsDetails(long templateId, DownloadAnswer answer) { - OVFInformationTO ovfInformationTO = answer.getOvfInformationTO(); - if (ovfInformationTO != null) { - List ovfProperties = ovfInformationTO.getProperties(); - List networkRequirements = ovfInformationTO.getNetworks(); - OVFVirtualHardwareSectionTO ovfHardwareSection = ovfInformationTO.getHardwareSection(); - List eulaSections = ovfInformationTO.getEulaSections(); - Pair guestOsInfo = ovfInformationTO.getGuestOsInfo(); + public boolean persistTemplateDeployAsIsDetails(long templateId, DownloadAnswer answer, TemplateDataStoreVO tmpltStoreVO) { + try { + OVFInformationTO ovfInformationTO = answer.getOvfInformationTO(); + if (ovfInformationTO != null) { + List ovfProperties = ovfInformationTO.getProperties(); + List networkRequirements = ovfInformationTO.getNetworks(); + OVFVirtualHardwareSectionTO ovfHardwareSection = ovfInformationTO.getHardwareSection(); + List eulaSections = ovfInformationTO.getEulaSections(); + Pair guestOsInfo = ovfInformationTO.getGuestOsInfo(); - if (CollectionUtils.isNotEmpty(ovfProperties)) { - persistTemplateDeployAsIsInformationTOList(templateId, ovfProperties); - } - if (CollectionUtils.isNotEmpty(networkRequirements)) { - persistTemplateDeployAsIsInformationTOList(templateId, networkRequirements); - } - if (CollectionUtils.isNotEmpty(eulaSections)) { - persistTemplateDeployAsIsInformationTOList(templateId, eulaSections); - } - String minimumHardwareVersion = null; - if (ovfHardwareSection != null) { - if (CollectionUtils.isNotEmpty(ovfHardwareSection.getConfigurations())) { - persistTemplateDeployAsIsInformationTOList(templateId, ovfHardwareSection.getConfigurations()); + if (CollectionUtils.isNotEmpty(ovfProperties)) { + persistTemplateDeployAsIsInformationTOList(templateId, ovfProperties); } - if (CollectionUtils.isNotEmpty(ovfHardwareSection.getCommonHardwareItems())) { - persistTemplateDeployAsIsInformationTOList(templateId, ovfHardwareSection.getCommonHardwareItems()); + if (CollectionUtils.isNotEmpty(networkRequirements)) { + persistTemplateDeployAsIsInformationTOList(templateId, networkRequirements); } - minimumHardwareVersion = ovfHardwareSection.getMinimiumHardwareVersion(); - } - if (guestOsInfo != null) { - String osType = guestOsInfo.first(); - String osDescription = guestOsInfo.second(); - LOGGER.info("Guest OS information retrieved from the template: " + osType + " - " + osDescription); - try { + if (CollectionUtils.isNotEmpty(eulaSections)) { + persistTemplateDeployAsIsInformationTOList(templateId, eulaSections); + } + String minimumHardwareVersion = null; + if (ovfHardwareSection != null) { + if (CollectionUtils.isNotEmpty(ovfHardwareSection.getConfigurations())) { + persistTemplateDeployAsIsInformationTOList(templateId, ovfHardwareSection.getConfigurations()); + } + if (CollectionUtils.isNotEmpty(ovfHardwareSection.getCommonHardwareItems())) { + persistTemplateDeployAsIsInformationTOList(templateId, ovfHardwareSection.getCommonHardwareItems()); + } + minimumHardwareVersion = ovfHardwareSection.getMinimiumHardwareVersion(); + } + if (guestOsInfo != null) { + String osType = guestOsInfo.first(); + String osDescription = guestOsInfo.second(); + LOGGER.info("Guest OS information retrieved from the template: " + osType + " - " + osDescription); handleGuestOsFromOVFDescriptor(templateId, osType, osDescription, minimumHardwareVersion); - } catch (Exception e) { - LOGGER.error("Error handling the guest OS read from the OVF " + osType, e); } } + } catch (Exception e) { + LOGGER.error("Error persisting deploy-as-is details for template " + templateId, e); + tmpltStoreVO.setErrorString(e.getMessage()); + tmpltStoreVO.setState(Failed); + tmpltStoreVO.setDownloadState(VMTemplateStorageResourceAssoc.Status.DOWNLOAD_ERROR); + templateDataStoreDao.update(tmpltStoreVO.getId(), tmpltStoreVO); + return false; } + LOGGER.info("Successfully persisted deploy-as-is details for template " + templateId); + return true; } /** @@ -161,22 +173,14 @@ public class DeployAsIsHelperImpl implements DeployAsIsHelper { List guestOsMappings = guestOSHypervisorDao.listByOsNameAndHypervisorMinimumVersion(guestOsType, hypervisor.toString(), minimumHypervisorVersion); - Transaction.execute(new TransactionCallbackNoReturn() { - @Override - public void doInTransactionWithoutResult(TransactionStatus status) { - 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); - } else { - // The guest OS is not present in DB, create a new guest OS entry and mappings for supported versions - List hypervisorVersions = guestOSHypervisorDao.listHypervisorSupportedVersionsFromMinimumVersion( - hypervisor.toString(), minimumHypervisorVersion); - updateDeployAsIsTemplateToNewGuestOs(template, guestOsType, guestOsDescription, hypervisor, hypervisorVersions); - } - } - }); + 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); + } else { + throw new CloudRuntimeException("Did not find a guest OS with type " + guestOsType); + } } /**