Skip to content

Guard OS type update for iso/template with existing vms #11215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sureshanaparti merged 5 commits into from
Jul 22, 2025
Merged

Guard OS type update for iso/template with existing vms #11215

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
12f2d80
Guard OS type update for iso/template with existing vms
abh1sar Jul 16, 2025
b317abf
fix identation
abh1sar Jul 16, 2025
fb3fc51
rename vm -> instance
abh1sar Jul 16, 2025
1bc93d3
force update iso/template as true by default via api
abh1sar Jul 21, 2025
356c03c
add missing message.success.update.iso label
abh1sar Jul 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ public class ApiConstants {
public static final String FORCED_DESTROY_LOCAL_STORAGE = "forcedestroylocalstorage";
public static final String FORCE_DELETE_HOST = "forcedeletehost";
public static final String FORCE_MS_TO_IMPORT_VM_FILES = "forcemstoimportvmfiles";
public static final String FORCE_UPDATE_OS_TYPE = "forceupdateostype";
public static final String FORMAT = "format";
public static final String FOR_VIRTUAL_NETWORK = "forvirtualnetwork";
public static final String FOR_SYSTEM_VMS = "forsystemvms";
Expand Down
8 changes: 8 additions & 0 deletions api/src/main/java/org/apache/cloudstack/api/BaseUpdateTemplateOrIsoCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ public abstract class BaseUpdateTemplateOrIsoCmd extends BaseCmd {
description = "the ID of the OS type that best represents the OS of this image.")
private Long osTypeId;

@Parameter(name = ApiConstants.FORCE_UPDATE_OS_TYPE, type = CommandType.BOOLEAN, since = "4.21", description = "Force OS type update. Warning: Updating OS type will " +
"update the guest OS configuration for all the existing Instances deployed with this template/iso, which may affect their behavior.")
private Boolean forceUpdateOsType;

@Parameter(name = ApiConstants.FORMAT, type = CommandType.STRING, description = "the format for the image")
private String format;

Expand Down Expand Up @@ -112,6 +116,10 @@ public Long getOsTypeId() {
return osTypeId;
}

public Boolean getForceUpdateOsType() {
return forceUpdateOsType;
}

public Boolean getPasswordEnabled() {
return passwordEnabled;
}
Expand Down
6 changes: 6 additions & 0 deletions server/src/main/java/com/cloud/template/TemplateManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2238,6 +2238,12 @@ private VMTemplateVO updateTemplateOrIso(BaseUpdateTemplateOrIsoCmd cmd) {
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())) {
Copy link
Copy Markdown
Member

@weizhouapache weizhouapache Jul 17, 2025

Choose a reason for hiding this comment

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

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

on UI, we can uncheck the checkbox by default.

Copy link
Copy Markdown
Contributor Author

@abh1sar abh1sar Jul 17, 2025

Choose a reason for hiding this comment

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

@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.

Copy link
Copy Markdown
Member

@weizhouapache weizhouapache Jul 17, 2025

Choose a reason for hiding this comment

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

@abh1sar
I am good with both.

@DaanHoogland @sureshanaparti
Your advise ?

Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland Jul 18, 2025

Choose a reason for hiding this comment

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

@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.

Copy link
Copy Markdown
Contributor Author

@abh1sar abh1sar Jul 22, 2025

Choose a reason for hiding this comment

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

Done

String message = String.format("Updating OS type will update the guest OS configuration " +
"for all of the %d Instance(s) deployed with this Template/ISO, which may affect their behavior. " +
"To proceed, please set the 'forceupdateostype' parameter to true.", vms.size());
throw new InvalidParameterValueException(message);
}
for (VMInstanceVO vm: vms) {
vm.setGuestOSId(guestOSId);
_vmInstanceDao.update(vm.getId(), vm);
Expand Down
1 change: 1 addition & 0 deletions ui/public/locales/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1038,6 +1038,7 @@
"label.forbidden": "Forbidden",
"label.forced": "Force",
"label.force.ms.to.import.vm.files": "Enable to force OVF Download via Management Server. Disable to use KVM Host ovftool (if installed)",
"label.force.update.os.type": "Force update OS type",
"label.force.stop": "Force stop",
"label.force.reboot": "Force reboot",
"label.forceencap": "Force UDP encapsulation of ESP packets",
Expand Down
19 changes: 17 additions & 2 deletions ui/src/views/image/UpdateISO.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,13 @@
</a-select-option>
</a-select>
</a-form-item>
<a-form-item name="forceupdateostype" ref="forceupdateostype" v-if="hasOstypeidChanged()">
<template #label>
<tooltip-label :title="$t('label.force.update.os.type')" :tooltip="apiParams.forceupdateostype.description"/>
</template>
<a-switch v-model:checked="form.forceupdateostype" />
</a-form-item>

<a-form-item name="isdynamicallyscalable" ref="isdynamicallyscalable">
<template #label>
<tooltip-label :title="$t('label.isdynamicallyscalable')" :tooltip="apiParams.isdynamicallyscalable.description"/>
Expand Down Expand Up @@ -172,7 +179,8 @@ export default {
userdataid: null,
userdatapolicy: null,
userdatapolicylist: {},
architectureTypes: {}
architectureTypes: {},
originalOstypeid: null
}
},
beforeCreate () {
Expand All @@ -195,7 +203,7 @@ export default {
displaytext: [{ required: true, message: this.$t('message.error.required.input') }],
ostypeid: [{ required: true, message: this.$t('message.error.select') }]
})
const resourceFields = ['name', 'displaytext', 'passwordenabled', 'isdynamicallyscalable', 'ostypeid', 'userdataid', 'userdatapolicy']
const resourceFields = ['name', 'displaytext', 'passwordenabled', 'isdynamicallyscalable', 'ostypeid', 'forceupdateostype', 'userdataid', 'userdatapolicy']

for (var field of resourceFields) {
var fieldValue = this.resource[field]
Expand All @@ -207,13 +215,20 @@ export default {
case 'userdatapolicy':
this.userdatapolicy = fieldValue
break
case 'ostypeid':
this.form[field] = fieldValue
this.originalOstypeid = fieldValue
break
default:
this.form[field] = fieldValue
break
}
}
}
},
hasOstypeidChanged () {
return this.form.ostypeid !== this.originalOstypeid
},
fetchData () {
this.fetchOsTypes()
this.architectureTypes.opts = this.$fetchCpuArchitectureTypes()
Expand Down
18 changes: 17 additions & 1 deletion ui/src/views/image/UpdateTemplate.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,14 @@
</a-select>
</a-form-item>
</a-col>
<a-col :md="24" :lg="24" v-if="hasOstypeidChanged()">
<a-form-item name="forceupdateostype" ref="forceupdateostype">
<template #label>
<tooltip-label :title="$t('label.force.update.os.type')" :tooltip="apiParams.forceupdateostype.description"/>
</template>
<a-switch v-model:checked="form.forceupdateostype" />
</a-form-item>
</a-col>
</a-row>
<a-row :gutter="12">
<a-col :md="24" :lg="12">
Expand Down Expand Up @@ -251,7 +259,8 @@ export default {
userdataid: null,
userdatapolicy: null,
userdatapolicylist: {},
architectureTypes: {}
architectureTypes: {},
originalOstypeid: null
}
},
beforeCreate () {
Expand Down Expand Up @@ -295,6 +304,10 @@ export default {
case 'userdatapolicy':
this.userdatapolicy = fieldValue
break
case 'ostypeid':
this.form[field] = fieldValue
this.originalOstypeid = fieldValue
break
default:
this.form[field] = fieldValue
break
Expand Down Expand Up @@ -546,6 +559,9 @@ export default {
}).finally(() => {
this.loading = false
})
},
hasOstypeidChanged () {
return this.form.ostypeid !== this.originalOstypeid
}
}
}
Expand Down
Loading