cloudstack icon indicating copy to clipboard operation
cloudstack copied to clipboard

Fix disk offering override in VM deployment wizard

Open winterhazel opened this issue 1 year ago • 20 comments

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 selected compute offering 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)
  • [X] 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
  • [X] 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 an ISO, the compute offering C1 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 an ISO, the compute offering C1, the disk size Medium and deployed a VM. I verified that the volume was using Medium and had 20 GB.

winterhazel avatar Oct 10 '23 17:10 winterhazel

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

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     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Oct 10 '23 19:10 codecov[bot]

Could someone review this PR?

winterhazel avatar Nov 29 '23 16:11 winterhazel

@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

DaanHoogland avatar Nov 30 '23 08:11 DaanHoogland

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

winterhazel avatar Dec 01 '23 17:12 winterhazel

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

harikrishna-patnala avatar Dec 09 '23 00:12 harikrishna-patnala

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

winterhazel avatar Dec 11 '23 19:12 winterhazel

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

github-actions[bot] avatar Dec 11 '23 19:12 github-actions[bot]

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

winterhazel avatar Jan 15 '24 17:01 winterhazel

@blueorangutan ui

DaanHoogland avatar Feb 28 '24 14:02 DaanHoogland

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

blueorangutan avatar Feb 28 '24 14:02 blueorangutan

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

blueorangutan avatar Feb 28 '24 14:02 blueorangutan

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

JoaoJandre avatar Mar 08 '24 17:03 JoaoJandre

Merging based on approvals and manual tests.

JoaoJandre avatar Mar 08 '24 19:03 JoaoJandre