enable update tags on disk offerings - #4194

Merged

DaanHoogland merged 4 commits into

apache:masterfrom

RodrigoDLopez:enable_update_tags_on_disk_offering

Oct 16, 2020

RodrigoDLopez commented Jul 1, 2020

Contributor

Description

On this PR #2636, @rafaelweingartner proposed a consistent validation mechanism to select a storage pool to deploy a volume when a virtual machine is deployed or when a new data disk is allocated.
The following table presents the scenarios when the storage_pool supports or not a new data disk.

#	Disk offering tags	Storage tags	Does the storage support the disk offering?
1	A,B	A	NO
2	A,B,C	A,B,C,D,X	YES
3	A,B,C	X,Y,Z	NO
4	null	A,S,D	YES
5	A	null	NO
6	null	null	YES

With that in mind, this PR enables updateDiskOfferingCmd to edit storage pool tags for diskoffering. Thus, providing greater control to ADMINS decide where disks will be allocated.

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)

Screenshots (if appropriate):

How Has This Been Tested?

To test the PR i created 2 storage pools.
Storage Pools:

spool01 with tag hdd
spool02 with tag hdd2

And create one disk Offering:

Test without tag

Test 01

Create a volume using Test diskoffering
Attach the volume to an instance
Update the disk's diskoffering to another one that has a tag that does not have a corresponding Storage pool
Detach the volume
try to migrate the disk

Expected: we expect an exception since no storage tags match with the diskoffering tag
Result:

Migration target pool [null, tags:null] has no matching tags for volume [small, uuid:ce026b50-8b04-44d6-bb66-3d324b64ca7d, tags:fake]

Test 02

Using the same volume from Test 01
Update the disk's diskoffering tag to hdd2
try to migrate the disk to spool02

Expected: Migrate the volume to spool02 storage.
Result: all good, as expected

enable update tags on disk offerings

e78de2e

This PR enables `updateDiskOfferingCmd` to edit storage pool tags for `diskoffering`. Thus, providing greater control to ADMINS decide where disks will be allocated.
Changes on UI to enable the user to edit diskoffering tags

GabrielBrascher reviewed

View reviewed changes

api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java Outdated

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java Outdated

improve api msg and fix checkstyle issue

ca08e28

GabrielBrascher approved these changes

View reviewed changes

GabrielBrascher left a comment

Member

Code LGTM, thanks for the PR @RodrigoDLopez.

GabrielBrascher assigned RodrigoDLopez

GabrielBrascher added this to the 4.15.0.0 milestone

RodrigoDLopez commented Jul 1, 2020

ContributorAuthor

@blueorangutan package

blueorangutan commented Jul 1, 2020

@RodrigoDLopez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan commented Jul 1, 2020

Packaging result: ✔centos7 ✔debian. JID-1501

svenvogel added type:enhancement component:api component:UI labels

RodrigoDLopez closed this

RodrigoDLopez reopened this

RodrigoDLopez closed this

RodrigoDLopez reopened this

weizhouapache reviewed

View reviewed changes

api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java

add RoleType.Admin when updating tag for diskoffering

d5d13d9

RodrigoDLopez closed this

RodrigoDLopez reopened this

RodrigoDLopez closed this

RodrigoDLopez reopened this

RodrigoDLopez commented Jul 24, 2020

ContributorAuthor

can I get some reviews here, please?

rafaelweingartner requested changes

View reviewed changes

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java Outdated

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java Outdated

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java Outdated

rafaelweingartner reviewed

View reviewed changes

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java Outdated

Extract new methods

c8fa157

add two new methods: updateDiskOfferingTagsIfIsNotNull that will check and set tags to the diskOffering; isUpdateDiskOfferingNeeded that verify if any update is needed.
Create unit tests do cover all those new methods

RodrigoDLopez requested a review from rafaelweingartner

August 2, 2020 15:08

GabrielBrascher commented Aug 5, 2020

Member

@weizhouapache @rafaelweingartner did @RodrigoDLopez addressed your comments? Are you ok with the PR the way it is now?

GabrielBrascher commented Aug 5, 2020

Member

@blueorangutan package

blueorangutan commented Aug 5, 2020

@GabrielBrascher a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan commented Sep 1, 2020

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan commented Sep 1, 2020

Packaging result: ✔centos7 ✖centos8 ✔debian. JID-1869

yadvr commented Sep 2, 2020

Member

@blueorangutan package

blueorangutan commented Sep 4, 2020

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

1 similar comment

blueorangutan commented Sep 4, 2020

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

yadvr commented Sep 5, 2020

Member

@blueorangutan package

blueorangutan commented Sep 5, 2020

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan commented Sep 5, 2020

Packaging result: ✖centos7 ✖centos8 ✖debian. JID-1900

yadvr commented Sep 9, 2020

Member

@blueorangutan package

blueorangutan commented Sep 9, 2020

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan commented Sep 9, 2020

Packaging result: ✔centos7 ✖centos8 ✖debian. JID-1936

yadvr commented Sep 9, 2020

Member

@blueorangutan package

blueorangutan commented Sep 9, 2020

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan commented Sep 9, 2020

Packaging result: ✖centos7 ✖centos8 ✔debian. JID-1942

DaanHoogland commented Sep 18, 2020

Contributor

@blueorangutan package

DaanHoogland closed this

rafaelweingartner commented Sep 23, 2020

Member

@DaanHoogland is there a reason for us to close this PR?

rafaelweingartner reopened this

DaanHoogland commented Sep 23, 2020

Contributor

@DaanHoogland is there a reason for us to close this PR?

other than me making a mistake, @rafaelweingartner ?
@blueorangutan package

blueorangutan commented Sep 23, 2020

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

blueorangutan commented Sep 23, 2020

Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2072

DaanHoogland commented Oct 15, 2020

Contributor

@blueorangutan test

apache deleted a comment from blueorangutan

apache deleted a comment from blueorangutan

apache deleted a comment from blueorangutan

blueorangutan commented Oct 15, 2020

@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan commented Oct 16, 2020

Trillian test result (tid-2973)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 39213 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4194-t2973-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 84 look OK, 1 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_hostha_kvm_host_fencing	`Error`	174.80	test_hostha_kvm.py

DaanHoogland merged commit c222d0b into apache:master

RodrigoDLopez deleted the enable_update_tags_on_disk_offering branch

October 30, 2020 13:46

RodrigoDLopez mentioned this pull request

updateDiskOffering can't add/edit tag on existent disk Offering #4095

Closed

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