You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "mprokopchuk (via GitHub)" <gi...@apache.org> on 2023/07/20 21:39:55 UTC
[GitHub] [cloudstack] mprokopchuk opened a new pull request, #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
mprokopchuk opened a new pull request, #7760:
URL: https://github.com/apache/cloudstack/pull/7760
Description
deployVirtualMachine can be provided with disk offerings for data disks, and destroyVirtualMachine can be set to expunge data disks. This specific circumstance is not publishing volume delete event.
New PR instead of closed https://github.com/apache/cloudstack/pull/7714/
Create VM with Volume
Delete VM and set expunge and add Volume to be removal as well
<!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
<!-- Fixes: # -->
<!--- ********************************************************************************* -->
<!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
<!--- PLEASE PUT AN 'X' in only **ONE** box -->
<!--- ********************************************************************************* -->
### 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
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [X] Minor
- [ ] Trivial
### Screenshots (if appropriate):
Create VM:
<img width="1483" alt="250657320-3afe9b65-837d-4a44-a676-683cc0126bf2" src="https://github.com/apache/cloudstack/assets/742444/561ed2f1-4153-4597-8175-1dbe61d7c654">
Create VM auto-start:
<img width="1485" alt="250657382-5f870080-75f1-4055-999a-c5f82e97d1c8" src="https://github.com/apache/cloudstack/assets/742444/6c1f3e54-61da-4dd5-b33a-5510804262f3">
Delete VM:
<img width="1478" alt="250657446-8bbc3993-b319-4d15-86b8-008d954eea0d" src="https://github.com/apache/cloudstack/assets/742444/dd0249ff-735e-42ae-9f53-68b9aa048db2">
Expunge VM:
<img width="1483" alt="250657523-163dc70c-7d8a-4af2-a7a4-24a92d20a14b" src="https://github.com/apache/cloudstack/assets/742444/99f1e11f-6dd6-42ff-9c7f-211e8ffcd4c4">
### How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
Tested manually.
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document -->
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rohityadavcloud merged pull request #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud merged PR #7760:
URL: https://github.com/apache/cloudstack/pull/7760
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] codecov[bot] commented on pull request #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #7760:
URL: https://github.com/apache/cloudstack/pull/7760#issuecomment-1644745881
## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/7760?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#7760](https://app.codecov.io/gh/apache/cloudstack/pull/7760?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (44083c4) into [4.18](https://app.codecov.io/gh/apache/cloudstack/commit/f0cc76a3a824fb4c34ce07230557c2c5107131fa?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (f0cc76a) will **decrease** coverage by `0.01%`.
> The diff coverage is `2.22%`.
```diff
@@ Coverage Diff @@
## 4.18 #7760 +/- ##
============================================
- Coverage 13.02% 13.01% -0.01%
- Complexity 9027 9028 +1
============================================
Files 2719 2720 +1
Lines 256866 257007 +141
Branches 40051 40078 +27
============================================
+ Hits 33447 33449 +2
- Misses 219232 219371 +139
Partials 4187 4187
```
| [Impacted Files](https://app.codecov.io/gh/apache/cloudstack/pull/7760?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...n/java/com/cloud/vm/VirtualMachineManagerImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/7760?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZW5naW5lL29yY2hlc3RyYXRpb24vc3JjL21haW4vamF2YS9jb20vY2xvdWQvdm0vVmlydHVhbE1hY2hpbmVNYW5hZ2VySW1wbC5qYXZh) | `6.36% <0.00%> (-0.01%)` | :arrow_down: |
| [...stack/engine/orchestration/VolumeOrchestrator.java](https://app.codecov.io/gh/apache/cloudstack/pull/7760?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZW5naW5lL29yY2hlc3RyYXRpb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Nsb3Vkc3RhY2svZW5naW5lL29yY2hlc3RyYXRpb24vVm9sdW1lT3JjaGVzdHJhdG9yLmphdmE=) | `1.90% <0.00%> (-0.02%)` | :arrow_down: |
| [...n/java/com/cloud/storage/VolumeApiServiceImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/7760?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL3N0b3JhZ2UvVm9sdW1lQXBpU2VydmljZUltcGwuamF2YQ==) | `15.27% <0.00%> (-0.02%)` | :arrow_down: |
| [.../src/main/java/com/cloud/vm/UserVmManagerImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/7760?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL3ZtL1VzZXJWbU1hbmFnZXJJbXBsLmphdmE=) | `7.33% <0.00%> (-0.03%)` | :arrow_down: |
| [...ain/java/com/cloud/api/query/QueryManagerImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/7760?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL2FwaS9xdWVyeS9RdWVyeU1hbmFnZXJJbXBsLmphdmE=) | `3.03% <100.00%> (+0.03%)` | :arrow_up: |
... and [13 files with indirect coverage changes](https://app.codecov.io/gh/apache/cloudstack/pull/7760/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7760:
URL: https://github.com/apache/cloudstack/pull/7760#issuecomment-1657804813
Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 6604
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7760:
URL: https://github.com/apache/cloudstack/pull/7760#issuecomment-1645173464
@DaanHoogland a [SF] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke 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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7760:
URL: https://github.com/apache/cloudstack/pull/7760#issuecomment-1657981361
@DaanHoogland a [SF] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke 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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7760:
URL: https://github.com/apache/cloudstack/pull/7760#issuecomment-1657727711
@DaanHoogland a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. 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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rohityadavcloud commented on pull request #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud commented on PR #7760:
URL: https://github.com/apache/cloudstack/pull/7760#issuecomment-1665094678
@blueorangutan test alma8 kvm-alma8
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7760:
URL: https://github.com/apache/cloudstack/pull/7760#issuecomment-1645054112
@DaanHoogland a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. 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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7760:
URL: https://github.com/apache/cloudstack/pull/7760#issuecomment-1645167479
@blueorangutan test
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7760:
URL: https://github.com/apache/cloudstack/pull/7760#issuecomment-1666125293
<b>[SF] Trillian test result (tid-7267)</b>
Environment: kvm-alma9 (x2), Advanced Networking with Mgmt server a9
Total time taken: 47265 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7760-t7267-kvm-alma9.zip
Smoke tests completed. 107 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
ContextSuite context=TestVMWareStoragePolicies>:setup | `Error` | 0.00 | test_storage_policy.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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7760:
URL: https://github.com/apache/cloudstack/pull/7760#issuecomment-1657979898
@blueorangutan test
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7760:
URL: https://github.com/apache/cloudstack/pull/7760#issuecomment-1664368330
@rohityadavcloud a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. 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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7760:
URL: https://github.com/apache/cloudstack/pull/7760#issuecomment-1664455366
Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 6656
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] mprokopchuk commented on a diff in pull request #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
Posted by "mprokopchuk (via GitHub)" <gi...@apache.org>.
mprokopchuk commented on code in PR #7760:
URL: https://github.com/apache/cloudstack/pull/7760#discussion_r1272739061
##########
api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java:
##########
@@ -765,28 +768,33 @@ public ApiCommandResourceType getApiResourceType() {
public void execute() {
UserVm result;
- try {
- CallContext.current().setEventDetails("Vm Id: " + getEntityUuid());
- result = _userVmService.startVirtualMachine(this);
- } catch (ResourceUnavailableException ex) {
- s_logger.warn("Exception: ", ex);
- throw new ServerApiException(ApiErrorCode.RESOURCE_UNAVAILABLE_ERROR, ex.getMessage());
- } catch (ResourceAllocationException ex) {
- s_logger.warn("Exception: ", ex);
- throw new ServerApiException(ApiErrorCode.RESOURCE_ALLOCATION_ERROR, ex.getMessage());
- } catch (ConcurrentOperationException ex) {
- s_logger.warn("Exception: ", ex);
- throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, ex.getMessage());
- } catch (InsufficientCapacityException ex) {
- StringBuilder message = new StringBuilder(ex.getMessage());
- if (ex instanceof InsufficientServerCapacityException) {
- if (((InsufficientServerCapacityException)ex).isAffinityApplied()) {
- message.append(", Please check the affinity groups provided, there may not be sufficient capacity to follow them");
+ CallContext.current().setEventDetails("Vm Id: " + getEntityUuid());
Review Comment:
In this case there is a call made from the command to API: _userVmService.startVirtualMachine(this);
startVirtualMachine(..) intercepted by ActionEventInterceptor, which needs entity Id to add it to the event.
If startVirtualMachine(..) called by API layer, entity Id set to the context from the filter and there is no issue, but if it is called from command or any other layer, which may not have entity Id, then Id is missing in the event and event itself does not point to the right resource.
This is why it is set explicitly before making call to the API layer.
As of now I don't see other simple enough way how to add Id to the context.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #7760:
URL: https://github.com/apache/cloudstack/pull/7760#discussion_r1274535393
##########
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##########
@@ -500,22 +500,29 @@ public void allocate(final String vmInstanceName, final VirtualMachineTemplate t
allocateRootVolume(persistedVm, template, rootDiskOfferingInfo, owner, rootDiskSizeFinal);
- if (dataDiskOfferings != null) {
- for (final DiskOfferingInfo dataDiskOfferingInfo : dataDiskOfferings) {
- volumeMgr.allocateRawVolume(Type.DATADISK, "DATA-" + persistedVm.getId(), dataDiskOfferingInfo.getDiskOffering(), dataDiskOfferingInfo.getSize(),
- dataDiskOfferingInfo.getMinIops(), dataDiskOfferingInfo.getMaxIops(), persistedVm, template, owner, null);
+ // Create new Volume context and inject event resource type, id and details to generate VOLUME.CREATE event for the ROOT disk.
Review Comment:
this comment indicates this could be an extracted method (and javadoc instead)
##########
api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java:
##########
@@ -765,28 +768,33 @@ public ApiCommandResourceType getApiResourceType() {
public void execute() {
UserVm result;
- try {
- CallContext.current().setEventDetails("Vm Id: " + getEntityUuid());
- result = _userVmService.startVirtualMachine(this);
- } catch (ResourceUnavailableException ex) {
- s_logger.warn("Exception: ", ex);
- throw new ServerApiException(ApiErrorCode.RESOURCE_UNAVAILABLE_ERROR, ex.getMessage());
- } catch (ResourceAllocationException ex) {
- s_logger.warn("Exception: ", ex);
- throw new ServerApiException(ApiErrorCode.RESOURCE_ALLOCATION_ERROR, ex.getMessage());
- } catch (ConcurrentOperationException ex) {
- s_logger.warn("Exception: ", ex);
- throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, ex.getMessage());
- } catch (InsufficientCapacityException ex) {
- StringBuilder message = new StringBuilder(ex.getMessage());
- if (ex instanceof InsufficientServerCapacityException) {
- if (((InsufficientServerCapacityException)ex).isAffinityApplied()) {
- message.append(", Please check the affinity groups provided, there may not be sufficient capacity to follow them");
+ CallContext.current().setEventDetails("Vm Id: " + getEntityUuid());
+ if (getStartVm()) {
+ try {
+ result = _userVmService.startVirtualMachine(this);
+ } catch (ResourceUnavailableException ex) {
+ s_logger.warn("Exception: ", ex);
+ throw new ServerApiException(ApiErrorCode.RESOURCE_UNAVAILABLE_ERROR, ex.getMessage());
+ } catch (ResourceAllocationException ex) {
+ s_logger.warn("Exception: ", ex);
+ throw new ServerApiException(ApiErrorCode.RESOURCE_ALLOCATION_ERROR, ex.getMessage());
+ } catch (ConcurrentOperationException ex) {
+ s_logger.warn("Exception: ", ex);
+ throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, ex.getMessage());
+ } catch (InsufficientCapacityException ex) {
+ StringBuilder message = new StringBuilder(ex.getMessage());
+ if (ex instanceof InsufficientServerCapacityException) {
+ if (((InsufficientServerCapacityException)ex).isAffinityApplied()) {
+ message.append(", Please check the affinity groups provided, there may not be sufficient capacity to follow them");
+ }
}
+ s_logger.info(ex);
+ s_logger.info(message.toString(), ex);
Review Comment:
a bit out of scope for this PR but these two `info`'s seem a bit redundant:
```suggestion
s_logger.info(String.format("%s: %s", message.toString(), ex.getLocalizedMessage()));
s_logger.debug(message.toString(), ex);
```
##########
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:
##########
@@ -721,6 +721,10 @@ private Pair<List<EventJoinVO>, Integer> searchForEventsInternal(ListEventsCmd c
ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third();
Filter searchFilter = new Filter(EventJoinVO.class, "createDate", false, cmd.getStartIndex(), cmd.getPageSizeVal());
+ // additional order by since createdDate does not have milliseconds
+ // and two events, created within one second can be incorrectly ordered (for example VM.CREATE Completed before Scheduled)
Review Comment:
yes, a known issue in usage. good fix. I would like this added to javadoc somewhere. (maybe a separate issue, not in this PR).
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -8018,15 +8031,29 @@ private void detachVolumesFromVm(List<VolumeVO> volumes) {
}
}
- private void deleteVolumesFromVm(List<VolumeVO> volumes, boolean expunge) {
+ private void deleteVolumesFromVm(UserVmVO vm, List<VolumeVO> volumes, boolean expunge) {
for (VolumeVO volume : volumes) {
+ destroyVolumeInContext(vm, expunge, volume);
+ }
+ }
+ private void destroyVolumeInContext(UserVmVO vm, boolean expunge, VolumeVO volume) {
+ // Create new context and inject correct event resource type, id and details,
+ // otherwise VOLUME.DESTROY event will be associated with VirtualMachine and contain VM id and other information.
+ CallContext volumeContext = CallContext.register(CallContext.current(), ApiCommandResourceType.Volume);
+ volumeContext.setEventDetails("Volume Type: " + volume.getVolumeType() + " Volume Id: " + this._uuidMgr.getUuid(Volume.class, volume.getId()) + " Vm Id: " + vm.getUuid());
+ volumeContext.setEventResourceType(ApiCommandResourceType.Volume);
+ volumeContext.setEventResourceId(volume.getId());
+ try {
Volume result = _volumeService.destroyVolume(volume.getId(), CallContext.current().getCallingAccount(), expunge, false);
if (result == null) {
- s_logger.error("DestroyVM remove volume - failed to delete volume " + volume.getInstanceId() + " from instance " + volume.getId());
+ s_logger.error("DestroyVM remove volume - failed to delete volume " + volume.getId() + " from instance " + volume.getInstanceId());
Review Comment:
```suggestion
s_logger.error(String.format("DestroyVM remove volume - failed to delete volume %s from instance %s", volume.getId(), volume.getInstanceId()));
```
##########
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java:
##########
@@ -1277,6 +1277,21 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
}
}
+ private void destroyVolumeInContext(Volume volume) {
+ // Create new context and inject correct event resource type, id and details,
+ // otherwise VOLUME.DESTROY event will be associated with VirtualMachine and contain VM id and other information.
Review Comment:
could these be javadoc?
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -8018,15 +8031,29 @@ private void detachVolumesFromVm(List<VolumeVO> volumes) {
}
}
- private void deleteVolumesFromVm(List<VolumeVO> volumes, boolean expunge) {
+ private void deleteVolumesFromVm(UserVmVO vm, List<VolumeVO> volumes, boolean expunge) {
for (VolumeVO volume : volumes) {
+ destroyVolumeInContext(vm, expunge, volume);
+ }
+ }
+ private void destroyVolumeInContext(UserVmVO vm, boolean expunge, VolumeVO volume) {
+ // Create new context and inject correct event resource type, id and details,
+ // otherwise VOLUME.DESTROY event will be associated with VirtualMachine and contain VM id and other information.
Review Comment:
this comment can be javadoc
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7760:
URL: https://github.com/apache/cloudstack/pull/7760#issuecomment-1657725714
@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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7760:
URL: https://github.com/apache/cloudstack/pull/7760#issuecomment-1665086469
@DaanHoogland a [SF] Trillian-Jenkins test job (alma9 mgmt + kvm-alma9) has been kicked to run smoke 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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7760:
URL: https://github.com/apache/cloudstack/pull/7760#issuecomment-1645111470
Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 6529
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #7760:
URL: https://github.com/apache/cloudstack/pull/7760#discussion_r1270353032
##########
api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java:
##########
@@ -765,28 +768,33 @@ public ApiCommandResourceType getApiResourceType() {
public void execute() {
UserVm result;
- try {
- CallContext.current().setEventDetails("Vm Id: " + getEntityUuid());
- result = _userVmService.startVirtualMachine(this);
- } catch (ResourceUnavailableException ex) {
- s_logger.warn("Exception: ", ex);
- throw new ServerApiException(ApiErrorCode.RESOURCE_UNAVAILABLE_ERROR, ex.getMessage());
- } catch (ResourceAllocationException ex) {
- s_logger.warn("Exception: ", ex);
- throw new ServerApiException(ApiErrorCode.RESOURCE_ALLOCATION_ERROR, ex.getMessage());
- } catch (ConcurrentOperationException ex) {
- s_logger.warn("Exception: ", ex);
- throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, ex.getMessage());
- } catch (InsufficientCapacityException ex) {
- StringBuilder message = new StringBuilder(ex.getMessage());
- if (ex instanceof InsufficientServerCapacityException) {
- if (((InsufficientServerCapacityException)ex).isAffinityApplied()) {
- message.append(", Please check the affinity groups provided, there may not be sufficient capacity to follow them");
+ CallContext.current().setEventDetails("Vm Id: " + getEntityUuid());
Review Comment:
As my comment on https://github.com/apache/cloudstack/pull/7714#discussion_r1251610251
I have not seen this done in Cmd classes yet. Usually it happens in the service, i think. Did you consider that as well? (and what were the pros and cons)
It would be good to keep such considerations for posterity.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7760:
URL: https://github.com/apache/cloudstack/pull/7760#issuecomment-1646341871
<b>[SF] Trillian test result (tid-7133)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 55302 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7760-t7133-kvm-centos7.zip
Smoke tests completed. 107 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_migrate_VM_and_root_volume | `Error` | 83.87 | test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks | `Error` | 56.55 | 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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7760:
URL: https://github.com/apache/cloudstack/pull/7760#issuecomment-1645053057
@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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rohityadavcloud commented on pull request #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud commented on PR #7760:
URL: https://github.com/apache/cloudstack/pull/7760#issuecomment-1664367824
@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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7760:
URL: https://github.com/apache/cloudstack/pull/7760#issuecomment-1665083715
@blueorangutan test alma9 kvm-alma9
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7760:
URL: https://github.com/apache/cloudstack/pull/7760#issuecomment-1665095636
@rohityadavcloud a [SF] Trillian-Jenkins test job (alma8 mgmt + kvm-alma8) has been kicked to run smoke 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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7760: VM.CREATE/VOLUME.DELETE/VOLUME.DESTROY not being emitted
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7760:
URL: https://github.com/apache/cloudstack/pull/7760#issuecomment-1666111918
<b>[SF] Trillian test result (tid-7268)</b>
Environment: kvm-alma8 (x2), Advanced Networking with Mgmt server a8
Total time taken: 45453 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7760-t7268-kvm-alma8.zip
Smoke tests completed. 107 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
ContextSuite context=TestVMWareStoragePolicies>:setup | `Error` | 0.00 | test_storage_policy.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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org