You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by nnesic <gi...@git.apache.org> on 2015/10/12 17:22:33 UTC
[GitHub] cloudstack pull request: Usage event fixes for deleted accounts
GitHub user nnesic opened a pull request:
https://github.com/apache/cloudstack/pull/924
Usage event fixes for deleted accounts
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.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/greenqloud/cloudstack pr-volume-usage-events-fixes
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/924.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #924
----
commit efb558ad6de41fb8103d630c72fe2cf20c5809f9
Author: nnesic <ne...@greenqloud.com>
Date: 2015-10-07T10:46:41Z
Changed usage utils to lookup deleted accounts.
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.
commit 7c9d2b48ea13f2d4d262ab25ca2a4414b3d34b8f
Author: nnesic <ne...@greenqloud.com>
Date: 2015-10-12T13:15:30Z
Emit a VOLUME_DELETE usage event when account deletion destroys an instance.
Currently the logic about volume deletion seems to be 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.
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.
Added 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.
commit 7290eb3b3848626f417f85d554b876de88e74570
Author: nnesic <ne...@greenqloud.com>
Date: 2015-10-12T13:28:58Z
Added the 'status' to volume event descriptions
to allow distinguishing between pre- and post-stateTransition events
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Usage event fixes for deleted accounts
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/924#issuecomment-175672214
@ProjectMoon can you rebase against latest master, and squash changes to a single commit
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Usage event fixes for deleted accounts
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/924#issuecomment-158148817
code reviewed LGTM, @remibergsma kan you start a bubble with this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Usage event fixes for deleted accounts
Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/924#issuecomment-175705151
I don't have the close button, so this one needs to be closed again.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Usage event fixes for deleted accounts
Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/924#issuecomment-160135926
What's the status of this pull request? Do we need to redo this PR on 4.6 instead? Also what are the thoughts about squashing some of these commits down?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Usage event fixes for deleted accounts
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/924#issuecomment-150836950
Pinging @borisroman @michaelandersen to review this PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Usage event fixes for deleted accounts
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/924#issuecomment-175692009
@ProjectMoon yes please squash them and maybe open a new PR against 4.7 (or 4.6) closing this one as github won't allow you to edit/change the source/dest branches.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Usage event fixes for deleted accounts
Posted by kishankavala <gi...@git.apache.org>.
Github user kishankavala commented on the pull request:
https://github.com/apache/cloudstack/pull/924#issuecomment-147945829
@remibergsma We not have any tests that will check for correct events on account delete.
@nnesic Would you be able to add one?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Usage event fixes for deleted accounts
Posted by nnesic <gi...@git.apache.org>.
Github user nnesic commented on the pull request:
https://github.com/apache/cloudstack/pull/924#issuecomment-148138708
I added two unit tests to check that the account manager adheres to its new responsibility of emitting the usage events for root volumes of instances which were not in destroyed state prior to the account deletion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Usage event fixes for deleted accounts
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/924#issuecomment-158348133
I tried rebasing against current master but I run into build issues. Tried again but same thing.
@nnesic Could you try rebasing the PR against master and run a build?
```
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 1:35.871s
[INFO] Finished at: Fri Nov 20 09:51:10 GMT 2015
[INFO] Final Memory: 50M/329M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.2:testCompile (default-testCompile) on project cloud-server: Compilation failure
[ERROR] /data/git/cs1/cloudstack/dist/rpmbuild/BUILD/cloudstack-4.7.0-SNAPSHOT/server/test/com/cloud/user/MockUsageEventDao.java:[19,7] error: MockUsageEventDao is not abstract and does not override abst
ract method searchAndDistinctCount(SearchCriteria<UsageEventVO>,Filter) in GenericDao
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Usage event fixes for deleted accounts
Posted by nnesic <gi...@git.apache.org>.
Github user nnesic closed the pull request at:
https://github.com/apache/cloudstack/pull/924
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Usage event fixes for deleted accounts
Posted by nnesic <gi...@git.apache.org>.
Github user nnesic commented on the pull request:
https://github.com/apache/cloudstack/pull/924#issuecomment-158052503
Hello, how is the review on this going?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Usage event fixes for deleted accounts
Posted by nnesic <gi...@git.apache.org>.
Github user nnesic commented on the pull request:
https://github.com/apache/cloudstack/pull/924#issuecomment-147469015
Hi, could you please re-trigger the build, since it seems to have crashed?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Usage event fixes for deleted accounts
Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/924#issuecomment-175704775
New request is located at https://github.com/apache/cloudstack/pull/1372.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Usage event fixes for deleted accounts
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/924#issuecomment-147493625
Hi @nnesic, you can do that by force pushing your commits again to the PR branch. The tests will then run again.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Usage event fixes for deleted accounts
Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/924#issuecomment-158961187
The PR has been fixed by the application of two more commits. Maybe squashing is in order before a merge.
The abuse of reflection required to make the unit test play nice is an indicator that maybe that class should be made non-static though.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Usage event fixes for deleted accounts
Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/924#issuecomment-175675521
Squashing and rebasing is definitely needed. But isn't this a bug fix though? And thus should go on to 4.6 and then be forward merged?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Usage event fixes for deleted accounts
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/924#issuecomment-175708872
@nnesic since you're the author you can close this PR @ProjectMoon thanks for the new PR
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---