update removed column for service and disk offering

[sudo87]

Description

This PR fixes #11508

removed column is not updated for deleteServiceOffering or deleteDiskOffering flow.

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

• BLOCKER
• Critical
• Major
• [] Minor
• Trivial

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

[codecov]

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 16.17%. Comparing base (5837c4f) to head (fcb56de).
⚠️ Report is 3 commits behind head on 4.20.

Files with missing lines	Patch %	Lines
.../cloud/configuration/ConfigurationManagerImpl.java	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##               4.20   #11544   +/-   ##
=========================================
  Coverage     16.17%   16.17%           
- Complexity    13283    13284    +1     
=========================================
  Files          5656     5656           
  Lines        498005   498007    +2     
  Branches      60404    60404           
=========================================
+ Hits          80533    80540    +7     
+ Misses       408510   408505    -5     
  Partials       8962     8962           

Flag	Coverage Δ
uitests	4.00% <ø> (ø)
unittests	17.02% <0.00%> (+<0.01%)	⬆️

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.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[sudo87]

@blueorangutan package

[blueorangutan]

@sudo87 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]

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

[harikrishna-patnala]

@sudo87 There is no check if there any VMs or volumes in use with those offerings, so directly setting removed column will cause issues.

For example

I think setting only state to Inactive is by design from the beginning. If we have to consider this fix, we need to check if there any VMs in use, also clearly mention in the message something like "Use updateServiceOffering with state Inactive" if VMs or volumes are there. I didnt think much about backward compatibility though

[sudo87]

@sudo87 There is no check if there any VMs or volumes in use with those offerings, so directly setting removed column will cause issues.

For example

I think setting only state to Inactive is by design from the beginning. If we have to consider this fix, we need to check if there any VMs in use, also clearly mention in the message something like "Use updateServiceOffering with state Inactive" if VMs or volumes are there. I didnt think much about backward compatibility though

Thank @harikrishna-patnala for reviewing pr. If I understand the issue correctly, service and disk offering remain in use by Instance/volume inspite of being Inactive. In that case, this fix wont suffice and design change will be needed. We can close this pr.

Can we close the issue logged as bug?

[harikrishna-patnala]

@sudo87 yeah some sort of design change may be needed but we can also give it a small try if time permits. If we can look for any resources which are not using those offerings we may try set the removed column then. Overall what checks I can think of are

1. Need to add conditions to check for usages of VMs, volumes, VM and volume snapshots
2. Need to check if this won't affect the usage server. If there are any code references for finding offerings without includingRemoved condition, it needs fixing.
3. We may have to add a new parameter "force" to delete offering APIs, only then we have to set the removed column. This is to address the backward compatibility of the APIs.

Please check if anything else needs to be checked.

[harikrishna-patnala]

@sudo87 also please check my comment here #11508 (comment)

we can somehow make some design changes and make this work like looking for any usages of these offering by VMs or volumes but I have a big concern or doubt on the usage server.

If the final usage records that are produced by usage server does not have to do anything with offering IDs then we can mark them as removed, if not we should not do this. Also I feel this should not be targeted for 4.20.2

[sudo87]

This pr addresses the issue, however contradicts with the design chosen for Service and Disk offering. There will be new pr after handling the concerns raised by @harikrishna-patnala , @weizhouapache.