Skip to content

Fix disk offering override in VM deployment wizard#8070

Merged
JoaoJandre merged 2 commits intoapache:4.18from
scclouds:fix-root-disk-offering-override
Mar 8, 2024
Merged

Fix disk offering override in VM deployment wizard#8070
JoaoJandre merged 2 commits intoapache:4.18from
scclouds:fix-root-disk-offering-override

Conversation

@winterhazel
Copy link
Copy Markdown
Member

@winterhazel winterhazel commented Oct 10, 2023

Description

This PR fixes four issues related to the disk offering override in the VM deployment wizard:

  1. When a user selects a compute offering associated to a disk offering, enables root disk offering override and selects an offer other than the default one, the UI freezes. This only happens when the UI is not running locally;
  2. When a user enables root disk offering override and selects an offer by clicking inside its radio button, the radio button becomes checked, but the selected root disk offering is not updated and the VM gets deployed with the default offering;
  3. When a user enables root disk offering override, selects an offer, disables the override and deploys a VM, the root disk offering still gets overridden and the VM is deployed with the selected offer;
  4. When a user deploys a VM using an ISO, if the compute offering has been selected before the ISO, and it does not have a compute only disk offering, the selected disk offering always gets overridden by the disk offering associated to the compute offering.

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)

Feature/Enhancement Scale or Bug Severity

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

First, I created two disk offerings:

  1. Name and description: D1. QoS type: Hypervisor. Disk read rate (IOPS) and disk write rate (IOPS): 500;
  2. Name and description: D2. QoS type: Hypervisor. Disk read rate (IOPS) and disk write rate (IOPS): 1000.

And one compute offering:

  • Name and description: C1. CPU cores: 1. CPU (in MHz): 2000. Memory (in MB): 1024. Compute only disk offering disabled. Disk offerings: D1.

Tests related to the UI freezing when selecting a root disk offering:

  • I selected the compute offering Small Instance and deployed a VM. I verified that the root disk did not use any disk offering and had the default size;
  • I selected the compute offering Small Instance, enabled the override, disabled the override and deployed a VM. I verified that the root disk did not use any disk offering and had the default size;
  • I selected the compute offering Small Instance, enabled the override, selected the disk offering D2, changed the disk size to 9 GB and deployed a VM. I verified that the root disk was using D2 and had 9 GB;
  • I selected the compute offering C1 and deployed a VM. I verified that the root disk was using D1 and had the default size;
  • I selected the compute offering C1, enabled the override, disabled the override and deployed a VM. I verified that the root disk was using D1 and had the default size;
  • I selected the compute offering C1, enabled the override, selected the disk offering D2, changed the disk size to 9 GB and deployed a VM. I verified that the root disk was using D2 and had 9 GB;
  • I selected the compute offering C1, enabled the override, selected the disk offering Medium and deployed a VM. I verified that the root disk was using Medium and had 20 GB;

Tests related to the selected root disk offering not updating when clicking inside a radio button:

  • I selected the compute offering C1, enabled the override, selected the disk offering D2 by clicking inside its radio button, changed the disk size to 9 GB and deployed a VM. I verified that the root disk was using D2 and had 9 GB.

Tests related to the root disk offering being overridden even though the override was disabled:

  • I selected the compute offering Small Instance, enabled the override, selected the disk offering D2, changed the disk size to 12 GB, disabled the override and deployed a VM. I verified that the root disk was not using any disk offering and had the default size.
  • I selected the compute offering C1, enabled the override, selected the disk offering D2, disabled the override and deployed a VM. I verified that the root disk was using D1 and had the default size;
  • I selected the compute offering C1, enabled the override, selected the disk offering Medium, disabled the override and deployed a VM. I verified that the root disk was using D1 and had the default size.

Tests related to the disk offering always being overridden when deploying using an ISO:

  • I selected the compute offering C1, an ISO and the disk size D1, changed the disk size to 12 GB and deployed a VM. I verified that the volume was using D1 and had 12 GB;
  • I selected the compute offering C1, an ISO, the disk size Medium and deployed a VM. I verified that the volume was using Medium and had 20 GB.

@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 10, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 13.16%. Comparing base (ce586e3) to head (b08c501).
Report is 35 commits behind head on 4.18.

Additional details and impacted files
@@             Coverage Diff              @@
##               4.18    #8070      +/-   ##
============================================
+ Coverage     13.12%   13.16%   +0.04%     
- Complexity     9135     9203      +68     
============================================
  Files          2720     2724       +4     
  Lines        257662   258137     +475     
  Branches      40174    40235      +61     
============================================
+ Hits          33807    33989     +182     
- Misses       219563   219840     +277     
- Partials       4292     4308      +16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@DaanHoogland DaanHoogland reopened this Oct 11, 2023
@winterhazel winterhazel changed the title Fix root disk offering override in VM deployment wizard Fix disk offering override in VM deployment wizard Nov 27, 2023
@winterhazel
Copy link
Copy Markdown
Member Author

Could someone review this PR?

@DaanHoogland
Copy link
Copy Markdown
Contributor

@winterhazel , I think you should check who recently changed the files you changed and ask them for a review directly. Your call for review in here would not make anybody feel addressed. Use git blame to see who to ask. I see github sugests me, but I have no time for it at the moment.
@blueorangutan ui

@winterhazel
Copy link
Copy Markdown
Member Author

@harikrishna-patnala @shwstppr can you guys review this? Thanks in advance.

Copy link
Copy Markdown
Member

@harikrishna-patnala harikrishna-patnala left a comment

Choose a reason for hiding this comment

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

Thanks for PR @winterhazel

I could reproduce the issues 1 and 2 and verified that it has fixed with this PR. I've not tested issue 4 yet.

Regarding issue 3
"When a user enables root disk offering override, selects an offer, disables the override and deploys a VM, the root disk offering still gets overridden and the VM is deployed with the selected offer;"
I could not reproduce this issue, it is working as expected before and after the fix.

Am I missing anything

  1. created testCO1 compute offering with override strictness false and link testDO1 disk offering
  2. in deploy VM wizard, selected testCO1 and tried overriding the root disk with a differnt disk offering testDO2
  3. Disabled the root disk override
  4. Deployed VM and I could see testDO1 in root disk, which is expected

@harikrishna-patnala
Copy link
Copy Markdown
Member

@winterhazel also can you please change the base branch to 4.18 as I can see this issue in 4.18 as well.

@winterhazel
Copy link
Copy Markdown
Member Author

winterhazel commented Dec 11, 2023

@harikrishna-patnala thanks for testing. Regarding issue 3, when running the UI locally, overridediskofferingid will be set to the selected disk offering's id in step 2; even if you disable the override, it will still have the id of the selected disk offering, but it will get set back to the id of the the selected compute offering's associated disk offering if you interact with the deployment form.

You should be able to reproduce issue 3 the following ways:

(i) by using a compute offering that does not have an associated disk offering.

  1. Select the compute offering Small Instance;
  2. Enable root disk offering override;
  3. Select the disk offering testDO2;
  4. Disable the override. Here you can interact with the rest of the page and the root disk offering will still get overridden;
  5. Deploy a VM by clicking the launch instance button.

(ii) by not interacting with anything other than the launch instance button after disabling the override.

  1. Select the compute offering testCO1;
  2. Enable root disk offering override;
  3. Select the disk offering testDO2;
  4. Disable the override. Do not interact with the deployment form after disabling it;
  5. Deploy a VM by clicking the launch instance button.

Before the fix, the deployed VM's volume will be created using testDO2.

@winterhazel also can you please change the base branch to 4.18 as I can see this issue in 4.18 as well.

Sure, I'll change.

@github-actions
Copy link
Copy Markdown

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

@winterhazel
Copy link
Copy Markdown
Member Author

@DaanHoogland @harikrishna-patnala @shwstppr @weizhouapache, could someone review and validate that issues 3 and 4 have been fixed?

@DaanHoogland
Copy link
Copy Markdown
Contributor

@blueorangutan ui

@blueorangutan
Copy link
Copy Markdown

@DaanHoogland a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

@apache apache deleted a comment from harikrishna-patnala Feb 28, 2024
@apache apache deleted a comment from blueorangutan Feb 28, 2024
@apache apache deleted a comment from blueorangutan Feb 28, 2024
@apache apache deleted a comment from blueorangutan Feb 28, 2024
@apache apache deleted a comment from blueorangutan Feb 28, 2024
@apache apache deleted a comment from blueorangutan Feb 28, 2024
@apache apache deleted a comment from blueorangutan Feb 28, 2024
@apache apache deleted a comment from blueorangutan Feb 28, 2024
@apache apache deleted a comment from shwstppr Feb 28, 2024
@apache apache deleted a comment from blueorangutan Feb 28, 2024
@blueorangutan
Copy link
Copy Markdown

UI build: ✔️
Live QA URL: https://qa.cloudstack.cloud/simulator/pr/8070 (QA-JID-293)

Comment thread ui/src/views/compute/DeployVM.vue Outdated
Comment thread ui/src/views/compute/DeployVM.vue Outdated
Comment thread ui/src/views/compute/DeployVM.vue
@JoaoJandre
Copy link
Copy Markdown
Contributor

@DaanHoogland @BryanMLima @harikrishna-patnala I was able to reproduce issues 3 and 4 in a local lab and validated that both were fixed by this PR.

Copy link
Copy Markdown
Contributor

@JoaoJandre JoaoJandre left a comment

Choose a reason for hiding this comment

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

CLGTM

@JoaoJandre JoaoJandre requested a review from DaanHoogland March 8, 2024 17:46
Copy link
Copy Markdown
Contributor

@BryanMLima BryanMLima left a comment

Choose a reason for hiding this comment

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

CLGTM

@JoaoJandre
Copy link
Copy Markdown
Contributor

Merging based on approvals and manual tests.

@JoaoJandre JoaoJandre merged commit d487a1c into apache:4.18 Mar 8, 2024
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Mar 21, 2024
* Fix disk offering override in VM deployment wizard

* Reduce indentation
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.

7 participants