Skip to content

Usage: usage generated for destroyed VMs with assigned backup offering#4997

Closed
Pearl1594 wants to merge 1 commit into
apache:4.15from
shapeblue:usage-bckup-destroyVM
Closed

Usage: usage generated for destroyed VMs with assigned backup offering#4997
Pearl1594 wants to merge 1 commit into
apache:4.15from
shapeblue:usage-bckup-destroyVM

Conversation

@Pearl1594
Copy link
Copy Markdown
Contributor

@Pearl1594 Pearl1594 commented May 5, 2021

Description

Fixes: #4990
When a VM associated with a backup offering is destroyed/expunged, the backup offering isn't unassigned, hence leading to backup usage generation. This PR fixes/addresses the following:

  • When a VM is destroyed, backup offering associated with the VM is removed
  • A cleanup job to disassociate the backup offering from expunged VMs

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)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

Created a VM and assigned a backup offering to it, destroyed the VM and verified no further usage events are generated and events corresponding to "BACKUP.OFFERING.REMOVE" is generated / added in the event table

@Pearl1594 Pearl1594 requested review from shwstppr and yadvr May 5, 2021 13:26
@Pearl1594 Pearl1594 changed the base branch from master to 4.15 May 5, 2021 13:27
@Pearl1594 Pearl1594 added this to the 4.15.1.0 milestone May 5, 2021
@Pearl1594
Copy link
Copy Markdown
Contributor Author

Pearl1594 commented May 5, 2021

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented May 5, 2021

@Pearl1594 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented May 5, 2021

Packaging result: ✔️ centos7 ✔️ centos8 ✔️ debian. SL-JID 522

@Pearl1594
Copy link
Copy Markdown
Contributor Author

Pearl1594 commented May 5, 2021

@blueorangutan test

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented May 5, 2021

@Pearl1594 a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@Pearl1594 Pearl1594 marked this pull request as ready for review May 5, 2021 13:56
@yadvr
Copy link
Copy Markdown
Member

yadvr commented May 5, 2021

cc @olivierlemasle can you help review and test this PR? Thanks

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented May 6, 2021

Trillian test result (tid-612)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 56132 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4997-t612-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Intermittent failure detected: /marvin/tests/smoke/test_storage_policy.py
Intermittent failure detected: /marvin/tests/smoke/test_templates.py
Intermittent failure detected: /marvin/tests/smoke/test_usage.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Smoke tests completed. 78 look OK, 9 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File
test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true Failure 305.76 test_routers_network_ops.py
test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false Failure 308.04 test_routers_network_ops.py
ContextSuite context=TestVMWareStoragePolicies>:setup Error 0.00 test_storage_policy.py
test_02_create_template_with_checksum_sha1 Error 65.39 test_templates.py
test_03_create_template_with_checksum_sha256 Error 65.41 test_templates.py
test_04_create_template_with_checksum_md5 Error 65.41 test_templates.py
test_05_create_template_with_no_checksum Error 65.41 test_templates.py
test_04_extract_template Failure 128.25 test_templates.py
ContextSuite context=TestISOUsage>:setup Error 0.00 test_usage.py
test_01_volume_usage Failure 786.55 test_usage.py
test_10_attachAndDetach_iso Failure 1510.64 test_vm_life_cycle.py
test_06_download_detached_volume Failure 311.65 test_volumes.py
ContextSuite context=TestVPCRedundancy>:setup Error 0.00 test_vpc_redundant.py
ContextSuite context=TestVPCNics>:setup Error 0.00 test_vpc_router_nics.py
ContextSuite context=TestRVPCSite2SiteVpn>:setup Error 0.00 test_vpc_vpn.py
ContextSuite context=TestVPCSite2SiteVPNMultipleOptions>:setup Error 0.00 test_vpc_vpn.py
ContextSuite context=TestVpcRemoteAccessVpn>:setup Error 0.00 test_vpc_vpn.py
ContextSuite context=TestVpcSite2SiteVpn>:setup Error 0.00 test_vpc_vpn.py

shwstppr
shwstppr approved these changes May 6, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@shwstppr shwstppr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good

@shwstppr
Copy link
Copy Markdown
Contributor

shwstppr commented May 6, 2021

Re-kicking tests

@blueorangutan test

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented May 6, 2021

@shwstppr a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@olivierlemasle
Copy link
Copy Markdown
Contributor

olivierlemasle commented May 6, 2021

Sure, I'll be able to look at this (and #4982) tonight (Paris time). Thank you!

yadvr
yadvr approved these changes May 6, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@yadvr yadvr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - let's wait for @olivierlemasle 's confirmation and smoketests before merging

olivierlemasle
olivierlemasle reviewed May 6, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@olivierlemasle olivierlemasle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should indeed fix the usage bug.

However, this also deletes all backups from deleted VMs, which seems to be breaking change.

Currently, it seems to be possible to restore an expunged VM from a backup (is it really possible @rhtyd? It's not possible with the dummy B&R provider, but it seems to be possible with Veeam, however when I tested it, I got errors). If backups on deleted VMs can indeed be restored, they are still useful and deleting them would be impactful.

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented May 7, 2021

Trillian test result (tid-613)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 100627 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4997-t613-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_accounts.py
Intermittent failure detected: /marvin/tests/smoke/test_affinity_groups_projects.py
Intermittent failure detected: /marvin/tests/smoke/test_async_job.py
Intermittent failure detected: /marvin/tests/smoke/test_deploy_virtio_scsi_vm.py
Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_iso.py
Intermittent failure detected: /marvin/tests/smoke/test_deploy_vms_with_varied_deploymentplanners.py
Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_with_userdata.py
Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
Intermittent failure detected: /marvin/tests/smoke/test_domain_service_offerings.py
Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
Intermittent failure detected: /marvin/tests/smoke/test_iso.py
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_supported_versions.py
Intermittent failure detected: /marvin/tests/smoke/test_list_ids_parameter.py
Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py
Intermittent failure detected: /marvin/tests/smoke/test_metrics_api.py
Intermittent failure detected: /marvin/tests/smoke/test_multipleips_per_nic.py
Intermittent failure detected: /marvin/tests/smoke/test_nested_virtualization.py
Intermittent failure detected: /marvin/tests/smoke/test_network_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_network.py
Intermittent failure detected: /marvin/tests/smoke/test_nic_adapter_type.py
Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
Intermittent failure detected: /marvin/tests/smoke/test_portforwardingrules.py
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_projects.py
Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
Smoke tests completed. 62 look OK, 25 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File
ContextSuite context=TestAccounts>:setup Error 0.00 test_accounts.py
ContextSuite context=TestAddVmToSubDomain>:setup Error 0.00 test_accounts.py
test_DeleteDomain Error 0.87 test_accounts.py
test_forceDeleteDomain Failure 0.79 test_accounts.py
test_forceDeleteDomain Error 2.94 test_accounts.py
ContextSuite context=TestRemoveUserFromAccount>:setup Error 5.04 test_accounts.py
ContextSuite context=TestDomainsServiceOfferings>:setup Error 1515.10 test_domain_service_offerings.py
ContextSuite context=TestInternalLb>:setup Error 0.00 test_internal_lb.py
ContextSuite context=TestDeployVmWithAffinityGroup>:setup Error 0.00 test_affinity_groups_projects.py
test_01_create_iso_with_checksum_sha1 Error 65.31 test_iso.py
test_02_create_iso_with_checksum_sha256 Error 65.34 test_iso.py
test_03_create_iso_with_checksum_md5 Error 65.33 test_iso.py
test_03_create_iso_with_checksum_md5 Error 66.40 test_iso.py
test_04_create_iso_with_no_checksum Error 65.35 test_iso.py
test_04_create_iso_with_no_checksum Error 66.41 test_iso.py
test_01_create_iso Failure 1511.29 test_iso.py
ContextSuite context=TestISO>:setup Error 3023.54 test_iso.py
ContextSuite context=TestAsyncJob>:setup Error 0.00 test_async_job.py
ContextSuite context=TestLoadBalance>:setup Error 0.00 test_loadbalance.py
ContextSuite context=TestDeployVirtioSCSIVM>:setup Error 0.00 test_deploy_virtio_scsi_vm.py
test_list_clusters_metrics Error 1512.03 test_metrics_api.py
test_list_vms_metrics Error 0.11 test_metrics_api.py
ContextSuite context=TestDeployVMFromISO>:setup Error 0.00 test_deploy_vm_iso.py
ContextSuite context=TestDeployVmWithVariedPlanners>:setup Error 0.00 test_deploy_vms_with_varied_deploymentplanners.py
ContextSuite context=TestNetworkACL>:setup Error 0.00 test_network_acl.py
ContextSuite context=TestDeployVmWithUserData>:setup Error 0.00 test_deploy_vm_with_userdata.py
ContextSuite context=TestRemoteDiagnostics>:setup Error 0.00 test_diagnostics.py
test_delete_account Error 1511.71 test_network.py
test_delete_network_while_vm_on_it Error 0.07 test_network.py
test_deploy_vm_l2network Error 0.05 test_network.py
test_l2network_restart Error 1.11 test_network.py
ContextSuite context=TestPortForwarding>:setup Error 3.33 test_network.py
ContextSuite context=TestPublicIP>:setup Error 1.02 test_network.py
test_reboot_router Error 0.05 test_network.py
test_releaseIP Error 0.45 test_network.py
ContextSuite context=TestRouterRules>:setup Error 0.49 test_network.py
ContextSuite context=TestAdapterTypeForNic>:setup Error 0.00 test_nic_adapter_type.py
test_01_invalid_upgrade_kubernetes_cluster Failure 0.01 test_kubernetes_clusters.py
test_02_deploy_and_upgrade_kubernetes_cluster Failure 0.00 test_kubernetes_clusters.py
test_03_deploy_and_scale_kubernetes_cluster Failure 0.00 test_kubernetes_clusters.py
test_04_basic_lifecycle_kubernetes_cluster Failure 0.00 test_kubernetes_clusters.py
test_05_delete_kubernetes_cluster Failure 0.00 test_kubernetes_clusters.py
test_06_deploy_invalid_kubernetes_ha_cluster Failure 0.00 test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster Failure 0.00 test_kubernetes_clusters.py
test_08_deploy_and_upgrade_kubernetes_ha_cluster Failure 0.00 test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster Failure 0.00 test_kubernetes_clusters.py
test_01_add_delete_kubernetes_supported_version Error 60.82 test_kubernetes_supported_versions.py
ContextSuite context=TestListIdsParams>:setup Error 0.00 test_list_ids_parameter.py
test_nic_secondaryip_add_remove Error 1511.85 test_multipleips_per_nic.py
ContextSuite context=TestNestedVirtualization>:setup Error 0.00 test_nested_virtualization.py
ContextSuite context=TestIsolatedNetworksPasswdServer>:setup Error 0.00 test_password_server.py
ContextSuite context=TestPortForwardingRules>:setup Error 0.00 test_portforwardingrules.py
ContextSuite context=TestPrivateGwACL>:setup Error 0.00 test_privategw_acl.py
ContextSuite context=TestProjectSuspendActivate>:setup Error 1517.57 test_projects.py

@yadvr yadvr marked this pull request as draft May 7, 2021 10:55
@yadvr yadvr requested review from shwstppr and yadvr May 10, 2021 10:42
GutoVeronezi
GutoVeronezi reviewed May 10, 2021
View reviewed changes
Comment thread server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
Comment on lines +2988 to +2989
if (destroyedVm.getBackupOfferingId() != null) {
final BackupOfferingVO offering = backupOfferingDao.findById(destroyedVm.getBackupOfferingId());
Copy link
Copy Markdown
Contributor

@GutoVeronezi GutoVeronezi May 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

destroyedVm.getBackupOfferingId() called twice, it could be extracted to a variable.

@Pearl1594 Pearl1594 closed this May 11, 2021
@Pearl1594
Copy link
Copy Markdown
Contributor Author

Pearl1594 commented May 11, 2021

Closing this. Raised #5017 to address the the issue of backup usage records being generated for VMs that are expunged and have no backups present.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@GutoVeronezi GutoVeronezi GutoVeronezi left review comments

@yadvr yadvr Awaiting requested review from yadvr

@shwstppr shwstppr Awaiting requested review from shwstppr

+1 more reviewer

@olivierlemasle olivierlemasle olivierlemasle left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

component:usage-server

Projects

None yet

Milestone

4.15.1.0

Development

Successfully merging this pull request may close these issues.

Backup usages are still generated after VM deletion

6 participants

@Pearl1594 @blueorangutan @yadvr @shwstppr @olivierlemasle @GutoVeronezi