Add new column `last_id` to the table volumes by FelipeM525 · Pull Request #9759 · apache/cloudstack

FelipeM525 · 2024-10-02T19:42:15Z

Description

In ACS Once a volume is migrated, it duplicates its row in the database and labels the previous one as removed; however, there is no reference to the deleted row, making it so the operator is unable to track the previous volume effectively. This PR aims to change this by adding a new column to the table volumes called last_id.

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)
build/CI
test (unit or integration test code)

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 by live migrating VM with storage and also by migrating a volume with the VM turned off; in both cases, the new column was populated accordingly.

…it easier to track volumes after their migration

codecov · 2024-10-02T19:50:09Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 4.30%. Comparing base (9df783c) to head (0c86f17).
Report is 57 commits behind head on 4.19.

❗ There is a different number of reports uploaded between BASE (9df783c) and HEAD (0c86f17). Click for more details.
HEAD has 1 upload less than BASE
Flag BASE (9df783c) HEAD (0c86f17)
unittests 1 0

Additional details and impacted files

@@ Coverage Diff @@## 4.19 #9759 +/- ##
============================================
- Coverage 15.11% 4.30% -10.81% 
============================================
Files 5402 366 -5036 Lines 473120 29541 -443579 Branches 58327 5172 -53155 ============================================
- Hits 71507 1272 -70235 + Misses 393812 28125 -365687 + Partials 7801 144 -7657

Flag	Coverage Δ
uitests	`4.30% <ø> (-0.47%)`	⬇️
unittests	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

sureshanaparti

clgtm

sureshanaparti · 2024-10-03T12:19:58Z

@blueorangutan package

blueorangutan · 2024-10-03T12:22:03Z

@sureshanaparti a [SL] 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 · 2024-10-03T13:25:18Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11265

DaanHoogland

clgtm

yadvr · 2024-10-16T05:16:09Z

@blueorangutan package

blueorangutan · 2024-10-16T05:18:03Z

@rohityadavcloud a [SL] 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 · 2024-10-16T06:20:52Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11356

yadvr · 2024-10-17T09:21:19Z

@blueorangutan test

blueorangutan · 2024-10-17T09:22:03Z

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

JoaoJandre · 2024-10-17T19:58:16Z

+
+CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('volumes','last_id', 'bigint(20) unsigned DEFAULT NULL');


Suggested change
CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('volumes','last_id', 'bigint(20) unsigned DEFAULT NULL');
-- Add last_id to the volumes table
CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('volumes','last_id', 'bigint(20) unsigned DEFAULT NULL');

Looks like maintainers cannot commit/merge suggested changes cc @FelipeM525

Also this should ideally be moved to 4.19.1.0->4.19.2.0 sql path

@FelipeM525 👆

blueorangutan · 2024-10-18T01:02:48Z

[SF] Trillian test result (tid-11680)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 54228 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9759-t11680-kvm-ol8.zip
Smoke tests completed. 131 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_provision_certificate	`Error`	120.43	test_certauthority_root.py
test_02_create_iso_with_checksum_sha256	`Error`	66.64	test_iso.py

…10.sql Co-authored-by: João Jandre <48719461+JoaoJandre@users.noreply.github.com>

vladimirpetrov · 2024-11-13T22:48:25Z

@blueorangutan package

blueorangutan · 2024-11-13T22:50:03Z

@vladimirpetrov a [SL] 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 · 2024-11-13T23:44:14Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11561

vladimirpetrov

LGTM based on manual testing.

weizhouapache · 2024-11-29T15:40:53Z

this will bring troubles for users who use 4.20.0, because 4.20.1 does not have the db changes for them

DaanHoogland · 2024-11-29T16:16:05Z

ok, so we need to revert this, or add a fix in 4.19.2 to 4.20.1. thanks for the headsup @weizhouapache

weizhouapache · 2024-11-29T19:39:52Z

ok, so we need to revert this, or add a fix in 4.19.2 to 4.20.1. thanks for the headsup @weizhouapache

maybe add to both schema-41910to41920.sql (4.19.1 to 4.19.2 upgrade) and schema-42000to42010.sql (for 4.20.0 to 4.20.1 upgrade)
anyway, schema-41900to41910.sql is not the right place to add SQL changes as 4.19.1.0 has been released.

* 4.20: UI: Tooltip on the host information card to display the CPU speed in MHz and the memory value in MB (to 3 decimal places) (#9971) UI: Allow accounts of the `User` type to add other accounts or users to projects through UI (#9927) enable to create VPC portfowarding rules with source cidr (#7081) Add new column `last_id` to the table volumes (#9759) Allow VMWare import via another host (#9787) Linstor: add support for ISO block devices and direct download (#9792) get expunged VM data for job result (#9949) fix section divider display on auth page (#9966)

Co-authored-by: João Jandre <48719461+JoaoJandre@users.noreply.github.com>

* 4.20: UI: Tooltip on the host information card to display the CPU speed in MHz and the memory value in MB (to 3 decimal places) (apache#9971) UI: Allow accounts of the `User` type to add other accounts or users to projects through UI (apache#9927) enable to create VPC portfowarding rules with source cidr (apache#7081) Add new column `last_id` to the table volumes (apache#9759) Allow VMWare import via another host (apache#9787) Linstor: add support for ISO block devices and direct download (apache#9792) get expunged VM data for job result (apache#9949) fix section divider display on auth page (apache#9966)

created a new column called previous_id in the table volumes to make …
e53a1ce
…it easier to track volumes after their migration

boring-cyborgBot added the component:storage label Oct 2, 2024

sureshanaparti approved these changes Oct 3, 2024
View reviewed changes

JoaoJandre modified the milestones: 4.21.0.0, 4.19.2.0 Oct 4, 2024

yadvr reviewed Oct 8, 2024
View reviewed changes

Comment threadengine/schema/src/main/java/com/cloud/storage/VolumeVO.java Outdated

changed the name of the column previous_id to last_id
36c88d1

DaanHoogland changed the title ~~Add new column previous_id to the table volumes~~Add new column last_id to the table volumesOct 8, 2024

DaanHoogland approved these changes Oct 8, 2024
View reviewed changes

yadvr approved these changes Oct 16, 2024
View reviewed changes

JoaoJandre reviewed Oct 17, 2024
View reviewed changes

Update engine/schema/src/main/resources/META-INF/db/schema-41900to419…
0c86f17
…10.sql Co-authored-by: João Jandre <48719461+JoaoJandre@users.noreply.github.com>

DaanHoogland assigned vladimirpetrov Nov 12, 2024

vladimirpetrov approved these changes Nov 14, 2024
View reviewed changes

DaanHoogland added the status:ready-for-merge label Nov 15, 2024

DaanHoogland removed the status:ready-for-merge label Nov 28, 2024

DaanHoogland merged commit a8cb7ab into apache:4.19Nov 28, 2024

bernardodemarco mentioned this pull request Dec 2, 2024
4.20.merge #10013
Closed
14 tasks

This was referenced Dec 4, 2024
move sql code to the right file #10035
Merged
move upgrade code to the right file #10036
Merged

dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Dec 12, 2024
Add new columnlast_idto the table volumes (apache#9759)
ce77e48
Co-authored-by: João Jandre <48719461+JoaoJandre@users.noreply.github.com>


		CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('volumes','last_id', 'bigint(20) unsigned DEFAULT NULL');

Uh oh!

Conversation

FelipeM525 commented Oct 2, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

codecovBot commented Oct 2, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sureshanaparti left a comment

Choose a reason for hiding this comment

Uh oh!

sureshanaparti commented Oct 3, 2024

Uh oh!

blueorangutan commented Oct 3, 2024

Uh oh!

blueorangutan commented Oct 3, 2024

Uh oh!

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

yadvr commented Oct 16, 2024

Uh oh!

blueorangutan commented Oct 16, 2024

Uh oh!

blueorangutan commented Oct 16, 2024

Uh oh!

yadvr commented Oct 17, 2024

Uh oh!

blueorangutan commented Oct 17, 2024

Uh oh!

JoaoJandreOct 17, 2024

Choose a reason for hiding this comment

Uh oh!

yadvrOct 18, 2024

Choose a reason for hiding this comment

Uh oh!

yadvrOct 18, 2024

Choose a reason for hiding this comment

Uh oh!

JoaoJandreOct 21, 2024

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Oct 18, 2024

Uh oh!

vladimirpetrov commented Nov 13, 2024

Uh oh!

blueorangutan commented Nov 13, 2024

Uh oh!

blueorangutan commented Nov 13, 2024

Uh oh!

vladimirpetrov left a comment

Choose a reason for hiding this comment

Uh oh!

weizhouapache commented Nov 29, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DaanHoogland commented Nov 29, 2024

Uh oh!

weizhouapache commented Nov 29, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

FelipeM525 commented Oct 2, 2024•
edited
Loading

codecovBot commented Oct 2, 2024•
edited
Loading

weizhouapache commented Nov 29, 2024•
edited
Loading