Uh oh!
There was an error while loading. Please reload this page.
Fixes regarding VOLUME_DELETE events resulting from account deletion. - #1373
Fixes regarding VOLUME_DELETE events resulting from account deletion.#1373ProjectMoon wants to merge 1 commit into
Conversation
ProjectMoon
commented
Jan 28, 2016
Added the license header to the MockUsageEventDao class to fix rat report error. |
rafaelweingartner
commented
Jan 28, 2016
Hi @ProjectMoon , |
ProjectMoon
commented
Jan 28, 2016
I'm not the original author of this work, but I think it was just because a class for testing was needed. Looking at it now I'm not really sure why it can't just be the regular Edit: reason I got back is that she was following the pattern of other tests (AccountManager tests specifically). |
ProjectMoon
commented
Jan 28, 2016
@rafaelweingartner I have removed the MockUsageEventDao and replaced it with a normal Mockito mock of the UsageEventDao interface. |
There was a problem hiding this comment.
I do not think you need this kind of comment.
rafaelweingartner
commented
Jan 28, 2016
@ProjectMoon that is great, |
The logic about volume deletion was that an event should be emitted when the volume delete is requested, not when the deletion completes. The VolumeStateListener specifically ignores destroy events for ROOT volumes, assuming that the ROOT volume only gets deleted when the instance is destroyed and the UserVmManager should take care of it. This was causing problems when deleting accounts which had running resources. The resources are stopped and destroyed, but we never get a usage event indicating so. When deleting an account, all of its resources get destroyed, but the instance expunging circumvents the UserVmManager, and thus we miss the VOLUME_DESTROY usage event. This commits adds a check in the AccountManager to emit the deletion event for ROOT volumes belonging to instances which weren't destroyed prior to the account deletion. It also adds the volume status to the event emitted on the event bus, for distinguishing between pre- and post- state transitions.
ProjectMoon
commented
Jan 29, 2016
I added the suggested changes. Also, now that #1382 has been merged to 4.6, the checks should succeed. |
rafaelweingartner
commented
Jan 29, 2016
Congratulations, the code is much better now. |
DaanHoogland
commented
Jan 31, 2016
1373.network.results.txt the errors in the network tests are due to moved tests en not related to volumes. I am satisfied with these results: LGTM |
ProjectMoon
commented
Feb 3, 2016
Who gets to be the authority on merging this? |
wido
commented
Feb 10, 2016
@ProjectMoon Currently waiting for a new Release Manager for version 4.9 to pick this up |
ProjectMoon
commented
Feb 10, 2016
Even for forward-merged bug fixes? |
bvbharatk
commented
Mar 18, 2016
ACS CI BVT RunSumarry: The follwing tests have known issues Link to logs Folder (search by build_no):https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0 Failed tests:
Skipped tests: Passed test suits: |
ProjectMoon
commented
Apr 13, 2016
We have found that this fix actually does not work on 4.7+. We are going to submit a new pull request based on 4.7 with a working fix. |
…-4.8 Fixes regarding VOLUME_DELETE events resulting from account deletionNew version of #1491. **Original Description** New version of #1373, but updated for the 4.7 branch with another fix that allows it to properly find expunged root volumes. This is a bug fix, which is why we target the 4.7 branch. Original pull request: Fixes regarding usage event emission. UsageEventUtils was previously not checking deleted accounts, which meant that if an account was deleted that had some resources running on it, those resources would get destroyed without emitting any events. Furthermore, the VOLUME_DELETE event of ROOT volumes is the responsibility of the UserVmManager, which gets circumvented when expunging resources following the account deletion. Added a check to the AccountManager which catches the ROOT volumes that need to be deleted and emits events for them. To test this: Create a new user. As that user, create and destroy an instance. This should cause the VM_CREATE, VM_START, VM_STOP, VM_DESTROY, VOLUME_CREATE, and VOLUME_DELETE events to be emitted. Create a new instance as the same user. Log in as admin, and delete the user. The same set of events should be emitted, and there should be no duplicate DELETE events for the ROOT volume of the previous instance. * pr/1624: Emit a VOLUME_DELETE usage event when account deletion destroys an instance. Signed-off-by: Rajani Karuturi <rajani.karuturi@accelerite.com>
New version of #924, but on the right branch with the commits squashed.
Original pull request:
Fixes regarding usage event emission.
UsageEventUtils was previously not checking deleted accounts, which meant that if an account was deleted that had some resources running on it, those resources would get destroyed without emitting any events.
Furthermore, the VOLUME_DELETE event of ROOT volumes is the responsibility of the UserVmManager, which gets circumvented when expunging resources following the account deletion. Added a check to the AccountManager which catches the ROOT volumes that need to be deleted and emits events for them.
To test this: Create a new user. As that user, create and destroy an instance. This should cause the VM_CREATE, VM_START, VM_STOP, VM_DESTROY, VOLUME_CREATE, and VOLUME_DELETE events to be emitted.
Create a new instance as the same user. Log in as admin, and delete the user. The same set of events should be emitted, and there should be no duplicate DELETE events for the ROOT volume of the previous instance.