Skip to content

Commit 046014b

Browse files
abh1sarsureshanaparti
abh1sar
and
sureshanaparti
authored
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 <[email protected]> * 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 <[email protected]>
1 parent c0a4392 commit 046014b

File tree

14 files changed

+133
-52
lines changed

14 files changed

+133
-52
lines changed

‎api/src/main/java/org/apache/cloudstack/api/ApiConstants.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ public class ApiConstants {
6464
public static final String BACKUP_STORAGE_LIMIT = "backupstoragelimit";
6565
public static final String BACKUP_STORAGE_TOTAL = "backupstoragetotal";
6666
public static final String BACKUP_VM_OFFERING_REMOVED = "vmbackupofferingremoved";
67+
public static final String IS_BACKUP_VM_EXPUNGED = "isbackupvmexpunged";
6768
public static final String BACKUP_TOTAL = "backuptotal";
6869
public static final String BASE64_IMAGE = "base64image";
6970
public static final String BGP_PEERS = "bgppeers";

‎api/src/main/java/org/apache/cloudstack/api/response/BackupOfferingResponse.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ public class BackupOfferingResponse extends BaseResponse {
6161
@Param(description = "zone name")
6262
private String zoneName;
6363

64+
@SerializedName(ApiConstants.CROSS_ZONE_INSTANCE_CREATION)
65+
@Param(description = "the backups with this offering can be used to create Instances on all Zones", since = "4.22.0")
66+
private Boolean crossZoneInstanceCreation;
67+
6468
@SerializedName(ApiConstants.CREATED)
6569
@Param(description = "the date this backup offering was created")
6670
private Date created;
@@ -97,6 +101,10 @@ public void setZoneName(String zoneName) {
97101
this.zoneName = zoneName;
98102
}
99103

104+
public void setCrossZoneInstanceCreation(Boolean crossZoneInstanceCreation) {
105+
this.crossZoneInstanceCreation = crossZoneInstanceCreation;
106+
}
107+
100108
public void setCreated(Date created) {
101109
this.created = created;
102110
}

‎api/src/main/java/org/apache/cloudstack/api/response/BackupResponse.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,10 @@ public class BackupResponse extends BaseResponse {
123123
@Param(description = "The backup offering corresponding to this backup was removed from the VM", since = "4.21.0")
124124
private Boolean vmOfferingRemoved;
125125

126+
@SerializedName(ApiConstants.IS_BACKUP_VM_EXPUNGED)
127+
@Param(description = "Indicates whether the VM from which the backup was taken is expunged or not", since = "4.22.0")
128+
private Boolean isVmExpunged;
129+
126130
public String getId() {
127131
return id;
128132
}
@@ -306,4 +310,8 @@ public Boolean getVmOfferingRemoved() {
306310
public void setVmOfferingRemoved(Boolean vmOfferingRemoved) {
307311
this.vmOfferingRemoved = vmOfferingRemoved;
308312
}
313+
314+
public void setVmExpunged(Boolean isVmExpunged) {
315+
this.isVmExpunged = isVmExpunged;
316+
}
309317
}

‎engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDao.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
import com.cloud.utils.db.GenericDao;
2525

2626
public interface BackupOfferingDao extends GenericDao<BackupOfferingVO, Long> {
27-
BackupOfferingResponse newBackupOfferingResponse(BackupOffering policy);
27+
BackupOfferingResponse newBackupOfferingResponse(BackupOffering policy, Boolean crossZoneInstanceCreation);
2828
BackupOffering findByExternalId(String externalId, Long zoneId);
2929
BackupOffering findByName(String name, Long zoneId);
3030
}

‎engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDaoImpl.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ protected void init() {
5050
}
5151

5252
@Override
53-
public BackupOfferingResponse newBackupOfferingResponse(BackupOffering offering) {
53+
public BackupOfferingResponse newBackupOfferingResponse(BackupOffering offering, Boolean crossZoneInstanceCreation) {
5454
DataCenterVO zone = dataCenterDao.findById(offering.getZoneId());
5555

5656
BackupOfferingResponse response = new BackupOfferingResponse();
@@ -64,6 +64,9 @@ public BackupOfferingResponse newBackupOfferingResponse(BackupOffering offering)
6464
response.setZoneId(zone.getUuid());
6565
response.setZoneName(zone.getName());
6666
}
67+
if (crossZoneInstanceCreation) {
68+
response.setCrossZoneInstanceCreation(true);
69+
}
6770
response.setCreated(offering.getCreated());
6871
response.setObjectName("backupoffering");
6972
return response;

‎server/src/main/java/com/cloud/api/ApiDBUtils.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,11 @@
7575
import org.apache.cloudstack.api.response.VpcOfferingResponse;
7676
import org.apache.cloudstack.api.response.ZoneResponse;
7777
import org.apache.cloudstack.backup.BackupOffering;
78+
import org.apache.cloudstack.backup.BackupRepository;
7879
import org.apache.cloudstack.backup.BackupSchedule;
7980
import org.apache.cloudstack.backup.dao.BackupDao;
8081
import org.apache.cloudstack.backup.dao.BackupOfferingDao;
82+
import org.apache.cloudstack.backup.dao.BackupRepositoryDao;
8183
import org.apache.cloudstack.backup.dao.BackupScheduleDao;
8284
import org.apache.cloudstack.context.CallContext;
8385
import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
@@ -493,6 +495,7 @@ public class ApiDBUtils {
493495
static BackupDao s_backupDao;
494496
static BackupScheduleDao s_backupScheduleDao;
495497
static BackupOfferingDao s_backupOfferingDao;
498+
static BackupRepositoryDao s_backupRepositoryDao;
496499
static NicDao s_nicDao;
497500
static ResourceManagerUtil s_resourceManagerUtil;
498501
static SnapshotPolicyDetailsDao s_snapshotPolicyDetailsDao;
@@ -751,6 +754,8 @@ public class ApiDBUtils {
751754
@Inject
752755
private BackupOfferingDao backupOfferingDao;
753756
@Inject
757+
private BackupRepositoryDao backupRepositoryDao;
758+
@Inject
754759
private BackupScheduleDao backupScheduleDao;
755760
@Inject
756761
private NicDao nicDao;
@@ -899,6 +904,7 @@ void init() {
899904
s_backupDao = backupDao;
900905
s_backupScheduleDao = backupScheduleDao;
901906
s_backupOfferingDao = backupOfferingDao;
907+
s_backupRepositoryDao = backupRepositoryDao;
902908
s_resourceIconDao = resourceIconDao;
903909
s_resourceManagerUtil = resourceManagerUtil;
904910
s_objectStoreDao = objectStoreDao;
@@ -2297,8 +2303,10 @@ public static BackupScheduleResponse newBackupScheduleResponse(BackupSchedule sc
22972303
return s_backupScheduleDao.newBackupScheduleResponse(schedule);
22982304
}
22992305

2300-
public static BackupOfferingResponse newBackupOfferingResponse(BackupOffering policy) {
2301-
return s_backupOfferingDao.newBackupOfferingResponse(policy);
2306+
public static BackupOfferingResponse newBackupOfferingResponse(BackupOffering offering) {
2307+
BackupRepository repository = s_backupRepositoryDao.findByUuid(offering.getExternalId());
2308+
Boolean crossZoneInstanceCreationEnabled = repository != null ? Boolean.TRUE.equals(repository.crossZoneInstanceCreationEnabled()) : false;
2309+
return s_backupOfferingDao.newBackupOfferingResponse(offering, crossZoneInstanceCreationEnabled);
23022310
}
23032311

23042312
public static NicVO findByIp4AddressAndNetworkId(String ip4Address, long networkId) {

‎server/src/main/java/com/cloud/api/ApiResponseHelper.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5040,8 +5040,8 @@ public BackupScheduleResponse createBackupScheduleResponse(BackupSchedule schedu
50405040
}
50415041

50425042
@Override
5043-
public BackupOfferingResponse createBackupOfferingResponse(BackupOffering policy) {
5044-
return ApiDBUtils.newBackupOfferingResponse(policy);
5043+
public BackupOfferingResponse createBackupOfferingResponse(BackupOffering offering) {
5044+
return ApiDBUtils.newBackupOfferingResponse(offering);
50455045
}
50465046

50475047
public ManagementServerResponse createManagementResponse(ManagementServerHost mgmt) {

‎server/src/main/java/com/cloud/vm/UserVmManagerImpl.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9606,11 +9606,11 @@ public UserVm allocateVMFromBackup(CreateVMFromBackupCmd cmd) throws Insufficien
96069606
} else {
96079607
String serviceOfferingUuid = backup.getDetail(ApiConstants.SERVICE_OFFERING_ID);
96089608
if (serviceOfferingUuid == null) {
9609-
throw new CloudRuntimeException("Backup doesn't contain service offering uuid. Please specify a valid service offering id while creating the instance");
9609+
throw new CloudRuntimeException("Backup doesn't contain a Service Offering UUID. Please specify a valid Service Offering while creating the Instance");
96109610
}
96119611
serviceOffering = serviceOfferingDao.findByUuid(serviceOfferingUuid);
96129612
if (serviceOffering == null) {
9613-
throw new CloudRuntimeException("Unable to find service offering with the uuid stored in backup. Please specify a valid service offering id while creating instance");
9613+
throw new CloudRuntimeException("Unable to find Service Offering with the UUID stored in the Backup. Please specify a valid Service Offering while creating the Instance");
96149614
}
96159615
}
96169616
verifyServiceOffering(cmd, serviceOffering);
@@ -9625,11 +9625,11 @@ public UserVm allocateVMFromBackup(CreateVMFromBackupCmd cmd) throws Insufficien
96259625
} else {
96269626
String templateUuid = backup.getDetail(ApiConstants.TEMPLATE_ID);
96279627
if (templateUuid == null) {
9628-
throw new CloudRuntimeException("Backup doesn't contain Template uuid. Please specify a valid Template/ISO while creating the instance");
9628+
throw new CloudRuntimeException("Backup doesn't contain a Template UUID. Please specify a valid Template/ISO while creating the Instance");
96299629
}
96309630
template = _templateDao.findByUuid(templateUuid);
96319631
if (template == null) {
9632-
throw new CloudRuntimeException("Unable to find template associated with the backup. Please specify a valid Template/ISO while creating instance");
9632+
throw new CloudRuntimeException("Unable to find Template with the UUID stored in the Backup. Please specify a valid Template/ISO while creating the Instance");
96339633
}
96349634
}
96359635
verifyTemplate(cmd, template, serviceOffering.getId());

‎server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java‎

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -122,12 +122,14 @@
122122
import com.cloud.serializer.GsonHelper;
123123
import com.cloud.service.dao.ServiceOfferingDao;
124124
import com.cloud.storage.DiskOfferingVO;
125+
import com.cloud.storage.GuestOSVO;
125126
import com.cloud.storage.ScopeType;
126127
import com.cloud.storage.Storage;
127128
import com.cloud.storage.Volume;
128129
import com.cloud.storage.VolumeApiService;
129130
import com.cloud.storage.VolumeVO;
130131
import com.cloud.storage.dao.DiskOfferingDao;
132+
import com.cloud.storage.dao.GuestOSDao;
131133
import com.cloud.storage.dao.VMTemplateDao;
132134
import com.cloud.storage.dao.VolumeDao;
133135
import com.cloud.template.VirtualMachineTemplate;
@@ -232,6 +234,8 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager {
232234
private ResourceLimitService resourceLimitMgr;
233235
@Inject
234236
private AlertManager alertManager;
237+
@Inject
238+
private GuestOSDao _guestOSDao;
235239

236240
private AsyncJobDispatcher asyncJobDispatcher;
237241
private Timer backupTimer;
@@ -379,7 +383,15 @@ public Map<String, String> getBackupDetailsFromVM(VirtualMachine vm) {
379383
ServiceOffering serviceOffering = serviceOfferingDao.findById(vm.getServiceOfferingId());
380384
details.put(ApiConstants.SERVICE_OFFERING_ID, serviceOffering.getUuid());
381385
VirtualMachineTemplate template = vmTemplateDao.findById(vm.getTemplateId());
382-
details.put(ApiConstants.TEMPLATE_ID, template.getUuid());
386+
if (template != null) {
387+
long guestOSId = template.getGuestOSId();
388+
details.put(ApiConstants.TEMPLATE_ID, template.getUuid());
389+
GuestOSVO guestOS = _guestOSDao.findById(guestOSId);
390+
if (guestOS != null) {
391+
details.put(ApiConstants.OS_TYPE_ID, guestOS.getUuid());
392+
details.put(ApiConstants.OS_NAME, guestOS.getDisplayName());
393+
}
394+
}
383395

384396
List<VMInstanceDetailVO> vmDetails = vmInstanceDetailsDao.listDetails(vm.getId());
385397
HashMap<String, String> settings = new HashMap<>();
@@ -2143,15 +2155,13 @@ Map<String, String> getDetailsFromBackupDetails(Long backupId) {
21432155
if (details.containsKey(ApiConstants.TEMPLATE_ID)) {
21442156
VirtualMachineTemplate template = vmTemplateDao.findByUuid(details.get(ApiConstants.TEMPLATE_ID));
21452157
if (template != null) {
2146-
details.put(ApiConstants.TEMPLATE_ID, template.getUuid());
21472158
details.put(ApiConstants.TEMPLATE_NAME, template.getName());
21482159
details.put(ApiConstants.IS_ISO, String.valueOf(template.getFormat().equals(Storage.ImageFormat.ISO)));
21492160
}
21502161
}
21512162
if (details.containsKey(ApiConstants.SERVICE_OFFERING_ID)) {
21522163
ServiceOffering serviceOffering = serviceOfferingDao.findByUuid(details.get(ApiConstants.SERVICE_OFFERING_ID));
21532164
if (serviceOffering != null) {
2154-
details.put(ApiConstants.SERVICE_OFFERING_ID, serviceOffering.getUuid());
21552165
details.put(ApiConstants.SERVICE_OFFERING_NAME, serviceOffering.getName());
21562166
}
21572167
}
@@ -2186,10 +2196,15 @@ public BackupResponse createBackupResponse(Backup backup, Boolean listVmDetails)
21862196
response.setId(backup.getUuid());
21872197
response.setName(backup.getName());
21882198
response.setDescription(backup.getDescription());
2189-
response.setVmName(vm.getHostName());
2190-
response.setVmId(vm.getUuid());
2191-
if (vm.getBackupOfferingId() == null || vm.getBackupOfferingId() != backup.getBackupOfferingId()) {
2192-
response.setVmOfferingRemoved(true);
2199+
if (vm != null) {
2200+
response.setVmName(vm.getHostName());
2201+
response.setVmId(vm.getUuid());
2202+
if (vm.getBackupOfferingId() == null || vm.getBackupOfferingId() != backup.getBackupOfferingId()) {
2203+
response.setVmOfferingRemoved(true);
2204+
}
2205+
}
2206+
if (vm == null || VirtualMachine.State.Expunging.equals(vm.getState())) {
2207+
response.setVmExpunged(true);
21932208
}
21942209
response.setExternalId(backup.getExternalId());
21952210
response.setType(backup.getType());
@@ -2205,9 +2220,11 @@ public BackupResponse createBackupResponse(Backup backup, Boolean listVmDetails)
22052220
}
22062221
}
22072222
// ACS 4.20: For backups taken prior this release the backup.backed_volumes column would be empty hence use vm_instance.backup_volumes
2208-
String backedUpVolumes;
2223+
String backedUpVolumes = "";
22092224
if (Objects.isNull(backup.getBackedUpVolumes())) {
2210-
backedUpVolumes = new Gson().toJson(vm.getBackupVolumeList().toArray(), Backup.VolumeInfo[].class);
2225+
if (vm != null) {
2226+
backedUpVolumes = new Gson().toJson(vm.getBackupVolumeList().toArray(), Backup.VolumeInfo[].class);
2227+
}
22112228
} else {
22122229
backedUpVolumes = new Gson().toJson(backup.getBackedUpVolumes().toArray(), Backup.VolumeInfo[].class);
22132230
}
@@ -2223,7 +2240,9 @@ public BackupResponse createBackupResponse(Backup backup, Boolean listVmDetails)
22232240

22242241
if (Boolean.TRUE.equals(listVmDetails)) {
22252242
Map<String, String> vmDetails = new HashMap<>();
2226-
vmDetails.put(ApiConstants.HYPERVISOR, vm.getHypervisorType().toString());
2243+
if (vm != null) {
2244+
vmDetails.put(ApiConstants.HYPERVISOR, vm.getHypervisorType().toString());
2245+
}
22272246
Map<String, String> details = getDetailsFromBackupDetails(backup.getId());
22282247
vmDetails.putAll(details);
22292248
response.setVmDetails(vmDetails);

‎server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import com.cloud.storage.VolumeApiService;
5050
import com.cloud.storage.VolumeVO;
5151
import com.cloud.storage.dao.DiskOfferingDao;
52+
import com.cloud.storage.dao.GuestOSDao;
5253
import com.cloud.storage.dao.VMTemplateDao;
5354
import com.cloud.storage.dao.VolumeDao;
5455
import com.cloud.user.Account;
@@ -237,6 +238,9 @@ public class BackupManagerTest {
237238
@Mock
238239
DomainDao domainDao;
239240

241+
@Mock
242+
private GuestOSDao _guestOSDao;
243+
240244
private Gson gson;
241245

242246
private String[] hostPossibleValues = {"127.0.0.1", "hostname"};
@@ -1572,14 +1576,12 @@ public void testNewBackupResponse() {
15721576

15731577
VMTemplateVO template = mock(VMTemplateVO.class);
15741578
when(template.getFormat()).thenReturn(Storage.ImageFormat.QCOW2);
1575-
when(template.getUuid()).thenReturn(templateUuid);
15761579
when(template.getName()).thenReturn("template1");
15771580
when(vmTemplateDao.findByUuid(templateUuid)).thenReturn(template);
15781581
Map<String, String> details = new HashMap<>();
15791582
details.put(ApiConstants.TEMPLATE_ID, templateUuid);
15801583

15811584
ServiceOfferingVO serviceOffering = mock(ServiceOfferingVO.class);
1582-
when(serviceOffering.getUuid()).thenReturn(serviceOfferingUuid);
15831585
when(serviceOffering.getName()).thenReturn("service-offering1");
15841586
when(serviceOfferingDao.findByUuid(serviceOfferingUuid)).thenReturn(serviceOffering);
15851587
details.put(ApiConstants.SERVICE_OFFERING_ID, serviceOfferingUuid);

0 commit comments

Comments
 (0)