Copilot commented on code in PR #12801:
URL: https://github.com/apache/cloudstack/pull/12801#discussion_r2926320624
##########
ui/src/components/view/DeployVMFromBackup.vue:
##########
@@ -2344,9 +2346,6 @@ export default {
this.clusterId = null
this.zone = _.find(this.options.zones, (option) => option.id === value)
this.isZoneSelectedMultiArch = this.zone.ismultiarch
Review Comment:
Removing the multi-arch defaulting in `onSelectZoneId` can leave
`selectedArchitecture` as `null` for the ISO flow (`dataPreFill.isIso ===
true`), because `created()` only initializes `selectedArchitecture` in the
template tab. `fetchIsos()` will still add `args.architecture =
this.selectedArchitecture` whenever `isZoneSelectedMultiArch` is true, so
multi-arch zones may end up calling `listIsos` with an empty/null architecture
filter and return no results. Fix by ensuring `selectedArchitecture` is
initialized (e.g. to `architectureTypes.opts[0].id`) when entering a multi-arch
zone if it’s currently unset, or by only sending the architecture param when it
has a value.
```suggestion
this.isZoneSelectedMultiArch = this.zone.ismultiarch
if (this.isZoneSelectedMultiArch &&
!this.selectedArchitecture &&
this.architectureTypes &&
Array.isArray(this.architectureTypes.opts) &&
this.architectureTypes.opts.length > 0) {
this.selectedArchitecture = this.architectureTypes.opts[0].id
}
```
##########
ui/src/views/storage/CreateVMFromBackup.vue:
##########
@@ -94,7 +94,8 @@ export default {
async created () {
await Promise.all[(
this.fetchServiceOffering(),
- this.fetchBackupOffering()
+ this.fetchBackupOffering(),
+ this.fetchTemplateArch()
)]
this.loading = false
Review Comment:
`Promise.all` is being indexed (`Promise.all[( ... )]`) instead of invoked
with an array (`Promise.all([ ... ])`). This means the three fetch calls are
not properly awaited (the bracket expression evaluates to `undefined`), and
`loading` can be set to `false` before the API calls finish, leaving
`serviceOffering`/`backupOffering`/`templateArch` unset when the UI renders.
##########
ui/src/views/storage/CreateVMFromBackup.vue:
##########
@@ -118,6 +119,18 @@ export default {
this.backupOffering = backupOfferings[0]
})
},
+ fetchTemplateArch () {
+ return getAPI('listTemplates', {
+ id: this.resource.vmdetails.templateid,
+ listall: true,
+ templatefilter: 'all'
+ }).then(response => {
+ const templates = response.listtemplatesresponse.template || []
+ this.templateArch = templates[0]?.arch || 'x86_64'
+ }).catch(() => {
+ this.templateArch = 'x86_64'
+ })
Review Comment:
`fetchTemplateArch()` falls back to hard-coding `'x86_64'` both when
`listTemplates` returns no results and on API errors. In a multi-arch
deployment this can incorrectly force the x86_64 architecture and hide the real
problem (e.g. template not visible/available), reintroducing the “random arch”
behavior for ARM backups. Consider leaving `templateArch` unset (so the user
can choose) and/or showing an error/notification when the template arch cannot
be determined, rather than silently defaulting to x86_64.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
