Skip to content

Load arch data for backup from template during create instance from backup#12801

Open
sudo87 wants to merge 1 commit intoapache:4.22from
shapeblue:fixArchBackupUI
Open

Load arch data for backup from template during create instance from backup#12801
sudo87 wants to merge 1 commit intoapache:4.22from
shapeblue:fixArchBackupUI

Conversation

@sudo87
Copy link
Contributor

@sudo87 sudo87 commented Mar 12, 2026

Description

This PR fixes issue #12444

Arch info is fetched from the template and is used to fetch the relevant templates.

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

Screen.Recording.2026-03-12.at.11.08.11.PM.mov

How Has This Been Tested?

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

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes UI behavior when creating an instance from a backup in multi-arch zones by carrying over the source template’s architecture so template/ISO listings can be filtered consistently.

Changes:

  • Fetch template architecture during “Create VM from Backup” initialization and pass it via dataPreFill.
  • Initialize/adjust selected architecture in the deployment wizard based on prefilled template arch and selected template arch.
  • Remove the zone-selection-time forced architecture reset to avoid overriding the backup/template architecture.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
ui/src/views/storage/CreateVMFromBackup.vue Adds template-arch lookup and passes it to the deploy wizard via dataPreFill.
ui/src/components/view/DeployVMFromBackup.vue Uses prefilled template architecture to set selectedArchitecture, updates it on template selection, and changes zone-selection behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@@ -2344,9 +2346,6 @@ export default {
this.clusterId = null
this.zone = _.find(this.options.zones, (option) => option.id === value)
this.isZoneSelectedMultiArch = this.zone.ismultiarch
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
this.isZoneSelectedMultiArch = this.zone.ismultiarch
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
}

Copilot uses AI. Check for mistakes.
Comment on lines 94 to 100
async created () {
await Promise.all[(
this.fetchServiceOffering(),
this.fetchBackupOffering()
this.fetchBackupOffering(),
this.fetchTemplateArch()
)]
this.loading = false
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
@codecov
Copy link

codecov bot commented Mar 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 17.60%. Comparing base (7aa0558) to head (1cf0ce6).
⚠️ Report is 4 commits behind head on 4.22.

Additional details and impacted files
@@             Coverage Diff              @@
##               4.22   #12801      +/-   ##
============================================
- Coverage     17.61%   17.60%   -0.01%     
+ Complexity    15664    15659       -5     
============================================
  Files          5917     5917              
  Lines        531402   531420      +18     
  Branches      64971    64976       +5     
============================================
- Hits          93596    93565      -31     
- Misses       427252   427299      +47     
- Partials      10554    10556       +2     
Flag Coverage Δ
uitests 3.70% <ø> (-0.01%) ⬇️
unittests 18.67% <ø> (-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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants