GutoVeronezi commented on code in PR #6899:
URL: https://github.com/apache/cloudstack/pull/6899#discussion_r1021929299
##########
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##########
@@ -1047,6 +1049,13 @@ public void orchestrateStart(final String vmUuid, final
Map<VirtualMachineProfil
resourceCountIncrement(owner.getAccountId(),new
Long(offering.getCpu()), new Long(offering.getRamSize()));
}
+ // Increment the VR resource count if the global setting is set to true
+ // and resource.count.running.vms is also true
+ if (VirtualMachine.Type.DomainRouter.equals(vm.type) &&
+ ResourceCountRouters.valueIn(owner.getDomainId())) {
Review Comment:
As this statement is repeated along the code, it could be turned to a method.
##########
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##########
@@ -1351,6 +1360,12 @@ public void orchestrateStart(final String vmUuid, final
Map<VirtualMachineProfil
if (VirtualMachine.Type.User.equals(vm.type) &&
ResourceCountRunningVMsonly.value()) {
resourceCountDecrement(owner.getAccountId(),new
Long(offering.getCpu()), new Long(offering.getRamSize()));
}
+ // Decrement VR resource count if the VR can't be started
Review Comment:
As the comment was intended to be a literal description of the following
line, it is not necessary.
##########
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##########
@@ -1047,6 +1049,13 @@ public void orchestrateStart(final String vmUuid, final
Map<VirtualMachineProfil
resourceCountIncrement(owner.getAccountId(),new
Long(offering.getCpu()), new Long(offering.getRamSize()));
}
+ // Increment the VR resource count if the global setting is set to true
+ // and resource.count.running.vms is also true
Review Comment:
As the comment was intended to be a literal description of the following
line, it is not necessary.
##########
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##########
@@ -1398,6 +1413,131 @@ private void
logBootModeParameters(Map<VirtualMachineProfile.Param, Object> para
}
}
+ /**
+ * Function to increment the VR resource count
+ *
+ * If the global setting resource.count.router is true then the VR
+ * resource count will be considered as well
+ * If the global setting resource.count.router.type is "all" then
+ * all VR resource count will be considered else the diff between
+ * the current VR service offering and the default offering will
+ * be considered
+ *
+ * @param offering
+ * @param owner
+ * @param isDeployOrDestroy
+ */
+ @Override
+ public void incrementVrResourceCount(ServiceOffering offering,
+ Account owner,
+ boolean isDeployOrDestroy) {
+ // During router deployment/destroy, we increment the resource
+ // count only if resource.count.running.vms is false else
+ // we increment it during VR start/stop. Same applies for
+ // decrementing resource count.
+ if (isDeployOrDestroy == ResourceCountRunningVMsonly.value()) {
+ return;
+ }
+ ServiceOffering defaultRouterOffering = null;
+ String globalRouterOffering =
_configDao.getValue("router.service.offering");
+ if (globalRouterOffering != null) {
+ defaultRouterOffering =
_serviceOfferingDao.findByUuid(globalRouterOffering);
+ }
+
+ if (defaultRouterOffering == null) {
+ defaultRouterOffering =
_serviceOfferingDao.findByName(ServiceOffering.routerDefaultOffUniqueName);
+ }
+
+ int cpuCount = 0;
+ int memoryCount = 0;
+ // Count VR resources for domain if global setting is true
+ // if value is "all" count all VR resources else get diff of
+ // current VR offering and default VR offering
+ if (ResourceCountRoutersType.valueIn(owner.getDomainId())
+ .equalsIgnoreCase(COUNT_ALL_VR_RESOURCES)) {
+ cpuCount = offering.getCpu();
+ memoryCount = offering.getRamSize();
+ } else if (ResourceCountRoutersType.valueIn(owner.getDomainId())
+ .equalsIgnoreCase(COUNT_DELTA_VR_RESOURCES)) {
+ // Default offering value can be greater than current offering
value
+ if (offering.getCpu() >= defaultRouterOffering.getCpu()) {
+ cpuCount = offering.getCpu() - defaultRouterOffering.getCpu();
+ }
+ if (offering.getRamSize() >= defaultRouterOffering.getRamSize()) {
+ memoryCount = offering.getRamSize() -
defaultRouterOffering.getRamSize();
+ }
+ }
+
+ // Check if resource count can be allocated to account/domain
+ try {
+ if (cpuCount > 0) {
+ _resourceLimitMgr.checkResourceLimit(owner, ResourceType.cpu,
cpuCount);
+ }
+ if (memoryCount > 0) {
+ _resourceLimitMgr.checkResourceLimit(owner,
ResourceType.memory, memoryCount);
+ }
+
+ } catch (ResourceAllocationException ex) {
+ throw new CloudRuntimeException("Unable to deploy/start routers
due to " + ex.getMessage());
+ }
+
+ // Increment the resource count
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug("Incrementing the CPU count with value " + cpuCount
+ " and " +
+ "RAM value with " + memoryCount);
+ }
+ _resourceLimitMgr.incrementResourceCount(owner.getAccountId(),
ResourceType.cpu, new Long(cpuCount));
+ _resourceLimitMgr.incrementResourceCount(owner.getAccountId(),
ResourceType.memory, new Long(memoryCount));
Review Comment:
This method can be separated in smaller methods, with the comments being
turned into javadocs. It would be more readable and easier to add unit tests.
##########
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##########
@@ -1398,6 +1413,131 @@ private void
logBootModeParameters(Map<VirtualMachineProfile.Param, Object> para
}
}
+ /**
+ * Function to increment the VR resource count
+ *
+ * If the global setting resource.count.router is true then the VR
+ * resource count will be considered as well
+ * If the global setting resource.count.router.type is "all" then
+ * all VR resource count will be considered else the diff between
+ * the current VR service offering and the default offering will
+ * be considered
+ *
+ * @param offering
+ * @param owner
+ * @param isDeployOrDestroy
+ */
+ @Override
+ public void incrementVrResourceCount(ServiceOffering offering,
+ Account owner,
+ boolean isDeployOrDestroy) {
+ // During router deployment/destroy, we increment the resource
+ // count only if resource.count.running.vms is false else
+ // we increment it during VR start/stop. Same applies for
+ // decrementing resource count.
+ if (isDeployOrDestroy == ResourceCountRunningVMsonly.value()) {
+ return;
+ }
+ ServiceOffering defaultRouterOffering = null;
+ String globalRouterOffering =
_configDao.getValue("router.service.offering");
+ if (globalRouterOffering != null) {
+ defaultRouterOffering =
_serviceOfferingDao.findByUuid(globalRouterOffering);
+ }
+
+ if (defaultRouterOffering == null) {
+ defaultRouterOffering =
_serviceOfferingDao.findByName(ServiceOffering.routerDefaultOffUniqueName);
+ }
+
+ int cpuCount = 0;
+ int memoryCount = 0;
+ // Count VR resources for domain if global setting is true
+ // if value is "all" count all VR resources else get diff of
+ // current VR offering and default VR offering
+ if (ResourceCountRoutersType.valueIn(owner.getDomainId())
+ .equalsIgnoreCase(COUNT_ALL_VR_RESOURCES)) {
+ cpuCount = offering.getCpu();
+ memoryCount = offering.getRamSize();
+ } else if (ResourceCountRoutersType.valueIn(owner.getDomainId())
+ .equalsIgnoreCase(COUNT_DELTA_VR_RESOURCES)) {
+ // Default offering value can be greater than current offering
value
+ if (offering.getCpu() >= defaultRouterOffering.getCpu()) {
+ cpuCount = offering.getCpu() - defaultRouterOffering.getCpu();
+ }
+ if (offering.getRamSize() >= defaultRouterOffering.getRamSize()) {
+ memoryCount = offering.getRamSize() -
defaultRouterOffering.getRamSize();
+ }
+ }
+
+ // Check if resource count can be allocated to account/domain
+ try {
+ if (cpuCount > 0) {
+ _resourceLimitMgr.checkResourceLimit(owner, ResourceType.cpu,
cpuCount);
+ }
+ if (memoryCount > 0) {
+ _resourceLimitMgr.checkResourceLimit(owner,
ResourceType.memory, memoryCount);
+ }
+
+ } catch (ResourceAllocationException ex) {
+ throw new CloudRuntimeException("Unable to deploy/start routers
due to " + ex.getMessage());
+ }
+
+ // Increment the resource count
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug("Incrementing the CPU count with value " + cpuCount
+ " and " +
+ "RAM value with " + memoryCount);
+ }
+ _resourceLimitMgr.incrementResourceCount(owner.getAccountId(),
ResourceType.cpu, new Long(cpuCount));
+ _resourceLimitMgr.incrementResourceCount(owner.getAccountId(),
ResourceType.memory, new Long(memoryCount));
+ }
+
+ /**
+ * Function to decrement the VR resource count
+ *
+ * @param offering
+ * @param owner
+ * @param isDeployOrDestroy
+ */
+ @Override
+ public void decrementVrResourceCount(ServiceOffering offering,
+ Account owner,
+ boolean isDeployOrDestroy) {
+ if (isDeployOrDestroy == ResourceCountRunningVMsonly.value()) {
+ return;
+ }
+
+ ServiceOffering defaultRouterOffering = null;
+ String globalRouterOffering =
_configDao.getValue("router.service.offering");
+ if (globalRouterOffering != null) {
+ defaultRouterOffering =
_serviceOfferingDao.findByUuid(globalRouterOffering);
+ }
+ if (defaultRouterOffering == null) {
+ defaultRouterOffering =
_serviceOfferingDao.findByName(ServiceOffering.routerDefaultOffUniqueName);
+ }
+
+ int cpuCount = 0;
+ int memoryCount = 0;
+
+ // Since we already incremented the resource count for the domain,
+ // decrement it if the VR can't be started
+ if (ResourceCountRoutersType.valueIn(owner.getDomainId())
+ .equalsIgnoreCase(COUNT_ALL_VR_RESOURCES)) {
+ cpuCount = offering.getCpu();
+ memoryCount = offering.getRamSize();
+ } else if (ResourceCountRoutersType.valueIn(owner.getDomainId())
+ .equalsIgnoreCase(COUNT_DELTA_VR_RESOURCES)) {
+ cpuCount = offering.getCpu() - defaultRouterOffering.getCpu();
+ memoryCount = offering.getRamSize() -
defaultRouterOffering.getRamSize();
+ }
+
+ // Decrement resource count if flag is true
+ if (s_logger.isDebugEnabled()) {
+ s_logger.error("Decrementing cpu resource count with value " +
cpuCount +
+ " and memory resource with value " + memoryCount);
+ }
+ _resourceLimitMgr.decrementResourceCount(owner.getAccountId(),
ResourceType.cpu, new Long(cpuCount));
+ _resourceLimitMgr.decrementResourceCount(owner.getAccountId(),
ResourceType.memory, new Long(memoryCount));
+ }
Review Comment:
This method can be separated in smaller methods, with the comments being
turned into javadocs. It would be more readable and easier to add unit tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
