Guard OS type update for iso/template with existing vms by abh1sar · Pull Request #11215 · apache/cloudstack

abh1sar · 2025-07-16T08:52:46Z

Description

This PR fixes #10588

The behaviour is as follows:
If no VMs are present which were deployed using the ISO/Template - OS type change is allowed

If VMs are present which were deployed using the ISO/Template - OS type change is disallowed but can still be done using the forceupdateostype parameter. In the UI, this field is displayed only if the OS type is changed.

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):

Force option not visible by default

Only visible if the OS type is changed

The error:

How Has This Been Tested?

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

codecov · 2025-07-16T09:00:37Z

Codecov Report

Attention: Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.

Project coverage is 16.57%. Comparing base (6059724) to head (356c03c).
Report is 20 commits behind head on main.

Files with missing lines	Patch %	Lines
...n/java/com/cloud/template/TemplateManagerImpl.java	0.00%	4 Missing ⚠️
...che/cloudstack/api/BaseUpdateTemplateOrIsoCmd.java	0.00%	3 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #11215      +/-   ##
============================================
- Coverage     16.58%   16.57%   -0.01%     
- Complexity    13991    14057      +66     
============================================
  Files          5745     5772      +27     
  Lines        510757   512956    +2199     
  Branches      62144    62308     +164     
============================================
+ Hits          84690    85023     +333     
- Misses       416598   418451    +1853     
- Partials       9469     9482      +13

Flag	Coverage Δ
uitests	`3.89% <ø> (-0.02%)`	⬇️
unittests	`17.47% <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.

abh1sar · 2025-07-16T10:36:29Z

@blueorangutan package

blueorangutan · 2025-07-16T10:38:04Z

@abh1sar 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 · 2025-07-16T12:47:04Z

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

weizhouapache · 2025-07-17T02:35:39Z

server/src/main/java/com/cloud/template/TemplateManagerImpl.java

                sc.addAnd("state", SearchCriteria.Op.NEQ, State.Expunging);
                List<VMInstanceVO> vms = _vmInstanceDao.search(sc, null);
                if (vms != null && !vms.isEmpty()) {
+                    if (!Boolean.TRUE.equals(cmd.getForceUpdateOsType())) {


for backwards compatibility, I suggest to force update vm os types by default.

on UI, we can uncheck the checkbox by default.

@weizhouapache I can make this change, but the issue will still be there for the api calls.
Do you foresee this impacting any workflows? My initial thought was it would be relatively uncommon for users to change the OS type of a template or ISO after deploying VMs from them, so the risk of breaking existing setups seems low. Happy to proceed as you recommend. let me know your thoughts.

@abh1sar
I am good with both.

@DaanHoogland @sureshanaparti
Your advise ?

@abh1sar I would say, keep existing functionality as default in the API as @weizhouapache says. We can change the default at any time if users massively protest.

sureshanaparti · 2025-07-21T14:28:27Z

@blueorangutan package

blueorangutan · 2025-07-21T14:30: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 · 2025-07-21T15:40:22Z

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

sureshanaparti · 2025-07-21T15:55:40Z

@blueorangutan test

blueorangutan · 2025-07-21T15:58:03Z

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

blueorangutan · 2025-07-22T08:30:56Z

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

Test	Result	Time (s)	Test File

RosiKyu

LGTM.

Force update is disabled by default in the UI. Changing the OS type with force update disabled, results in an error (if Instances created from that ISO - exist).

Changing the OS Type with "Force update OS type" enabled - is possible.

Via the API - changing OS type is possible without having to use the 'forceupdateostype' flag. If it is set to 'false' the OS type is not updated.

Same behavior is valid for Templates.

sureshanaparti

clgtm

sureshanaparti

@abh1sar check the cmk error msg cc @rosi-shapeblue

sureshanaparti · 2025-07-22T13:09:34Z

@abh1sar check the cmk error msg cc @rosi-shapeblue

msg looks ok, with the updated results.

* Guard OS type update for iso/template with existing vms * fix identation * rename vm -> instance * force update iso/template as true by default via api * add missing message.success.update.iso label

abh1sar added 3 commits July 16, 2025 14:03

Guard OS type update for iso/template with existing vms

12f2d80

fix identation

b317abf

rename vm -> instance

fb3fc51

boring-cyborg bot added component:api component:UI labels Jul 16, 2025

weizhouapache reviewed Jul 17, 2025

View reviewed changes

abh1sar added 2 commits July 21, 2025 16:52

force update iso/template as true by default via api

1bc93d3

add missing message.success.update.iso label

356c03c

sureshanaparti added the status:needs-testing label Jul 21, 2025

sureshanaparti added this to the 4.21.0 milestone Jul 21, 2025

sureshanaparti added this to Apache CloudStack 4.21.0 Jul 21, 2025

sureshanaparti moved this to In Progress in Apache CloudStack 4.21.0 Jul 21, 2025

abh1sar requested review from DaanHoogland and weizhouapache July 22, 2025 09:46

RosiKyu self-assigned this Jul 22, 2025

RosiKyu approved these changes Jul 22, 2025

View reviewed changes

RosiKyu removed the status:needs-testing label Jul 22, 2025

sureshanaparti approved these changes Jul 22, 2025

View reviewed changes

sureshanaparti requested changes Jul 22, 2025

View reviewed changes

sureshanaparti approved these changes Jul 22, 2025

View reviewed changes

sureshanaparti merged commit e717216 into apache:main Jul 22, 2025
23 of 26 checks passed

github-project-automation bot moved this from In Progress to Done in Apache CloudStack 4.21.0 Jul 22, 2025

sureshanaparti mentioned this pull request Jul 22, 2025

ISO Image OS Type is tied to the Instance, even after the ISO is disconnected, changes affect all Instances #10588

Closed

sureshanaparti changed the title ~~Guard OS type update for iso/template with existing vms~~ Guest OS type update for iso/template with existing vms Jul 22, 2025

sureshanaparti changed the title ~~Guest OS type update for iso/template with existing vms~~ Guard OS type update for iso/template with existing vms Jul 22, 2025

abh1sar deleted the ostype-iso branch July 22, 2025 13:18

Conversation

abh1sar commented Jul 16, 2025

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

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

Uh oh!

codecov bot commented Jul 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

abh1sar commented Jul 16, 2025

Uh oh!

blueorangutan commented Jul 16, 2025

Uh oh!

blueorangutan commented Jul 16, 2025

Uh oh!

weizhouapache Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

abh1sar Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

weizhouapache Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

DaanHoogland Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

abh1sar Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

sureshanaparti commented Jul 21, 2025

Uh oh!

blueorangutan commented Jul 21, 2025

Uh oh!

blueorangutan commented Jul 21, 2025

Uh oh!

sureshanaparti commented Jul 21, 2025

Uh oh!

blueorangutan commented Jul 21, 2025

Uh oh!

blueorangutan commented Jul 22, 2025

Uh oh!

RosiKyu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sureshanaparti left a comment

Choose a reason for hiding this comment

Uh oh!

sureshanaparti left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sureshanaparti commented Jul 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov bot commented Jul 16, 2025 •

edited

Loading

RosiKyu left a comment •

edited

Loading

sureshanaparti left a comment •

edited

Loading