From 046014b4c55056e96098c6a58da8fd88ef64f131 Mon Sep 17 00:00:00 2001 From: Abhisar Sinha <63767682+abh1sar@users.noreply.github.com> Date: Tue, 14 Oct 2025 14:51:57 +0530 Subject: [PATCH] NAS BnR: Create Instance from Backup issues (#11754) * add createCrossZoneInstnaceEnabled to BackupOfferingResponse * show use IP Address from Backup button when orignal instance is expunged * Fix NPE in takeBackup if the vm template is deleted. * Add since to Cross zone instance creation in BackupOfferingResponse.java Co-authored-by: Suresh Kumar Anaparti * Store and show Guest os type in the backup metadata * show warning in create instance from backup form if guest os type is different * show warning in create instance from backup form if guest os type is different * backupvmexpunged -> isbackupvmexpunged * review comments * fix npe * improve err msg * err msg --------- Co-authored-by: Suresh Kumar Anaparti --- .../apache/cloudstack/api/ApiConstants.java | 1 + .../api/response/BackupOfferingResponse.java | 8 ++++ .../api/response/BackupResponse.java | 8 ++++ .../backup/dao/BackupOfferingDao.java | 2 +- .../backup/dao/BackupOfferingDaoImpl.java | 5 ++- .../main/java/com/cloud/api/ApiDBUtils.java | 12 +++++- .../java/com/cloud/api/ApiResponseHelper.java | 4 +- .../java/com/cloud/vm/UserVmManagerImpl.java | 8 ++-- .../cloudstack/backup/BackupManagerImpl.java | 39 ++++++++++++++----- .../cloudstack/backup/BackupManagerTest.java | 6 ++- ui/public/locales/en.json | 2 + ui/src/components/view/BackupMetadata.vue | 22 ++++++----- ui/src/components/view/DeployVMFromBackup.vue | 35 +++++++++++++++++ ui/src/views/storage/CreateVMFromBackup.vue | 33 ++++++---------- 14 files changed, 133 insertions(+), 52 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index 5d2bbe858d7..307104af92c 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -64,6 +64,7 @@ public class ApiConstants { public static final String BACKUP_STORAGE_LIMIT = "backupstoragelimit"; public static final String BACKUP_STORAGE_TOTAL = "backupstoragetotal"; public static final String BACKUP_VM_OFFERING_REMOVED = "vmbackupofferingremoved"; + public static final String IS_BACKUP_VM_EXPUNGED = "isbackupvmexpunged"; public static final String BACKUP_TOTAL = "backuptotal"; public static final String BASE64_IMAGE = "base64image"; public static final String BGP_PEERS = "bgppeers"; diff --git a/api/src/main/java/org/apache/cloudstack/api/response/BackupOfferingResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/BackupOfferingResponse.java index 0e895fa4e96..4120f68d9da 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/BackupOfferingResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/BackupOfferingResponse.java @@ -61,6 +61,10 @@ public class BackupOfferingResponse extends BaseResponse { @Param(description = "zone name") private String zoneName; + @SerializedName(ApiConstants.CROSS_ZONE_INSTANCE_CREATION) + @Param(description = "the backups with this offering can be used to create Instances on all Zones", since = "4.22.0") + private Boolean crossZoneInstanceCreation; + @SerializedName(ApiConstants.CREATED) @Param(description = "the date this backup offering was created") private Date created; @@ -97,6 +101,10 @@ public class BackupOfferingResponse extends BaseResponse { this.zoneName = zoneName; } + public void setCrossZoneInstanceCreation(Boolean crossZoneInstanceCreation) { + this.crossZoneInstanceCreation = crossZoneInstanceCreation; + } + public void setCreated(Date created) { this.created = created; } diff --git a/api/src/main/java/org/apache/cloudstack/api/response/BackupResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/BackupResponse.java index 0ae558ac803..b9eca67d9a8 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/BackupResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/BackupResponse.java @@ -123,6 +123,10 @@ public class BackupResponse extends BaseResponse { @Param(description = "The backup offering corresponding to this backup was removed from the VM", since = "4.21.0") private Boolean vmOfferingRemoved; + @SerializedName(ApiConstants.IS_BACKUP_VM_EXPUNGED) + @Param(description = "Indicates whether the VM from which the backup was taken is expunged or not", since = "4.22.0") + private Boolean isVmExpunged; + public String getId() { return id; } @@ -306,4 +310,8 @@ public class BackupResponse extends BaseResponse { public void setVmOfferingRemoved(Boolean vmOfferingRemoved) { this.vmOfferingRemoved = vmOfferingRemoved; } + + public void setVmExpunged(Boolean isVmExpunged) { + this.isVmExpunged = isVmExpunged; + } } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDao.java b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDao.java index d001de8b6c6..d40e828121e 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDao.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDao.java @@ -24,7 +24,7 @@ import org.apache.cloudstack.backup.BackupOfferingVO; import com.cloud.utils.db.GenericDao; public interface BackupOfferingDao extends GenericDao { - BackupOfferingResponse newBackupOfferingResponse(BackupOffering policy); + BackupOfferingResponse newBackupOfferingResponse(BackupOffering policy, Boolean crossZoneInstanceCreation); BackupOffering findByExternalId(String externalId, Long zoneId); BackupOffering findByName(String name, Long zoneId); } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDaoImpl.java index 9d67d07fe5e..a41e4e70d33 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDaoImpl.java @@ -50,7 +50,7 @@ public class BackupOfferingDaoImpl extends GenericDaoBase vmDetails = vmInstanceDetailsDao.listDetails(vm.getId()); HashMap settings = new HashMap<>(); @@ -2143,7 +2155,6 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { if (details.containsKey(ApiConstants.TEMPLATE_ID)) { VirtualMachineTemplate template = vmTemplateDao.findByUuid(details.get(ApiConstants.TEMPLATE_ID)); if (template != null) { - details.put(ApiConstants.TEMPLATE_ID, template.getUuid()); details.put(ApiConstants.TEMPLATE_NAME, template.getName()); details.put(ApiConstants.IS_ISO, String.valueOf(template.getFormat().equals(Storage.ImageFormat.ISO))); } @@ -2151,7 +2162,6 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { if (details.containsKey(ApiConstants.SERVICE_OFFERING_ID)) { ServiceOffering serviceOffering = serviceOfferingDao.findByUuid(details.get(ApiConstants.SERVICE_OFFERING_ID)); if (serviceOffering != null) { - details.put(ApiConstants.SERVICE_OFFERING_ID, serviceOffering.getUuid()); details.put(ApiConstants.SERVICE_OFFERING_NAME, serviceOffering.getName()); } } @@ -2186,10 +2196,15 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { response.setId(backup.getUuid()); response.setName(backup.getName()); response.setDescription(backup.getDescription()); - response.setVmName(vm.getHostName()); - response.setVmId(vm.getUuid()); - if (vm.getBackupOfferingId() == null || vm.getBackupOfferingId() != backup.getBackupOfferingId()) { - response.setVmOfferingRemoved(true); + if (vm != null) { + response.setVmName(vm.getHostName()); + response.setVmId(vm.getUuid()); + if (vm.getBackupOfferingId() == null || vm.getBackupOfferingId() != backup.getBackupOfferingId()) { + response.setVmOfferingRemoved(true); + } + } + if (vm == null || VirtualMachine.State.Expunging.equals(vm.getState())) { + response.setVmExpunged(true); } response.setExternalId(backup.getExternalId()); response.setType(backup.getType()); @@ -2205,9 +2220,11 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { } } // ACS 4.20: For backups taken prior this release the backup.backed_volumes column would be empty hence use vm_instance.backup_volumes - String backedUpVolumes; + String backedUpVolumes = ""; if (Objects.isNull(backup.getBackedUpVolumes())) { - backedUpVolumes = new Gson().toJson(vm.getBackupVolumeList().toArray(), Backup.VolumeInfo[].class); + if (vm != null) { + backedUpVolumes = new Gson().toJson(vm.getBackupVolumeList().toArray(), Backup.VolumeInfo[].class); + } } else { backedUpVolumes = new Gson().toJson(backup.getBackedUpVolumes().toArray(), Backup.VolumeInfo[].class); } @@ -2223,7 +2240,9 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { if (Boolean.TRUE.equals(listVmDetails)) { Map vmDetails = new HashMap<>(); - vmDetails.put(ApiConstants.HYPERVISOR, vm.getHypervisorType().toString()); + if (vm != null) { + vmDetails.put(ApiConstants.HYPERVISOR, vm.getHypervisorType().toString()); + } Map details = getDetailsFromBackupDetails(backup.getId()); vmDetails.putAll(details); response.setVmDetails(vmDetails); diff --git a/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java b/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java index c9391211fac..8b13fd47494 100644 --- a/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java +++ b/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java @@ -49,6 +49,7 @@ import com.cloud.storage.Volume; import com.cloud.storage.VolumeApiService; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.DiskOfferingDao; +import com.cloud.storage.dao.GuestOSDao; import com.cloud.storage.dao.VMTemplateDao; import com.cloud.storage.dao.VolumeDao; import com.cloud.user.Account; @@ -237,6 +238,9 @@ public class BackupManagerTest { @Mock DomainDao domainDao; + @Mock + private GuestOSDao _guestOSDao; + private Gson gson; private String[] hostPossibleValues = {"127.0.0.1", "hostname"}; @@ -1572,14 +1576,12 @@ public class BackupManagerTest { VMTemplateVO template = mock(VMTemplateVO.class); when(template.getFormat()).thenReturn(Storage.ImageFormat.QCOW2); - when(template.getUuid()).thenReturn(templateUuid); when(template.getName()).thenReturn("template1"); when(vmTemplateDao.findByUuid(templateUuid)).thenReturn(template); Map details = new HashMap<>(); details.put(ApiConstants.TEMPLATE_ID, templateUuid); ServiceOfferingVO serviceOffering = mock(ServiceOfferingVO.class); - when(serviceOffering.getUuid()).thenReturn(serviceOfferingUuid); when(serviceOffering.getName()).thenReturn("service-offering1"); when(serviceOfferingDao.findByUuid(serviceOfferingUuid)).thenReturn(serviceOffering); details.put(ApiConstants.SERVICE_OFFERING_ID, serviceOfferingUuid); diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index 5f1621c334e..0b6a4e3ceb9 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -2887,6 +2887,8 @@ "message.action.create.snapshot.from.vmsnapshot": "Please confirm that you want to create Snapshot from Instance Snapshot", "message.action.create.instance.from.backup": "Please confirm that you want to create a new Instance from the given Backup.
Click on configure to edit the parameters for the new Instance before creation.", "message.create.instance.from.backup.different.zone": "Creating Instance from Backup on a different Zone. Please ensure that the backup repository is accessible in the selected Zone.", +"message.template.ostype.different.from.backup": "Selected Template has a different OS type than the Backup. Please proceed with caution.", +"message.iso.ostype.different.from.backup": "Selected ISO has a different OS type than the Backup. Please proceed with caution.", "message.action.delete.asnrange": "Please confirm the AS range that you want to delete", "message.action.delete.autoscale.vmgroup": "Please confirm that you want to delete this autoscaling group.", "message.action.delete.backup.offering": "Please confirm that you want to delete this backup offering?", diff --git a/ui/src/components/view/BackupMetadata.vue b/ui/src/components/view/BackupMetadata.vue index fea93a4e9c7..b8bf7fda916 100644 --- a/ui/src/components/view/BackupMetadata.vue +++ b/ui/src/components/view/BackupMetadata.vue @@ -42,6 +42,11 @@ {{ getTemplateDisplayName() }} +
+ + {{ backupMetadata.osname }} + +
{{ getServiceOfferingDisplayName() }} @@ -84,15 +89,14 @@ export default { if (!this.backupMetadata || Object.keys(this.backupMetadata).length === 0) { return [] } - const fieldOrder = [] - fieldOrder.push('templateid') - if (this.backupMetadata.isiso === 'true') { - fieldOrder.push('hypervisor') - } - fieldOrder.push('serviceofferingid') - fieldOrder.push('nics') - fieldOrder.push('vmsettings') - + const fieldOrder = [ + 'templateid', + 'ostypeid', + 'hypervisor', + 'serviceofferingid', + 'nics', + 'vmsettings' + ] return fieldOrder.filter(field => this.backupMetadata[field] !== undefined) }, getNicEntities () { diff --git a/ui/src/components/view/DeployVMFromBackup.vue b/ui/src/components/view/DeployVMFromBackup.vue index 1f6a7cc2c8d..c1f0ffc6d14 100644 --- a/ui/src/components/view/DeployVMFromBackup.vue +++ b/ui/src/components/view/DeployVMFromBackup.vue @@ -168,6 +168,23 @@ :tabList="tabList" :activeTabKey="tabKey" @tabChange="key => onTabChange(key, 'tabKey')"> + + +
{{ $t('message.template.desc') }}
@@ -933,6 +950,7 @@ export default { dataPreFill: {}, showDetails: false, showRootDiskSizeChanger: false, + showOsTypeWarning: false, showOverrideDiskOfferingOption: false, securitygroupids: [], rootDiskSizeFixed: 0, @@ -978,6 +996,15 @@ export default { isNormalUserOrProject () { return ['User'].includes(this.$store.getters.userInfo.roletype) || store.getters.project.id }, + selectedTemplateIso () { + if (this.tabKey === 'templateid' && this.template?.ostypeid) { + return { label: this.$t('label.template'), item: this.template, message: this.$t('message.template.ostype.different.from.backup') } + } + if (this.tabKey === 'isoid' && this.iso?.ostypeid) { + return { label: this.$t('label.iso'), item: this.iso, message: this.$t('message.iso.ostype.different.from.backup') } + } + return null + }, diskSize () { const customRootDiskSize = _.get(this.instanceConfig, 'rootdisksize', null) const customDataDiskSize = _.get(this.instanceConfig, 'size', null) @@ -1687,6 +1714,7 @@ export default { this.form.iothreadsenabled = template.details && Object.prototype.hasOwnProperty.call(template.details, 'iothreads') this.form.iodriverpolicy = template.details?.['io.policy'] this.form.keyboard = template.details?.keyboard + this.showOsTypeWarning = this.dataPreFill.ostypeid && template.ostypeid !== this.dataPreFill.ostypeid if (template.details['vmware-to-kvm-mac-addresses']) { this.dataPreFill.macAddressArray = JSON.parse(template.details['vmware-to-kvm-mac-addresses']) } @@ -1700,6 +1728,13 @@ export default { this.tabKey = 'isoid' this.form.isoid = value this.form.templateid = null + for (const key in this.options.isos) { + var iso = _.find(_.get(this.options.isos[key], 'iso', []), (option) => option.id === value) + if (iso) { + this.showOsTypeWarning = this.dataPreFill.ostypeid && iso.ostypeid !== this.dataPreFill.ostypeid + break + } + } } else if (['cpuspeed', 'cpunumber', 'memory'].includes(name)) { this.vm[name] = value this.form[name] = value diff --git a/ui/src/views/storage/CreateVMFromBackup.vue b/ui/src/views/storage/CreateVMFromBackup.vue index 0edee18ddd3..8d29397e45a 100644 --- a/ui/src/views/storage/CreateVMFromBackup.vue +++ b/ui/src/views/storage/CreateVMFromBackup.vue @@ -29,7 +29,7 @@ - +