Fix to make recovered volumes be accounted for by Usage by JoaoJandre · Pull Request #6772 · apache/cloudstack

JoaoJandre · 2022-09-27T13:52:22Z

Description

When a volume is removed, ACS inserts a VOLUME.DELETE record into the cloud.usage_event table. If the volume is removed without the expunge option, it can still be restored.
When the volume is restored, ACS does not insert a new record in the cloud.usage_event stating that it has been recreated. Thus, the volume continues to be used without being accounted for.
This PR adds an event in the volume restore process that indicates the volume was recreated. So it will be accounted for.

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

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

This PR was tested in a local lab, by destroying and recreating volumes and checking the DB to see if the event was added.
Some unit tests were also added.

DaanHoogland · 2022-09-28T06:35:34Z

@JoaoJandre did you look at the flow for changeOfferingForVolume, migrateVolume, resizeVolume and updateVolume?
Not sure but the code you extracted may be used there as well.
Please check, CLGTM otherwise.

JoaoJandre · 2022-09-28T15:31:41Z

@JoaoJandre did you look at the flow for changeOfferingForVolume, migrateVolume, resizeVolume and updateVolume? Not sure but the code you extracted may be used there as well. Please check, CLGTM otherwise.

@DaanHoogland I checked these flows and the event publishes in these places are a bit different, the ones I saw at least already had the diskOfferingVO available, so if we used the new method I extracted we would be making an unnecessary call to the DB.

DaanHoogland

clgtm

DaanHoogland · 2022-09-28T15:42:18Z

@blueorangutan package

blueorangutan · 2022-09-28T15:43:03Z

@DaanHoogland a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2022-09-28T16:58:52Z

Packaging result: ✖️ el7 ✖️ el8 ✔️ debian ✖️ suse15. SL-JID 4305

blueorangutan · 2022-09-29T08:47:38Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 4314

DaanHoogland · 2022-09-29T09:10:50Z

@blueorangutan test

blueorangutan · 2022-09-29T09:11:03Z

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

DaanHoogland · 2022-09-29T09:13:37Z

@harikrishna-patnala @GutoVeronezi can you guys have a look?

blueorangutan · 2022-09-29T21:21:32Z

Trillian test result (tid-5046)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 42392 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6772-t5046-kvm-centos7.zip
Smoke tests completed. 103 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java

codecov · 2022-09-30T18:30:46Z

Codecov Report

Merging #6772 (947768e) into main (d9dd4c1) will decrease coverage by 0.00%.
The diff coverage is 84.61%.

@@             Coverage Diff              @@
##               main    #6772      +/-   ##
============================================
- Coverage     10.52%   10.52%   -0.01%     
- Complexity     6784     6789       +5     
============================================
  Files          2464     2464              
  Lines        243988   244171     +183     
  Branches      38185    38205      +20     
============================================
+ Hits          25690    25709      +19     
- Misses       215065   215228     +163     
- Partials       3233     3234       +1

Impacted Files	Coverage Δ
.../src/main/java/com/cloud/vm/UserVmManagerImpl.java	`5.61% <0.00%> (+0.01%)`	⬆️
...n/java/com/cloud/storage/VolumeApiServiceImpl.java	`12.87% <91.66%> (+0.42%)`	⬆️
...dstack/network/contrail/model/ModelObjectBase.java	`21.15% <0.00%> (-7.70%)`	⬇️
.../main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java	`27.34% <0.00%> (-0.74%)`	⬇️
...n/java/com/cloud/capacity/dao/CapacityDaoImpl.java	`3.24% <0.00%> (-0.13%)`	⬇️
.../src/main/java/com/cloud/vm/dao/UserVmDaoImpl.java	`0.77% <0.00%> (-0.07%)`	⬇️
...n/java/com/cloud/template/TemplateManagerImpl.java	`10.97% <0.00%> (-0.02%)`	⬇️
...che/cloudstack/metrics/PrometheusExporterImpl.java	`0.00% <0.00%> (ø)`
...e/cloudstack/metrics/PrometheusExporterServer.java	`0.00% <0.00%> (ø)`
...apache/cloudstack/alert/snmp/SnmpTrapAppender.java	`61.05% <0.00%> (+2.10%)`	⬆️
... and 2 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

acs-robot · 2022-09-30T18:32:21Z

Found UI changes, kicking a new UI QA build
@blueorangutan ui

blueorangutan · 2022-09-30T18:33:03Z

@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

acs-robot · 2022-09-30T18:35:55Z

Found UI changes, kicking a new UI QA build
@blueorangutan ui

blueorangutan · 2022-09-30T18:37:03Z

@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

acs-robot · 2022-09-30T18:38:26Z

Found UI changes, kicking a new UI QA build
@blueorangutan ui

blueorangutan · 2022-09-30T18:39:00Z

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6772 (SL-JID-2448)

blueorangutan · 2022-09-30T18:39:03Z

@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

blueorangutan · 2022-09-30T18:42:12Z

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6772 (SL-JID-2449)

blueorangutan · 2022-09-30T18:44:42Z

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6772 (SL-JID-2450)

GutoVeronezi

LGTM

sonarqubecloud · 2022-09-30T19:53:51Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

72.2% Coverage
0.0% Duplication

DaanHoogland · 2022-10-02T14:59:48Z

@JoaoJandre almost ready to merge. I might be overcautious but I´d like someone to test the merged code.
@vladimirpetrov can you do that?

yadvr · 2022-10-08T06:24:04Z

@blueorangutan package

blueorangutan · 2022-10-08T06:25:03Z

@rohityadavcloud a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2022-10-08T08:50:34Z

Packaging result: ✖️ el7 ✔️ el8 ✔️ debian ✖️ suse15. SL-JID 4379

yadvr · 2022-10-09T09:54:03Z

@blueorangutan package

blueorangutan · 2022-10-09T09:55:05Z

@rohityadavcloud a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2022-10-09T11:33:30Z

Packaging result: ✖️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 4399

yadvr · 2022-10-09T13:12:32Z

@blueorangutan package

blueorangutan · 2022-10-09T13:13:03Z

@rohityadavcloud a Jenkins job has been kicked to build packages. It will be bundled with SystemVM template(s). I'll keep you posted as I make progress.

blueorangutan · 2022-10-09T14:16:10Z

Packaging result: ✔️ el7 ✔️ el8 ✖️ debian ✔️ suse15. SL-JID 4402

yadvr · 2022-10-09T14:29:12Z

@blueorangutan test

blueorangutan · 2022-10-09T14:30:06Z

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

blueorangutan · 2022-10-09T14:35:00Z

Trillian Build Failed (tid-5091)

yadvr · 2022-10-09T14:35:53Z

@blueorangutan test

blueorangutan · 2022-10-09T14:37:03Z

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

blueorangutan · 2022-10-10T02:23:12Z

Trillian test result (tid-5092)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 40932 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6772-t5092-kvm-centos7.zip
Smoke tests completed. 103 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_08_upgrade_kubernetes_ha_cluster	`Failure`	586.47	test_kubernetes_clusters.py

boring-cyborg bot added component:api component:storage labels Sep 27, 2022

Pull recent changes and fix tests

ac3c49e

JoaoJandre force-pushed the add-event-to-recoverVolume branch from 2dbe96a to ac3c49e Compare September 28, 2022 15:27

DaanHoogland approved these changes Sep 28, 2022

View reviewed changes

DaanHoogland requested review from GutoVeronezi and harikrishna-patnala September 29, 2022 09:13

yadvr approved these changes Sep 30, 2022

View reviewed changes

GutoVeronezi reviewed Sep 30, 2022

View reviewed changes

server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java Outdated Show resolved Hide resolved

boring-cyborg bot added component:database component:quota labels Sep 30, 2022

Address reviews

710cf97

JoaoJandre force-pushed the add-event-to-recoverVolume branch from 166e846 to 710cf97 Compare September 30, 2022 18:34

Fix tests

947768e

GutoVeronezi approved these changes Sep 30, 2022

View reviewed changes

DaanHoogland added the status:needs-testing label Oct 2, 2022

yadvr added this to the 4.18.0.0 milestone Oct 8, 2022

DaanHoogland merged commit d6044fb into apache:main Oct 11, 2022

DaanHoogland removed the status:needs-testing label Jan 28, 2026

Conversation

JoaoJandre commented Sep 27, 2022

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

DaanHoogland commented Sep 28, 2022

Uh oh!

JoaoJandre commented Sep 28, 2022

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Sep 28, 2022

Uh oh!

blueorangutan commented Sep 28, 2022

Uh oh!

blueorangutan commented Sep 28, 2022

Uh oh!

blueorangutan commented Sep 29, 2022

Uh oh!

DaanHoogland commented Sep 29, 2022

Uh oh!

blueorangutan commented Sep 29, 2022

Uh oh!

DaanHoogland commented Sep 29, 2022

Uh oh!

blueorangutan commented Sep 29, 2022

Uh oh!

Uh oh!

codecov bot commented Sep 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

acs-robot commented Sep 30, 2022

Uh oh!

blueorangutan commented Sep 30, 2022

Uh oh!

acs-robot commented Sep 30, 2022

Uh oh!

blueorangutan commented Sep 30, 2022

Uh oh!

acs-robot commented Sep 30, 2022

Uh oh!

blueorangutan commented Sep 30, 2022

Uh oh!

blueorangutan commented Sep 30, 2022

Uh oh!

blueorangutan commented Sep 30, 2022

Uh oh!

blueorangutan commented Sep 30, 2022

Uh oh!

GutoVeronezi left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Sep 30, 2022

Uh oh!

DaanHoogland commented Oct 2, 2022

Uh oh!

yadvr commented Oct 8, 2022

Uh oh!

blueorangutan commented Oct 8, 2022

Uh oh!

blueorangutan commented Oct 8, 2022

Uh oh!

yadvr commented Oct 9, 2022

Uh oh!

blueorangutan commented Oct 9, 2022

Uh oh!

blueorangutan commented Oct 9, 2022

Uh oh!

yadvr commented Oct 9, 2022

Uh oh!

blueorangutan commented Oct 9, 2022

Uh oh!

blueorangutan commented Oct 9, 2022

codecov bot commented Sep 30, 2022 •

edited

Loading