Add support to clone existing offerings and update them#12357
Add support to clone existing offerings and update them#12357
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #12357 +/- ##
============================================
+ Coverage 17.93% 18.00% +0.07%
- Complexity 16160 16349 +189
============================================
Files 5939 5953 +14
Lines 533147 535395 +2248
Branches 65237 65775 +538
============================================
+ Hits 95607 96403 +796
- Misses 426797 428105 +1308
- Partials 10743 10887 +144
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Pearl1594
force-pushed
the
clone-edit-existing-offerings
branch
from
62368f3 to
3787aaa
Compare
Pearl1594
force-pushed
the
clone-edit-existing-offerings
branch
2 times, most recently
from
bb2e90d to
7fd4567
Compare
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
There was a problem hiding this comment.
could you make the new if and for blocks separte methods (with clear names please?
Pearl1594
force-pushed
the
clone-edit-existing-offerings
branch
3 times, most recently
from
1f5796d to
10333df
Compare
weizhouapache
requested review from
Copilot and
weizhouapache
and removed request for
ustcweizhou
There was a problem hiding this comment.
Pull request overview
This PR introduces “clone offering” functionality across the CloudStack API and UI, enabling users to clone existing offerings (compute/service, disk, network, VPC, backup) while overriding selected parameters. It also refactors network/VPC service-capability param building into shared UI helpers and wires the new actions into the offerings UI.
Changes:
- Added new API commands and server implementations for cloning multiple offering types (plus event types and service interfaces).
- Added new UI clone views (compute/disk/backup shown here) and integrated clone actions into the offerings section config.
- Refactored network/VPC service-capability parameter construction into a shared composable and extracted disk offering form into a reusable component.
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/views/offering/CloneDiskOffering.vue | New UI flow to clone disk offerings using the shared DiskOfferingForm. |
| ui/src/views/offering/CloneComputeOffering.vue | New UI flow to clone compute/service offerings (includes prefill + submit logic). |
| ui/src/views/offering/CloneBackupOffering.vue | New UI flow to clone backup offerings (zone/provider offering selection + async job polling). |
| ui/src/views/offering/AddVpcOffering.vue | Refactors VPC service capability param building into a shared helper. |
| ui/src/views/offering/AddNetworkOffering.vue | Refactors network service capability param building into a shared helper. |
| ui/src/views/offering/AddDiskOffering.vue | Refactors disk offering creation to use the new shared DiskOfferingForm component. |
| ui/src/config/section/offering.js | Adds new “Clone * Offering” actions in the UI offerings sections. |
| ui/src/composables/useServiceCapabilityParams.js | New shared helper(s) to build serviceCapability/serviceProvider param sets for network/VPC offerings. |
| ui/src/components/offering/DiskOfferingForm.vue | New reusable disk offering form component extracted from the previous view. |
| ui/src/components/CheckBoxSelectPair.vue | Adds support for setting a default select value. |
| ui/public/locales/en.json | Adds i18n strings for clone offering actions and messages. |
| server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java | Adds tests asserting cloned backup offering domain handling. |
| server/src/test/java/com/cloud/vpc/MockConfigurationManagerImpl.java | Updates test mock to support new clone APIs and new base cmd type for network offering creation. |
| server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java | Adds tests around reflection helpers and some clone-related scaffolding. |
| server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java | Implements backup offering clone behavior and registers the command. |
| server/src/main/java/com/cloud/server/ManagementServerImpl.java | Registers new clone commands in management server command list. |
| server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java | Implements VPC offering clone logic, including service/provider/capability reconstruction. |
| api/src/test/java/org/apache/cloudstack/api/command/admin/vpc/CloneVpcOfferingCmdTest.java | Adds unit tests for the new clone VPC offering command. |
| api/src/test/java/org/apache/cloudstack/api/command/admin/offering/CloneServiceOfferingCmdTest.java | Adds unit tests for the new clone service offering command. |
| api/src/test/java/org/apache/cloudstack/api/command/admin/network/CloneNetworkOfferingCmdTest.java | Adds unit tests for the new clone network offering command. |
| api/src/test/java/org/apache/cloudstack/api/command/admin/backup/CloneBackupOfferingCmdTest.java | Adds unit tests for the new clone backup offering command. |
| api/src/main/java/org/apache/cloudstack/backup/BackupManager.java | Adds cloneBackupOffering to the backup manager interface. |
| api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CreateVPCOfferingCmd.java | Adjusts supported-services behavior for external providers (validation moved server-side). |
| api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CloneVPCOfferingCmd.java | New API command for cloning VPC offerings. |
| api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CloneServiceOfferingCmd.java | New API command for cloning service offerings. |
| api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CloneDiskOfferingCmd.java | New API command for cloning disk offerings. |
| api/src/main/java/org/apache/cloudstack/api/command/admin/network/NetworkOfferingBaseCmd.java | New base command consolidating shared network-offering params/logic. |
| api/src/main/java/org/apache/cloudstack/api/command/admin/network/CreateNetworkOfferingCmd.java | Refactors create network offering cmd to extend NetworkOfferingBaseCmd. |
| api/src/main/java/org/apache/cloudstack/api/command/admin/network/CloneNetworkOfferingCmd.java | New API command for cloning network offerings. |
| api/src/main/java/org/apache/cloudstack/api/command/admin/backup/ImportBackupOfferingCmd.java | Makes backupManager protected and annotates domainIds param with since. |
| api/src/main/java/org/apache/cloudstack/api/command/admin/backup/CloneBackupOfferingCmd.java | New API command for cloning backup offerings (domainIds resolver support). |
| api/src/main/java/org/apache/cloudstack/api/ApiConstants.java | Adds SOURCE_OFFERING_ID constant. |
| api/src/main/java/com/cloud/network/vpc/VpcProvisioningService.java | Adds cloneVPCOffering to VPC provisioning service interface. |
| api/src/main/java/com/cloud/event/EventTypes.java | Adds new event types for offering/backup offering clone. |
| api/src/main/java/com/cloud/configuration/ConfigurationService.java | Adds clone methods and generalizes createNetworkOffering to accept the new base cmd. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
api/src/main/java/org/apache/cloudstack/api/command/admin/backup/CloneBackupOfferingCmd.java
Outdated
Show resolved
Hide resolved
| export default { | ||
| name: 'CreateComputeOffering', | ||
| mixins: [mixinForm], | ||
| components: { |
There was a problem hiding this comment.
The component is registered with name 'CreateComputeOffering', but this view is for cloning. This breaks devtools/component identification and may affect caching/keep-alive behavior if other components rely on the name. Rename it to 'CloneComputeOffering' (or another clone-specific name).
| } | ||
|
|
||
| params.sourceofferingid = this.resource.id | ||
|
|
||
| postAPI('cloneServiceOffering', params).then(json => { | ||
| const message = this.isSystem |
There was a problem hiding this comment.
this.loading is never set to true before calling postAPI('cloneServiceOffering', ...), so the submit-guard (if (this.loading) return) does not prevent double submits and the UI spinner never activates. Set this.loading = true immediately before the API call (and keep the existing finally that resets it).
api/src/main/java/org/apache/cloudstack/api/command/admin/backup/CloneBackupOfferingCmd.java
Outdated
Show resolved
Hide resolved
| }).then(json => { | ||
| this.diskOfferings = json.listdiskofferingsresponse.diskoffering || [] | ||
| if (this.selectedDiskOfferingId === '') { | ||
| this.selectedDiskOfferingId = this.diskOfferings[0].id || '' | ||
| } |
There was a problem hiding this comment.
this.diskOfferings[0].id will throw if listDiskOfferings returns an empty list. Guard with a length check (or use optional chaining) before reading the first element.
| <a-switch v-model:checked="form.encryptdisk" :checked="encryptdisk" @change="val => { encryptdisk = val }" /> | ||
| </a-form-item> | ||
| <a-form-item name="disksizestrictness" ref="disksizestrictness"> | ||
| <template #label> | ||
| <tooltip-label :title="$t('label.disksizestrictness')" :tooltip="apiParams.disksizestrictness.description" /> | ||
| </template> | ||
| <a-switch v-model:checked="form.disksizestrictness" :checked="disksizestrictness" @change="val => { disksizestrictness = val }" /> |
There was a problem hiding this comment.
The switch uses both v-model:checked and an explicit :checked bound to a different data property (encryptdisk). This can desynchronize the UI from form.encryptdisk (especially when initialValues sets a non-default). Prefer binding only via v-model:checked and remove the separate :checked/@change state mirror.
| <a-switch v-model:checked="form.encryptdisk" :checked="encryptdisk" @change="val => { encryptdisk = val }" /> | |
| </a-form-item> | |
| <a-form-item name="disksizestrictness" ref="disksizestrictness"> | |
| <template #label> | |
| <tooltip-label :title="$t('label.disksizestrictness')" :tooltip="apiParams.disksizestrictness.description" /> | |
| </template> | |
| <a-switch v-model:checked="form.disksizestrictness" :checked="disksizestrictness" @change="val => { disksizestrictness = val }" /> | |
| <a-switch v-model:checked="form.encryptdisk" /> | |
| </a-form-item> | |
| <a-form-item name="disksizestrictness" ref="disksizestrictness"> | |
| <template #label> | |
| <tooltip-label :title="$t('label.disksizestrictness')" :tooltip="apiParams.disksizestrictness.description" /> | |
| </template> | |
| <a-switch v-model:checked="form.disksizestrictness" /> |
| <a-switch v-model:checked="form.encryptdisk" :checked="encryptdisk" @change="val => { encryptdisk = val }" /> | ||
| </a-form-item> | ||
| <a-form-item name="disksizestrictness" ref="disksizestrictness"> | ||
| <template #label> | ||
| <tooltip-label :title="$t('label.disksizestrictness')" :tooltip="apiParams.disksizestrictness.description" /> | ||
| </template> | ||
| <a-switch v-model:checked="form.disksizestrictness" :checked="disksizestrictness" @change="val => { disksizestrictness = val }" /> |
There was a problem hiding this comment.
Same issue as encrypt switch: v-model:checked plus explicit :checked backed by disksizestrictness can cause the displayed value to diverge from form.disksizestrictness / initialValues. Bind the switch to a single source of truth (the form model).
| <a-switch v-model:checked="form.encryptdisk" :checked="encryptdisk" @change="val => { encryptdisk = val }" /> | |
| </a-form-item> | |
| <a-form-item name="disksizestrictness" ref="disksizestrictness"> | |
| <template #label> | |
| <tooltip-label :title="$t('label.disksizestrictness')" :tooltip="apiParams.disksizestrictness.description" /> | |
| </template> | |
| <a-switch v-model:checked="form.disksizestrictness" :checked="disksizestrictness" @change="val => { disksizestrictness = val }" /> | |
| <a-switch v-model:checked="form.encryptdisk" /> | |
| </a-form-item> | |
| <a-form-item name="disksizestrictness" ref="disksizestrictness"> | |
| <template #label> | |
| <tooltip-label :title="$t('label.disksizestrictness')" :tooltip="apiParams.disksizestrictness.description" /> | |
| </template> | |
| <a-switch v-model:checked="form.disksizestrictness" /> |
Pearl1594
force-pushed
the
clone-edit-existing-offerings
branch
from
a2179dc to
d53591c
Compare
|
@blueorangutan package |
|
@Pearl1594 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16963 |
|
@blueorangutan test |
|
@Pearl1594 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian Build Failed (tid-15538) |
|
@blueorangutan test |
|
@kiranchavala a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
|
[SF] Trillian test result (tid-15543)
|
|
@blueorangutan package |
|
@kiranchavala a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16997 |
|
@blueorangutan test |
|
@kiranchavala a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
kiranchavala
left a comment
kiranchavala
left a comment
There was a problem hiding this comment.
LGTM Tested manually
| Test Case Execution | Result |
|---|---|
| Test clone offering for compute offering | Pass |
| Test clone offering on system offerings | Pass |
| Test clone offering on disk offerings | Pass |
| Test clone offering on backup offering | Pass |
| Test clone offering on network offerings | Pass |
| Test clone offering on vpc offerings | Pass |
| Test copy of service ( compute offering ) from domainadmin user role | Pass |
| Test copy of service ( diskoffering ) from domainadmin user role | Pass |
|
@Pearl1594 , is this ready for merge? |
|
@DaanHoogland just waiting for test results and comments.. Probably would be ready by early next week. |
|
[SF] Trillian test result (tid-15561)
|
|
@Pearl1594 there are some test failures and extra simulator test run failures than the main branch health check PR. Can you please check and confirm if they are unrelated or not. |
|
@harikrishna-patnala the smoke test failures seem to be happening upstream on other PRs too: #12680 (comment) I have initiated re-run of the ci tests, will keep an eye |
|
@harikrishna-patnala the smoke test and ci test failures are unrelated to the PR as far as I've checked. |
Description
This PR adds APIs to clone existing offerings - service, system, network, VPC, backup and update certain parameters of the existing offering.
Doc PR: apache/cloudstack-documentation#630
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?