Decouple service (compute) offering and disk offering by harikrishna-patnala · Pull Request #5008 · apache/cloudstack

harikrishna-patnala · 2021-05-10T11:37:14Z

Description

Currently, our compute offerings and disk offerings are tightly coupled with respect to many aspects. For example, if a compute offering is created, a corresponding disk offering entry is also created with the same ID as the reference. Also creating compute offering takes few disk-related parameters which anyway goes to the corresponding disk offering only. I think this design was initially made to address compute offering for the root volume created from a template. Also changing the offering of a volume is tightly coupled with storage tags and has to be done in different APIs either migrateVolume or resizeVolume. Changing of disk offering should be seamless and should consider new storage tags, new size and place the volume in appropriate state as defined in disk offering.

more details are mentioned here https://cwiki.apache.org/confluence/display/CLOUDSTACK/Compute+offering+and+disk+offering+refactoring

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

Screenshots (if appropriate):

How Has This Been Tested?

harikrishna-patnala · 2021-05-10T17:33:14Z

@blueorangutan package

blueorangutan · 2021-05-10T17:34:03Z

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

blueorangutan · 2021-05-10T23:38:07Z

Packaging result: ✔️ centos7 ✔️ centos8 ✔️ debian. SL-JID 574

GutoVeronezi · 2021-05-17T15:06:43Z

 UniqueNameSearch = createSearchBuilder();
 UniqueNameSearch.and("name", UniqueNameSearch.entity().getUniqueName(), SearchCriteria.Op.EQ);
- UniqueNameSearch.and("system", UniqueNameSearch.entity().isSystemUse(), SearchCriteria.Op.EQ);
+ UniqueNameSearch.and("system_use", UniqueNameSearch.entity().isSystemUse(), SearchCriteria.Op.EQ);


As it will be used more times in this class, could it be extracted to a variable?

harikrishna-patnala · 2021-06-18T05:09:52Z

@blueorangutan package

blueorangutan · 2021-06-18T05:11:03Z

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

blueorangutan · 2021-06-18T05:35:54Z

Packaging result: ✔️ centos7 ✔️ centos8 ✔️ debian. SL-JID 281

harikrishna-patnala · 2021-06-20T19:58:37Z

@blueorangutan package

blueorangutan · 2021-06-20T19:59:03Z

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

blueorangutan · 2021-06-20T20:24:28Z

Packaging result: ✔️ centos7 ✔️ centos8 ✔️ debian. SL-JID 305

harikrishna-patnala · 2021-06-20T20:24:47Z

@blueorangutan test matrix

blueorangutan · 2021-06-20T20:25:03Z

@harikrishna-patnala a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

blueorangutan · 2021-06-21T10:34:36Z

Trillian test result (tid-1020)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 48456 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5008-t1020-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_accounts.py
Intermittent failure detected: /marvin/tests/smoke/test_domain_service_offerings.py
Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Smoke tests completed. 80 look OK, 8 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_01_service_offering_hierarchy	`Failure`	0.02	test_accounts.py
test_01_service_offering_siblings	`Failure`	0.02	test_accounts.py
test_01_create_domain_service_offering	`Error`	0.08	test_domain_service_offerings.py
test_02_edit_domain_service_offering	`Error`	0.06	test_domain_service_offerings.py
test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80	`Failure`	496.71	test_internal_lb.py
test_03_vpc_privategw_restart_vpc_cleanup	`Failure`	369.17	test_privategw_acl.py
test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true	`Failure`	340.16	test_routers_network_ops.py
test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false	`Failure`	337.95	test_routers_network_ops.py
test_07_resize_fail	`Failure`	122.81	test_volumes.py
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL	`Failure`	532.97	test_vpc_redundant.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers	`Failure`	545.35	test_vpc_redundant.py
test_05_rvpc_multi_tiers	`Failure`	486.30	test_vpc_redundant.py
test_05_rvpc_multi_tiers	`Error`	486.31	test_vpc_redundant.py
test_01_vpc_site2site_vpn_multiple_options	`Failure`	462.84	test_vpc_vpn.py

blueorangutan · 2021-06-21T10:57:13Z

Trillian test result (tid-1019)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 50472 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5008-t1019-xenserver-71.zip
Intermittent failure detected: /marvin/tests/smoke/test_accounts.py
Intermittent failure detected: /marvin/tests/smoke/test_domain_service_offerings.py
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Intermittent failure detected: /marvin/tests/smoke/test_host_maintenance.py
Smoke tests completed. 83 look OK, 5 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_01_service_offering_hierarchy	`Failure`	0.02	test_accounts.py
test_01_service_offering_siblings	`Failure`	0.02	test_accounts.py
test_01_create_domain_service_offering	`Error`	0.08	test_domain_service_offerings.py
test_02_edit_domain_service_offering	`Error`	0.06	test_domain_service_offerings.py
test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true	`Error`	39.91	test_routers_network_ops.py
test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false	`Failure`	362.68	test_routers_network_ops.py
test_07_resize_fail	`Failure`	677.33	test_volumes.py
test_07_resize_fail	`Error`	677.33	test_volumes.py
test_08_resize_volume	`Error`	1.07	test_volumes.py
test_10_list_volumes	`Failure`	373.21	test_volumes.py
test_11_attach_volume_with_unstarted_vm	`Error`	2.19	test_volumes.py
ContextSuite context=TestVolumes>:teardown	`Error`	15.61	test_volumes.py
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL	`Failure`	650.79	test_vpc_redundant.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers	`Failure`	681.79	test_vpc_redundant.py
test_04_rvpc_network_garbage_collector_nics	`Failure`	406.85	test_vpc_redundant.py
test_05_rvpc_multi_tiers	`Failure`	574.85	test_vpc_redundant.py
test_05_rvpc_multi_tiers	`Error`	574.87	test_vpc_redundant.py

sureshanaparti · 2021-06-21T13:54:38Z

 ProvisioningType provisioningType, boolean localStorageRequired, boolean offerHA, String tags) {
 tags = cleanupTags(tags);
+ DiskOfferingVO diskOfferingVO = new DiskOfferingVO(name, displayText, provisioningType, false, tags, false, false, false, true);
+ diskOfferingVO.setUniqueName("Cloud.Com-" + name);


is this prefix "Cloud.Com-" required or offerings ?

Here we are splitting the service offering and disk offering, so using the same name for both offerings.

Post upgrade, the existing service offerings and related disk offerings have the same name. While creating a new service offering, this prefix may not be required. what do you think?

Is this just for default offerings?

blueorangutan · 2021-06-21T16:38:38Z

Trillian test result (tid-1021)
Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
Total time taken: 70477 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5008-t1021-vmware-65u2.zip
Intermittent failure detected: /marvin/tests/smoke/test_accounts.py
Intermittent failure detected: /marvin/tests/smoke/test_domain_service_offerings.py
Intermittent failure detected: /marvin/tests/smoke/test_nic.py
Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Intermittent failure detected: /marvin/tests/smoke/test_routers.py
Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
Intermittent failure detected: /marvin/tests/smoke/test_storage_policy.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_deployment_planner.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
Smoke tests completed. 75 look OK, 13 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_01_service_offering_hierarchy	`Failure`	0.02	test_accounts.py
test_01_service_offering_siblings	`Failure`	0.02	test_accounts.py
test_01_create_domain_service_offering	`Error`	0.07	test_domain_service_offerings.py
test_02_edit_domain_service_offering	`Error`	0.05	test_domain_service_offerings.py
ContextSuite context=TestResetVmOnReboot>:setup	`Error`	0.00	test_reset_vm_on_reboot.py
ContextSuite context=TestRAMCPUResourceAccounting>:setup	`Error`	0.00	test_resource_accounting.py
ContextSuite context=TestRouterDHCPHosts>:setup	`Error`	0.00	test_router_dhcphosts.py
ContextSuite context=TestRouterDHCPOpts>:setup	`Error`	0.00	test_router_dhcphosts.py
ContextSuite context=TestRouterDnsService>:setup	`Error`	0.00	test_router_dnsservice.py
ContextSuite context=TestRouterIpTablesPolicies>:setup	`Error`	0.00	test_routers_iptables_default_policy.py
ContextSuite context=TestVPCIpTablesPolicies>:setup	`Error`	0.00	test_routers_iptables_default_policy.py
ContextSuite context=TestIsolatedNetworks>:setup	`Error`	0.00	test_routers_network_ops.py
ContextSuite context=TestRedundantIsolateNetworks>:setup	`Error`	0.00	test_routers_network_ops.py
ContextSuite context=TestRouterServices>:setup	`Error`	0.00	test_routers.py
ContextSuite context=TestServiceOfferings>:setup	`Error`	1518.34	test_service_offerings.py
ContextSuite context=TestSnapshotRootDisk>:setup	`Error`	0.00	test_snapshots.py
test_01_migrate_VM_and_root_volume	`Error`	124.27	test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks	`Error`	109.40	test_vm_life_cycle.py
test_01_vapps_vm_cycle	`Error`	0.21	test_vm_life_cycle.py
test_02_attach_volume	`Failure`	1624.56	test_volumes.py
test_02_attach_volume	`Error`	1624.58	test_volumes.py
test_03_download_attached_volume	`Error`	1.06	test_volumes.py
test_04_delete_attached_volume	`Error`	0.04	test_volumes.py
test_05_detach_volume	`Error`	0.05	test_volumes.py
test_06_download_detached_volume	`Error`	1.06	test_volumes.py
test_07_resize_fail	`Error`	2.17	test_volumes.py
test_08_resize_volume	`Error`	1.06	test_volumes.py
test_11_attach_volume_with_unstarted_vm	`Error`	2.54	test_volumes.py
ContextSuite context=TestVolumes>:teardown	`Error`	19.66	test_volumes.py

GabrielBrascher

@harikrishna-patnala can you please provide more details on this PR goal?
The PR's description does not help at all:

Description
This PR...
Types of changes

PS: I understand that this PR is a Draft. If you plan to enhance the PR description before making it ready-to-review please ignore my comment.

DaanHoogland

looks promising @harikrishna-patnala , thanks for picking this up.
You know I'm going to ask you to factor out the bits that you touched when done, right ? ;)

…fferings API

harikrishna-patnala · 2022-01-25T04:31:07Z

@blueorangutan package

blueorangutan · 2022-01-25T04:32:03Z

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

blueorangutan · 2022-01-25T05:27:03Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 2294

harikrishna-patnala · 2022-01-26T16:18:35Z

@blueorangutan test

blueorangutan · 2022-01-26T16:19:03Z

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

blueorangutan · 2022-01-27T01:56:43Z

Trillian test result (tid-3016)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 33163 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5008-t3016-kvm-centos7.zip
Smoke tests completed. 92 look OK, 0 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File

borisstoyanov

@harikrishna-patnala does this requires any additional testing after fixing the merge conflicts?

harikrishna-patnala · 2022-01-27T09:06:05Z

@borisstoyanov those are very minor conflicts with resizeVolume UI form (only one line), which I've already verified after rebasing. I don't think additional testing is required. Thanks for asking.

borisstoyanov · 2022-01-27T09:17:38Z

ok great!

yadvr · 2022-01-27T09:38:05Z

Last round of smoketests LGTM

harikrishna-patnala force-pushed the decoupleServiceOfferingDiskOffering branch from 3453738 to 296a507Compare May 10, 2021 11:39

harikrishna-patnala closed this May 10, 2021

harikrishna-patnala reopened this May 10, 2021

yadvr added this to the 4.16.0.0 milestone May 10, 2021

GutoVeronezi reviewed May 17, 2021
View reviewed changes

harikrishna-patnala force-pushed the decoupleServiceOfferingDiskOffering branch 2 times, most recently from 1b05122 to dd9dcd7Compare June 18, 2021 05:09

apache deleted a comment from blueorangutan Jun 20, 2021

sureshanaparti changed the title ~~Decouple service offering disk offering~~Decouple service (compute) offering and disk offeringJun 21, 2021

sureshanaparti reviewed Jun 21, 2021
View reviewed changes

Comment threadapi/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java

sureshanaparti reviewed Jun 21, 2021
View reviewed changes

Comment threadapi/src/main/java/org/apache/cloudstack/api/response/DiskOfferingResponse.java Outdated

sureshanaparti reviewed Jun 21, 2021
View reviewed changes

GabrielBrascher reviewed Jun 21, 2021
View reviewed changes

DaanHoogland reviewed Jun 22, 2021
View reviewed changes

Comment threadengine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java Outdated

Fix migratevolume form to pick selected storage pool id in list disko…
c16321f
…fferings API

harikrishna-patnala force-pushed the decoupleServiceOfferingDiskOffering branch from 1bccb83 to c16321fCompare January 25, 2022 03:54

harikrishna-patnala marked this pull request as ready for review January 25, 2022 07:16

apache deleted a comment from blueorangutan Jan 26, 2022

borisstoyanov reviewed Jan 27, 2022
View reviewed changes

NuxRo approved these changes Jan 27, 2022
View reviewed changes

yadvr approved these changes Jan 27, 2022
View reviewed changes

yadvr merged commit f15cab1 into mainJan 27, 2022

yadvr deleted the decoupleServiceOfferingDiskOffering branch January 27, 2022 09:38

This was referenced Feb 17, 2022
Allow scaling VM to a service offering having different storage tags #5996
Merged
regular user has no access to offferings for creating a new volume #6013
Closed

vladimirpetrov mentioned this pull request Mar 21, 2022
Compute offerings are inaccessible as regular user. #6145
Closed

nvazquez mentioned this pull request Mar 21, 2022
Cleanup: Remove redundant API to change service offering for VMs #6150
Closed

yadvr added type:enhancement type:improvement and removed status:work-in-progress labels Mar 24, 2022

nvazquez added the type:new-feature label Mar 30, 2022

weizhouapache mentioned this pull request Oct 19, 2023
ACS 4.18 diskofferingid missing from deployVirtualMachine response #8120
Closed

shwstppr mentioned this pull request Oct 24, 2023
server: fix diskoffering details in vm response #8135
Merged
13 tasks

weizhouapache mentioned this pull request Jun 11, 2024
Disk offering doesn't change when resize operation is performed #8413
Closed

nvazquez mentioned this pull request Mar 11, 2026
regression: disk offering strictness not honored when resizing root volume (4.22.0) #12099
Closed

Uh oh!

Conversation

harikrishna-patnala commented May 10, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

harikrishna-patnala commented May 10, 2021

Uh oh!

blueorangutan commented May 10, 2021

Uh oh!

blueorangutan commented May 10, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GutoVeroneziMay 17, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

harikrishna-patnala commented Jun 18, 2021

Uh oh!

blueorangutan commented Jun 18, 2021

Uh oh!

blueorangutan commented Jun 18, 2021

Uh oh!

harikrishna-patnala commented Jun 20, 2021

Uh oh!

blueorangutan commented Jun 20, 2021

Uh oh!

blueorangutan commented Jun 20, 2021

Uh oh!

harikrishna-patnala commented Jun 20, 2021

Uh oh!

blueorangutan commented Jun 20, 2021

Uh oh!

blueorangutan commented Jun 21, 2021

Uh oh!

blueorangutan commented Jun 21, 2021

Uh oh!

Uh oh!

Uh oh!

sureshanapartiJun 21, 2021

Choose a reason for hiding this comment

Uh oh!

harikrishna-patnalaAug 9, 2021

Choose a reason for hiding this comment

Uh oh!

sureshanapartiAug 10, 2021

Choose a reason for hiding this comment

Uh oh!

yadvrAug 27, 2021

Choose a reason for hiding this comment

Uh oh!

harikrishna-patnalaAug 31, 2021

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Jun 21, 2021

Uh oh!

GabrielBrascher left a comment

Choose a reason for hiding this comment

Description

Types of changes

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

harikrishna-patnala commented Jan 25, 2022

Uh oh!

harikrishna-patnala commented May 10, 2021•
edited
Loading