You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/02/22 17:26:53 UTC
[GitHub] [cloudstack] GutoVeronezi opened a new pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
GutoVeronezi opened a new pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722
### Description
When ACS is deploying a VM, it defines a minimum memory|CPU to the VM based on a calculation through the overprovisioning (defined mem|cpu speed / overprovisioning factor); Therefore, it will define a range of memory|CPU, even if it does not use scalable (dynamic) service offerings.
This PR intends to externalize 2 cluster's configurations to allow operators to decide if they want the minimum memory|CPU to be different from the allocated value when the overprovisioning's configurations are different than `1`.
When we set the overprovisioning factor, it means that we (as operators) are willing to allocate more virtual resources than the physical ones. This is interesting when the hypervisor has optimization techniques such as the KVM ones for optimizing the allocation of common blocks among running virtual machines (Kernel Samepage Merging - KSM). Therefore, we do not need to reduce the minimum amount of memory that the VM must receive when the overprovisioning is higher than one. This is important for public cloud environments, where such definition can be perceived by final users.
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [ ] Major
- [x] Minor
### How Has This Been Tested?
It has been tested locally in a test lab.
1. I had created VM and observed the `dumpxml`.
2. When I was using the configuration as `true`, it kept the same behavior (allocated memory <> currentMemory).
3. When I was using the configuration as `false`, it took the current memory equals the allocated memory.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#issuecomment-834324418
@GabrielBrascher a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#issuecomment-850053061
This one looks ready to merge
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#issuecomment-809771812
<b>Trillian test result (tid-278)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 49122 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4722-t278-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_nested_virtualization.py
Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
Intermittent failure detected: /marvin/tests/smoke/test_projects.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Smoke tests completed. 84 look OK, 2 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 72.09 | test_kubernetes_clusters.py
test_01_migrate_VM_and_root_volume | `Error` | 91.37 | test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks | `Error` | 103.67 | test_vm_life_cycle.py
test_08_migrate_vm | `Failure` | 49.45 | test_vm_life_cycle.py
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#issuecomment-808348706
@blueorangutan package
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#issuecomment-808349959
@GutoVeronezi a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#issuecomment-808345563
We found another process that need to use these configs to do the calculation.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi edited a comment on pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
GutoVeronezi edited a comment on pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#issuecomment-806027891
@GabrielBrascher @DaanHoogland
Is this PR OK? Is there anything else to do to merge it?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#issuecomment-807909215
<b>Trillian test result (tid-267)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35853 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4722-t267-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Smoke tests completed. 84 look OK, 2 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 73.09 | test_kubernetes_clusters.py
test_01_migrate_VM_and_root_volume | `Error` | 67.16 | test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks | `Error` | 53.17 | test_vm_life_cycle.py
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#issuecomment-822261080
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 434
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#issuecomment-834416297
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 534
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#issuecomment-814912980
@DaanHoogland @GabrielBrascher is there anything else to do?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on a change in pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#discussion_r619479420
##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -4564,18 +4574,23 @@ private VMInstanceVO orchestrateReConfigureVm(final String vmUuid, final Service
_capacityMgr.allocateVmCapacity(vm, false); // lock the new capacity
}
- final Answer reconfigureAnswer = _agentMgr.send(vm.getHostId(), reconfigureCmd);
- if (reconfigureAnswer == null || !reconfigureAnswer.getResult()) {
- s_logger.error("Unable to scale vm due to " + (reconfigureAnswer == null ? "" : reconfigureAnswer.getDetails()));
- throw new CloudRuntimeException("Unable to scale vm due to " + (reconfigureAnswer == null ? "" : reconfigureAnswer.getDetails()));
+ Answer scaleVmAnswer = _agentMgr.send(vm.getHostId(), scaleVmCommand);
+ if (scaleVmAnswer == null || !scaleVmAnswer.getResult()) {
+ String msg = String.format("Unable to scale %s due to [%s].", vm.toString(), (scaleVmAnswer == null ? "" : scaleVmAnswer.getDetails()));
+ s_logger.error(msg);
+ throw new CloudRuntimeException(msg);
}
if (vm.getType().equals(VirtualMachine.Type.User)) {
_userVmMgr.generateUsageEvent(vm, vm.isDisplayVm(), EventTypes.EVENT_VM_DYNAMIC_SCALE);
}
success = true;
- } catch (final OperationTimedoutException e) {
- throw new AgentUnavailableException("Operation timed out on reconfiguring " + vm, dstHostId);
- } catch (final AgentUnavailableException e) {
+ } catch (OperationTimedoutException e) {
+ String msg = String.format("Unable to scale %s due to [%s].", vm.toString(), e.getMessage());
+ s_logger.error(msg, e);
+ throw new AgentUnavailableException(msg, dstHostId);
+ } catch (AgentUnavailableException e) {
+ String msg = String.format("Unable to scale %s due to [%s].", vm.toString(), e.getMessage());
+ s_logger.error(msg, e);
throw e;
} finally {
Review comment:
@DaanHoogland lack of attention, perhaps. Done.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#issuecomment-785044841
> @DaanHoogland I was about to do the changes requested by Gabriel, but I saw that you already did it. I would ask you to wait for the contributor to do it, instead of doing the changes on your own. Otherwise, it feels a bit weird for us here on the other side proposing things to contribute back to ACS, and interacting with reviewers to see somebody "taking over the process". The PR was not stale; therefore, it would be only a matter of hours until I had time to address the reviewer request.
makes sense, sorry @GutoVeronezi
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#issuecomment-808367255
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 270
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#discussion_r581876436
##########
File path: server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java
##########
@@ -85,6 +89,14 @@
private ServiceOfferingDao _serviceOfferingDao;
@Inject
private NetworkDetailsDao networkDetailsDao;
+ @Inject
+ private HostDao hostDao;
+
+ static final ConfigKey<Boolean> VmMinMemoryEqualsMemoryDividedByMemOverprovisioningFactor = new ConfigKey<Boolean>("Advanced", Boolean.class, "vm.min.memory.equals.memory.divided.by.mem.overprovisioning.factor", "true",
+ "If we set this to 'true', a minimum memory (memory/ mem.overprovisioning.factor) will be setted to the VM, independent of using a scalable service offering or not.", true, ConfigKey.Scope.Cluster);
Review comment:
```suggestion
"If we set this to 'true', a minimum memory (memory/ mem.overprovisioning.factor) will be set to the VM, independent of using a scalable service offering or not.", true, ConfigKey.Scope.Cluster);
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#issuecomment-847035352
Can anyone review this?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland merged pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
DaanHoogland merged pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#issuecomment-806090326
@GutoVeronezi proof of a succesful smoke test run. I'm arranging that for you.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#issuecomment-785041845
@DaanHoogland I was about to do the changes requested by Gabriel, but I saw that you already did it. I would ask you to wait for the contributor to do it, instead of doing the changes on your own. Otherwise, it feels a bit weird for us here on the other side proposing things to contribute back to ACS, and interacting with reviewers to see somebody "taking over the process". The PR was not stale; therefore, it would be only a matter of hours until I had time to address the reviewer request.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#issuecomment-816979798
<b>Trillian test result (tid-391)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 49114 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4722-t391-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Smoke tests completed. 84 look OK, 2 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_deploy_kubernetes_cluster | `Failure` | 3605.40 | test_kubernetes_clusters.py
test_02_invalid_upgrade_kubernetes_cluster | `Failure` | 3603.58 | test_kubernetes_clusters.py
test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 0.07 | test_kubernetes_clusters.py
test_04_deploy_and_scale_kubernetes_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
test_05_delete_kubernetes_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 39.24 | test_kubernetes_clusters.py
test_01_migrate_VM_and_root_volume | `Error` | 71.20 | test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks | `Error` | 53.14 | test_vm_life_cycle.py
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#issuecomment-825908165
@DaanHoogland done.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#issuecomment-837953767
the kubernetes errors are not related to this code, but we do need more review/testing, anybody...
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#issuecomment-834324034
@blueorangutan package
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#issuecomment-822682779
<b>Trillian test result (tid-482)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 33997 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4722-t482-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
Smoke tests completed. 87 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_03_ssvm_internals | `Failure` | 3.05 | test_ssvm.py
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#issuecomment-837358912
<b>Trillian test result (tid-629)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 44646 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4722-t629-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Smoke tests completed. 87 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_07_deploy_kubernetes_ha_cluster | `Failure` | 3616.59 | test_kubernetes_clusters.py
test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.06 | test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 78.11 | test_kubernetes_clusters.py
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#discussion_r617267935
##########
File path: server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java
##########
@@ -167,8 +179,13 @@ protected void addServiceOfferingExtraConfiguration(ServiceOffering offering, Vi
protected VirtualMachineTO toVirtualMachineTO(VirtualMachineProfile vmProfile) {
ServiceOffering offering = _serviceOfferingDao.findById(vmProfile.getId(), vmProfile.getServiceOfferingId());
VirtualMachine vm = vmProfile.getVirtualMachine();
- Long minMemory = (long)(offering.getRamSize() / vmProfile.getMemoryOvercommitRatio());
- int minspeed = (int)(offering.getSpeed() / vmProfile.getCpuOvercommitRatio());
+ HostVO host = hostDao.findById(vm.getHostId());
+
+ boolean divideMemoryByOverprovisioning = VM_MIN_MEMORY_EQUALS_MEMORY_DIVIDED_BY_MEM_OVERPROVISIONING_FACTOR.valueIn(host.getClusterId());
+ boolean divideCpuByOverprovisioning = VM_MIN_CPU_SPEED_EQUALS_CPU_SPEED_DIVIDED_BY_CPU_OVERPROVISIONING_FACTOR.valueIn(host.getClusterId());
+
Review comment:
```suggestion
boolean divideMemoryByOverprovisioning = VmMinMemoryEqualsMemoryDividedByMemOverprovisioningFactor.valueIn(host.getClusterId());
boolean divideCpuByOverprovisioning = VmMinCpuSpeedEqualsCpuSpeedDividedByCpuOverprovisioningFactor.valueIn(host.getClusterId());
```
##########
File path: server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java
##########
@@ -301,4 +318,15 @@ public boolean attachRestoredVolumeToVirtualMachine(long zoneId, String location
public List<Command> finalizeMigrate(VirtualMachine vm, Map<Volume, StoragePool> volumeToPool) {
return null;
}
+
+ @Override
+ public String getConfigComponentName() {
+ return HypervisorGuruBase.class.getSimpleName();
+ }
+
+ @Override
+ public ConfigKey<?>[] getConfigKeys() {
+ return new ConfigKey<?>[] {VM_MIN_MEMORY_EQUALS_MEMORY_DIVIDED_BY_MEM_OVERPROVISIONING_FACTOR, VM_MIN_CPU_SPEED_EQUALS_CPU_SPEED_DIVIDED_BY_CPU_OVERPROVISIONING_FACTOR };
Review comment:
```suggestion
return new ConfigKey<?>[] {VmMinCpuSpeedEqualsCpuSpeedDividedByCpuOverprovisioningFactor, VmMinCpuSpeedEqualsCpuSpeedDividedByCpuOverprovisioningFactor };
```
##########
File path: server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java
##########
@@ -85,6 +89,14 @@
private ServiceOfferingDao _serviceOfferingDao;
@Inject
private NetworkDetailsDao networkDetailsDao;
+ @Inject
+ private HostDao hostDao;
+
+ public static ConfigKey<Boolean> VM_MIN_MEMORY_EQUALS_MEMORY_DIVIDED_BY_MEM_OVERPROVISIONING_FACTOR = new ConfigKey<Boolean>("Advanced", Boolean.class, "vm.min.memory.equals.memory.divided.by.mem.overprovisioning.factor", "true",
Review comment:
```suggestion
public static ConfigKey<Boolean> VmMinMemoryEqualsMemoryDividedByMemOverprovisioningFactor = new ConfigKey<Boolean>("Advanced", Boolean.class, "vm.min.memory.equals.memory.divided.by.mem.overprovisioning.factor", "true",
```
##########
File path: server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java
##########
@@ -85,6 +89,14 @@
private ServiceOfferingDao _serviceOfferingDao;
@Inject
private NetworkDetailsDao networkDetailsDao;
+ @Inject
+ private HostDao hostDao;
+
+ public static ConfigKey<Boolean> VM_MIN_MEMORY_EQUALS_MEMORY_DIVIDED_BY_MEM_OVERPROVISIONING_FACTOR = new ConfigKey<Boolean>("Advanced", Boolean.class, "vm.min.memory.equals.memory.divided.by.mem.overprovisioning.factor", "true",
+ "If we set this to 'true', a minimum memory (memory/ mem.overprovisioning.factor) will be set to the VM, independent of using a scalable service offering or not.", true, ConfigKey.Scope.Cluster);
+
+ public static ConfigKey<Boolean> VM_MIN_CPU_SPEED_EQUALS_CPU_SPEED_DIVIDED_BY_CPU_OVERPROVISIONING_FACTOR = new ConfigKey<Boolean>("Advanced", Boolean.class, "vm.min.cpu.speed.equals.cpu.speed.divided.by.cpu.overprovisioning.factor", "true",
Review comment:
```suggestion
public static ConfigKey<Boolean> VmMinCpuSpeedEqualsCpuSpeedDividedByCpuOverprovisioningFactor = new ConfigKey<Boolean>("Advanced", Boolean.class, "vm.min.cpu.speed.equals.cpu.speed.divided.by.cpu.overprovisioning.factor", "true",
```
##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -4564,18 +4574,23 @@ private VMInstanceVO orchestrateReConfigureVm(final String vmUuid, final Service
_capacityMgr.allocateVmCapacity(vm, false); // lock the new capacity
}
- final Answer reconfigureAnswer = _agentMgr.send(vm.getHostId(), reconfigureCmd);
- if (reconfigureAnswer == null || !reconfigureAnswer.getResult()) {
- s_logger.error("Unable to scale vm due to " + (reconfigureAnswer == null ? "" : reconfigureAnswer.getDetails()));
- throw new CloudRuntimeException("Unable to scale vm due to " + (reconfigureAnswer == null ? "" : reconfigureAnswer.getDetails()));
+ Answer scaleVmAnswer = _agentMgr.send(vm.getHostId(), scaleVmCommand);
+ if (scaleVmAnswer == null || !scaleVmAnswer.getResult()) {
+ String msg = String.format("Unable to scale %s due to [%s].", vm.toString(), (scaleVmAnswer == null ? "" : scaleVmAnswer.getDetails()));
+ s_logger.error(msg);
+ throw new CloudRuntimeException(msg);
}
if (vm.getType().equals(VirtualMachine.Type.User)) {
_userVmMgr.generateUsageEvent(vm, vm.isDisplayVm(), EventTypes.EVENT_VM_DYNAMIC_SCALE);
}
success = true;
- } catch (final OperationTimedoutException e) {
- throw new AgentUnavailableException("Operation timed out on reconfiguring " + vm, dstHostId);
- } catch (final AgentUnavailableException e) {
+ } catch (OperationTimedoutException e) {
+ String msg = String.format("Unable to scale %s due to [%s].", vm.toString(), e.getMessage());
+ s_logger.error(msg, e);
+ throw new AgentUnavailableException(msg, dstHostId);
+ } catch (AgentUnavailableException e) {
+ String msg = String.format("Unable to scale %s due to [%s].", vm.toString(), e.getMessage());
+ s_logger.error(msg, e);
throw e;
} finally {
Review comment:
log, handle or rethrow, pick one i'd say @GutoVeronezi Why do you print the stack and rethrow?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on a change in pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#discussion_r581497547
##########
File path: server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java
##########
@@ -85,6 +89,14 @@
private ServiceOfferingDao _serviceOfferingDao;
@Inject
private NetworkDetailsDao networkDetailsDao;
+ @Inject
+ private HostDao hostDao;
+
+ static final ConfigKey<Boolean> VmMinMemoryEqualsMemoryDividedByMemOverprovisioningFactor = new ConfigKey<Boolean>("Advanced", Boolean.class, "vm.min.memory.equals.memory.divided.by.mem.overprovisioning.factor", "true",
+ "If we set this to 'true', a minimum memory (memory/ mem.overprovisioning.factor) will be setted to the VM, independent of using a scalable service offering or not.", true, ConfigKey.Scope.Cluster);
Review comment:
Please, change from **setted** to **set**
##########
File path: server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java
##########
@@ -85,6 +89,14 @@
private ServiceOfferingDao _serviceOfferingDao;
@Inject
private NetworkDetailsDao networkDetailsDao;
+ @Inject
+ private HostDao hostDao;
+
+ static final ConfigKey<Boolean> VmMinMemoryEqualsMemoryDividedByMemOverprovisioningFactor = new ConfigKey<Boolean>("Advanced", Boolean.class, "vm.min.memory.equals.memory.divided.by.mem.overprovisioning.factor", "true",
+ "If we set this to 'true', a minimum memory (memory/ mem.overprovisioning.factor) will be setted to the VM, independent of using a scalable service offering or not.", true, ConfigKey.Scope.Cluster);
+
+ static final ConfigKey<Boolean> VmMinCpuSpeedEqualsCpuSpeedDividedByCpuOverprovisioningFactor = new ConfigKey<Boolean>("Advanced", Boolean.class, "vm.min.cpu.speed.equals.cpu.speed.divided.by.cpu.overprovisioning.factor", "true",
+ "If we set this to 'true', a minimum cpu speed (cpu speed/ cpu.overprovisioning.factor) will be setted to the VM, independent of using a scalable service offering or not.", true, ConfigKey.Scope.Cluster);
Review comment:
Same here.
Additionally, what do you think of using **CPU** instead of **cpu** in the configuration description? (quite a cosmetic change, but IMHO it fits better as CPU is an acronyms)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#discussion_r581876865
##########
File path: server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java
##########
@@ -85,6 +89,14 @@
private ServiceOfferingDao _serviceOfferingDao;
@Inject
private NetworkDetailsDao networkDetailsDao;
+ @Inject
+ private HostDao hostDao;
+
+ static final ConfigKey<Boolean> VmMinMemoryEqualsMemoryDividedByMemOverprovisioningFactor = new ConfigKey<Boolean>("Advanced", Boolean.class, "vm.min.memory.equals.memory.divided.by.mem.overprovisioning.factor", "true",
+ "If we set this to 'true', a minimum memory (memory/ mem.overprovisioning.factor) will be setted to the VM, independent of using a scalable service offering or not.", true, ConfigKey.Scope.Cluster);
+
+ static final ConfigKey<Boolean> VmMinCpuSpeedEqualsCpuSpeedDividedByCpuOverprovisioningFactor = new ConfigKey<Boolean>("Advanced", Boolean.class, "vm.min.cpu.speed.equals.cpu.speed.divided.by.cpu.overprovisioning.factor", "true",
+ "If we set this to 'true', a minimum cpu speed (cpu speed/ cpu.overprovisioning.factor) will be setted to the VM, independent of using a scalable service offering or not.", true, ConfigKey.Scope.Cluster);
Review comment:
```suggestion
"If we set this to 'true', a minimum CPU speed (cpu speed/ cpu.overprovisioning.factor) will be set on the VM, independent of using a scalable service offering or not.", true, ConfigKey.Scope.Cluster);
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#issuecomment-806153309
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 249
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on pull request #4722: Externalize config to set min memory/cpu with division by overprovisi…
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on pull request #4722:
URL: https://github.com/apache/cloudstack/pull/4722#issuecomment-806027891
@GabrielBrascher @DaanHoogland
Is this PR OK? Is there anything else needed to do the merge?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org