Fixes regarding VOLUME_DELETE events resulting from account deletion. #1372
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
|
Which file is missing the license? Still don't see that in the Jenkins build. |
|
@ProjectMoon do you want to merge this only on master or since 4.6/4.7? Check the target branch of the PR. |
|
Yes.. wrong branch again. GitHub's cross-fork interface tends to be a bit confusing for me due to how it loads when you change repos. Attempt #3 coming up. |
|
@ProjectMoon :) Github need to fix this, really annoying |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.