-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Handle with VM snapshot events #4251
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handle with VM snapshot events #4251
Conversation
Create methods to handle with VM snapshot create and delete events Add unit tests to cover new code
|
@blueorangutan package |
|
@RodrigoDLopez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos7 ✔debian. JID-1701 |
|
@blueorangutan test |
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos7 ✔debian. JID-1740 |
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos7 ✖centos8 ✖debian. JID-1843 |
rohityadavcloud
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, did not test it; travis tests LGTM
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos7 ✖centos8 ✖debian. JID-1946 |
|
@blueorangutan package |
|
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2043 |
|
@blueorangutan test |
|
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
@blueorangutan test |
|
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-2840)
|
DaanHoogland
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code looks good but the double records are kind of a problem, does this happen in the old/current implementation as well?
Did you create an issue yet? please do so before merging. maybe these double records should be mentioned in release notes as well (cc @PaulAngus ) |
|
Hi @DaanHoogland As you requested, I will open an issue to report these duplicate records for |
GabrielBrascher
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM
|
@nvazquez can you review this? thanks. |
nvazquez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good, just left a comment
|
@blueorangutan package |
|
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2187 |
|
@blueorangutan test |
|
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-2971)
|
Description
On these days, when a VM snapshot is deleted, the
cloudstack-usagestill seeing that snapshot as active and continues billing this deleted snapshot. This PR proposes new methods to handle withvm_snapshot_createandvm_snapshot_deleteevents.Also, add some unit tests to cover those new methods.
Types of changes
How Has This Been Tested?
I don't know why the
cloudstack-usageduplicate entries for vm_snapshot. I think this is a bug and I'll fix it in a future PR.To run tests, I create a VM snapshot and used the
cloudmonkeyto list usageRecords type=25.Created at 14:12
Deleted at 14:18