mirror of
				https://github.com/apache/cloudstack.git
				synced 2025-10-26 08:42:29 +01:00 
			
		
		
		
	* When scaling with custom offering, which changes only CPU/Memory and keeps same disk offering an exception is thrown. This commit fixes such cases by checking if the operation is happening on a custom service offering. * Improve the unit tests that cover null objects.
This commit is contained in:
		
							parent
							
								
									fecc5254de
								
							
						
					
					
						commit
						50b2dc2789
					
				| @ -1705,6 +1705,12 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic | |||||||
|         boolean volumeMigrateRequired = false; |         boolean volumeMigrateRequired = false; | ||||||
|         boolean volumeResizeRequired = false; |         boolean volumeResizeRequired = false; | ||||||
| 
 | 
 | ||||||
|  |         // Skip the Disk offering change on Volume if Disk Offering is the same and is linked to a custom Service Offering | ||||||
|  |         if(isNewDiskOfferingTheSameAndCustomServiceOffering(existingDiskOffering, newDiskOffering)) { | ||||||
|  |             s_logger.debug(String.format("Scaling CPU and/or Memory of VM with custom service offering. New disk offering stills the same. Skipping the Disk offering change on Volume %s.", volume.getUuid())); | ||||||
|  |             return volume; | ||||||
|  |         } | ||||||
|  | 
 | ||||||
|         // VALIDATIONS |         // VALIDATIONS | ||||||
|         Long[] updateNewSize = {newSize}; |         Long[] updateNewSize = {newSize}; | ||||||
|         Long[] updateNewMinIops = {newMinIops}; |         Long[] updateNewMinIops = {newMinIops}; | ||||||
| @ -1796,6 +1802,19 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic | |||||||
|         return volume; |         return volume; | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|  |     /** | ||||||
|  |      * Returns true if the new disk offering is the same than current offering, and the respective Service offering is a custom (constraint or unconstraint) offering. | ||||||
|  |      */ | ||||||
|  |     protected boolean isNewDiskOfferingTheSameAndCustomServiceOffering(DiskOfferingVO existingDiskOffering, DiskOfferingVO newDiskOffering) { | ||||||
|  |         if (newDiskOffering.getId() == existingDiskOffering.getId()) { | ||||||
|  |             ServiceOfferingVO serviceOffering = _serviceOfferingDao.findServiceOfferingByComputeOnlyDiskOffering(newDiskOffering.getId()); | ||||||
|  |             if (serviceOffering != null && serviceOffering.isCustomized()) { | ||||||
|  |                 return true; | ||||||
|  |             } | ||||||
|  |         } | ||||||
|  |         return false; | ||||||
|  |     } | ||||||
|  | 
 | ||||||
|     private VolumeVO resizeVolumeInternal(VolumeVO volume, DiskOfferingVO newDiskOffering, Long currentSize, Long newSize, Long newMinIops, Long newMaxIops, Integer newHypervisorSnapshotReserve, boolean shrinkOk) throws ResourceAllocationException { |     private VolumeVO resizeVolumeInternal(VolumeVO volume, DiskOfferingVO newDiskOffering, Long currentSize, Long newSize, Long newMinIops, Long newMaxIops, Integer newHypervisorSnapshotReserve, boolean shrinkOk) throws ResourceAllocationException { | ||||||
|         UserVmVO userVm = _userVmDao.findById(volume.getInstanceId()); |         UserVmVO userVm = _userVmDao.findById(volume.getInstanceId()); | ||||||
|         HypervisorType hypervisorType = _volsDao.getHypervisorType(volume.getId()); |         HypervisorType hypervisorType = _volsDao.getHypervisorType(volume.getId()); | ||||||
| @ -1896,11 +1915,11 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic | |||||||
|         } |         } | ||||||
| 
 | 
 | ||||||
|         if (newDiskOffering.getId() == existingDiskOffering.getId()) { |         if (newDiskOffering.getId() == existingDiskOffering.getId()) { | ||||||
|             throw new InvalidParameterValueException(String.format("Volume %s already have the new disk offering %s provided", volume.getUuid(), existingDiskOffering.getUuid())); |             throw new InvalidParameterValueException(String.format("Volume %s already have the new disk offering %s provided.", volume.getUuid(), existingDiskOffering.getUuid())); | ||||||
|         } |         } | ||||||
| 
 | 
 | ||||||
|         if (existingDiskOffering.getDiskSizeStrictness() != newDiskOffering.getDiskSizeStrictness()) { |         if (existingDiskOffering.getDiskSizeStrictness() != newDiskOffering.getDiskSizeStrictness()) { | ||||||
|             throw new InvalidParameterValueException("Disk offering size strictness does not match with new disk offering"); |             throw new InvalidParameterValueException("Disk offering size strictness does not match with new disk offering."); | ||||||
|         } |         } | ||||||
| 
 | 
 | ||||||
|         if (MatchStoragePoolTagsWithDiskOffering.valueIn(volume.getDataCenterId())) { |         if (MatchStoragePoolTagsWithDiskOffering.valueIn(volume.getDataCenterId())) { | ||||||
|  | |||||||
| @ -1156,4 +1156,48 @@ public class VolumeApiServiceImplTest { | |||||||
|         boolean result = volumeApiServiceImpl.isNotPossibleToResize(volume, diskOffering); |         boolean result = volumeApiServiceImpl.isNotPossibleToResize(volume, diskOffering); | ||||||
|         Assert.assertEquals(expectedIsNotPossibleToResize, result); |         Assert.assertEquals(expectedIsNotPossibleToResize, result); | ||||||
|     } |     } | ||||||
|  | 
 | ||||||
|  |     @Test | ||||||
|  |     public void isNewDiskOfferingTheSameAndCustomServiceOfferingTestDifferentOfferings() { | ||||||
|  |         prepareAndRunIsNewDiskOfferingTheSameAndCustomServiceOffering(1l, 2l, false, true, false); | ||||||
|  |     } | ||||||
|  | 
 | ||||||
|  |     @Test | ||||||
|  |     public void isNewDiskOfferingTheSameAndCustomServiceOfferingTestDifferentOfferingsCustom() { | ||||||
|  |         prepareAndRunIsNewDiskOfferingTheSameAndCustomServiceOffering(1l, 2l, true, true, false); | ||||||
|  |     } | ||||||
|  | 
 | ||||||
|  |     @Test | ||||||
|  |     public void isNewDiskOfferingTheSameAndCustomServiceOfferingTestSameOfferingsCustom() { | ||||||
|  |         prepareAndRunIsNewDiskOfferingTheSameAndCustomServiceOffering(1l, 1l, true, true, true); | ||||||
|  |     } | ||||||
|  | 
 | ||||||
|  |     @Test | ||||||
|  |     public void isNewDiskOfferingTheSameAndCustomServiceOfferingTestSameOfferingsNotCustom() { | ||||||
|  |         prepareAndRunIsNewDiskOfferingTheSameAndCustomServiceOffering(1l, 1l, false, true, false); | ||||||
|  |     } | ||||||
|  | 
 | ||||||
|  |     @Test | ||||||
|  |     public void isNewDiskOfferingTheSameAndCustomServiceOfferingTestDifferentOfferingsAndNullOffering() { | ||||||
|  |         prepareAndRunIsNewDiskOfferingTheSameAndCustomServiceOffering(1l, 2l, true, false, false); | ||||||
|  |     } | ||||||
|  |     @Test | ||||||
|  |     public void isNewDiskOfferingTheSameAndCustomServiceOfferingTestSameOfferingsNullOffering() { | ||||||
|  |         prepareAndRunIsNewDiskOfferingTheSameAndCustomServiceOffering(1l, 1l, false, false, false); | ||||||
|  |     } | ||||||
|  | 
 | ||||||
|  |     private void prepareAndRunIsNewDiskOfferingTheSameAndCustomServiceOffering(long existingDiskOfferingId, long newDiskOfferingId, boolean isCustomized, | ||||||
|  |             boolean isNotNullServiceOffering, boolean expectedResult) { | ||||||
|  |         DiskOfferingVO existingDiskOffering = Mockito.mock(DiskOfferingVO.class); | ||||||
|  |         when(existingDiskOffering.getId()).thenReturn(existingDiskOfferingId); | ||||||
|  |         DiskOfferingVO newDiskOffering = Mockito.mock(DiskOfferingVO.class); | ||||||
|  |         when(newDiskOffering.getId()).thenReturn(newDiskOfferingId); | ||||||
|  |         if(isNotNullServiceOffering) { | ||||||
|  |             ServiceOfferingVO serviceOfferingVO = Mockito.mock(ServiceOfferingVO.class); | ||||||
|  |             when(serviceOfferingVO.isCustomized()).thenReturn(isCustomized); | ||||||
|  |             when(serviceOfferingDao.findServiceOfferingByComputeOnlyDiskOffering(anyLong())).thenReturn(serviceOfferingVO); | ||||||
|  |         } | ||||||
|  |         boolean result = volumeApiServiceImpl.isNewDiskOfferingTheSameAndCustomServiceOffering(existingDiskOffering, newDiskOffering); | ||||||
|  |         Assert.assertEquals(expectedResult, result); | ||||||
|  |     } | ||||||
| } | } | ||||||
|  | |||||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user