-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Allow altering only either CPU or memory during VM live scale #8234
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
83af447
e817a59
d05f25f
17f64f4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -400,7 +400,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir | |
| @Inject | ||
| private VMTemplateZoneDao _templateZoneDao; | ||
| @Inject | ||
| private TemplateDataStoreDao _templateStoreDao; | ||
| protected TemplateDataStoreDao _templateStoreDao; | ||
| @Inject | ||
| private DomainDao _domainDao; | ||
| @Inject | ||
|
|
@@ -1227,6 +1227,39 @@ public UserVm upgradeVirtualMachine(UpgradeVMCmd cmd) throws ResourceAllocationE | |
| return userVm; | ||
| } | ||
|
|
||
| /** | ||
| Updates the instance details map with the current values of the instance for the CPU speed, memory, and CPU number if they have not been specified. | ||
| @param details Map containing the instance details. | ||
| @param vmInstance The virtual machine instance. | ||
| @param newServiceOfferingId The ID of the new service offering. | ||
| */ | ||
|
|
||
| protected void updateInstanceDetails (Map<String, String> details, VirtualMachine vmInstance, Long newServiceOfferingId) { | ||
| ServiceOfferingVO currentServiceOffering = serviceOfferingDao.findByIdIncludingRemoved(vmInstance.getId(), vmInstance.getServiceOfferingId()); | ||
| ServiceOfferingVO newServiceOffering = serviceOfferingDao.findById(newServiceOfferingId); | ||
| updateInstanceDetailsKeepCurrentValueIfNull(newServiceOffering.getSpeed(), details, VmDetailConstants.CPU_SPEED, currentServiceOffering.getSpeed()); | ||
| updateInstanceDetailsKeepCurrentValueIfNull(newServiceOffering.getRamSize(), details, VmDetailConstants.MEMORY, currentServiceOffering.getRamSize()); | ||
| updateInstanceDetailsKeepCurrentValueIfNull(newServiceOffering.getCpu(), details, VmDetailConstants.CPU_NUMBER, currentServiceOffering.getCpu()); | ||
| } | ||
|
|
||
| /** | ||
| * Updates a specific instance detail with the current instance value if the new value is null. | ||
| * | ||
| * @param newValue the new value to be set | ||
| * @param details a map of instance details | ||
| * @param detailsConstant the name of the detail constant to be updated | ||
| * @param currentValue the current value of the detail constant | ||
| */ | ||
|
|
||
| protected void updateInstanceDetailsKeepCurrentValueIfNull(Integer newValue, Map<String, String> details, String detailsConstant, Integer currentValue) { | ||
| if (newValue == null && details.get(detailsConstant) == null) { | ||
| String currentValueString = String.valueOf(currentValue); | ||
| s_logger.debug(String.format("[%s] was not specified, keeping the current value: [%s].", detailsConstant, currentValueString)); | ||
| details.put(detailsConstant, currentValueString); | ||
| } | ||
| } | ||
|
|
||
|
|
||
| private void validateOfferingMaxResource(ServiceOfferingVO offering) { | ||
| Integer maxCPUCores = ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_CPU_CORES.value() == 0 ? Integer.MAX_VALUE: ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_CPU_CORES.value(); | ||
| if (offering.getCpu() > maxCPUCores) { | ||
|
|
@@ -1892,7 +1925,11 @@ public UserVm upgradeVirtualMachine(ScaleVMCmd cmd) throws ResourceUnavailableEx | |
| } | ||
| CallContext.current().setEventDetails("Vm Id: " + vm.getUuid()); | ||
|
|
||
| boolean result = upgradeVirtualMachine(vmId, newServiceOfferingId, cmd.getDetails()); | ||
| Map<String, String> cmdDetails = cmd.getDetails(); | ||
|
|
||
| updateInstanceDetails(cmdDetails, vm, newServiceOfferingId); | ||
|
|
||
| boolean result = upgradeVirtualMachine(vmId, newServiceOfferingId, cmdDetails); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry for late review. @GaOrtiga ping @DaanHoogland @GutoVeronezi , anyone has tested it ?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, it was tested.
The PR does not address changes to this behavior. it remains the same; we can discuss it in an issue or another PR.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ok, I asked it because I did not see a comment mentioning it has been manually tested ok. If not, it would be good to be manually tested.
Do you think it is a regression of this PR we should address?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I don't think I understood your question.
The PR does not address changes to method
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Sorry typo.
please ignore my question regarding the vm details, They are not saved in database before upgrade, so no need to restore.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I tested it; I do not know if anybody else has tested it too.
Yes. The only thing this PR does is facilitate calling the API. For instance, without this PR, if one has a VM with 1 vCPU and 1 GB RAM and wants to scale it to 2 GB RAM, one has to call the API with both the current vCPU count and the new RAM size. With this PR, one will only have to pass the new RAM size in the parameters; ACS will retrieve the current VM's vCPU count and will add it to the I created PR #8673 to improve the methods names and docs. |
||
| if (result) { | ||
| UserVmVO vmInstance = _vmDao.findById(vmId); | ||
| if (vmInstance.getState().equals(State.Stopped)) { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.