Skip to content

Conversation

@BryanMLima
Copy link
Contributor

Description

While trying to update a disk offering through the UI, the tag fields are always sent, even when they are not changed. This causes a bug when active volumes are using a disk offering in storage pools with tags. As ACS would interpret this as an update on the tags as well. This PR aims to fix this problem, only sending the tags if they are different from the current tags, allowing the user to update the name and/or description of disk offerings, as it is already allowed through CloudMonkey.

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

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

On a local lab, I tried updating the disk offering with the following use cases:

  • updating only the name
  • updating only the description
  • updating only the name and description fields

All of the tests worked as expected, the UI only sent the the fields that were changed.

Then, while trying to update the name and/or description fields, and changing the tags as well, the API was sending the tags parameters, as expected.

@acs-robot
Copy link

Found UI changes, kicking a new UI QA build
@blueorangutan ui

@blueorangutan
Copy link

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

@blueorangutan
Copy link

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6410 (SL-JID-1619)

Copy link

@utchoang utchoang left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@GabrielBrascher GabrielBrascher left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@GutoVeronezi GutoVeronezi left a comment

Choose a reason for hiding this comment

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

LGTM

@shwstppr
Copy link
Contributor

@BryanMLima Similar issue is with updateServiceOffering for host and storage tags, is it possible to add check for that as well?

@acs-robot
Copy link

Found UI changes, kicking a new UI QA build
@blueorangutan ui

@BryanMLima
Copy link
Contributor Author

@shwstppr I added the verification for service offerings as well. I tested on my lab, and worked as intended.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@DaanHoogland DaanHoogland added this to the 4.18.0.0 milestone Jun 27, 2022
@DaanHoogland DaanHoogland merged commit e1fb2be into apache:main Jun 27, 2022
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.

8 participants