You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "hsato03 (via GitHub)" <gi...@apache.org> on 2025/10/08 21:41:13 UTC
[PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
hsato03 opened a new pull request, #11810:
URL: https://github.com/apache/cloudstack/pull/11810
### Description
This PR adds two new preset variables for `Network` and `VPC` usage type Quota tariffs.
For `Network` tariffs, the variables `value.networkOffering.id` and `value.networkOffering.name` have been created. For `VPC` tariffs, the variables `value.vpcOffering.id` and `value.vpcOffering.name` have been added.
They represent the resource's offering ID and name, respectively.
### 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)
- [X] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
- [ ] Build/CI
- [ ] Test (unit or integration test code)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [ ] Major
- [X] Minor
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [ ] Minor
- [ ] Trivial
### Screenshots (if appropriate):
### How Has This Been Tested?
I created an isolated network and a VPC with the following offerings.
| Offering | ID | Name |
| ------ | ------ | ------ |
| Network | 30b8c61a-f75a-4b65-8ff3-52a20b179333 | DefaultIsolatedNetworkOfferingWithSourceNatService
| VPC | 2128acd0-9753-4e2a-8c99-2815da566873 | Default VPC offering |
Then, I created a `Network` tariff (`offering-test-network`) with the following activation rule.
```js
if (value.networkOffering.id === '30b8c61a-f75a-4b65-8ff3-52a20b179333 && value.networkOffering.name === 'DefaultIsolatedNetworkOfferingWithSourceNatService') {
300
} else {
0
}
```
Next, I created a `VPC` tariff (`offering-test-vpc`) with the following activation rule.
```js
if (value.vpcOffering.id === '2128acd0-9753-4e2a-8c99-2815da566873 && value.vpcOffering.name === 'Default VPC offering') {
500
} else {
0
}
```
I called the `quotaUpdate` API and verified that the value 300 was returned by the `offering-test-network` activation rule, while `offering-test-vpc` returned 500.
#### How did you try to break this feature and the system with this change?
<!-- see how your change affects other areas of the code, etc. -->
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #11810:
URL: https://github.com/apache/cloudstack/pull/11810#issuecomment-3383335490
@hsato03 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
Posted by "hsato03 (via GitHub)" <gi...@apache.org>.
hsato03 commented on PR #11810:
URL: https://github.com/apache/cloudstack/pull/11810#issuecomment-4156047502
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #11810:
URL: https://github.com/apache/cloudstack/pull/11810#issuecomment-3769501974
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
Posted by "winterhazel (via GitHub)" <gi...@apache.org>.
winterhazel commented on code in PR #11810:
URL: https://github.com/apache/cloudstack/pull/11810#discussion_r3005386092
##########
framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelperTest.java:
##########
@@ -1289,4 +1294,36 @@ public void testGetSnapshotImageStoreRefNotNull() {
Mockito.when(imageStoreDaoMock.findById(1L)).thenReturn(store);
Assert.assertNotNull(presetVariableHelperSpy.getSnapshotImageStoreRef(1L, 1L));
}
+
+ @Test
+ public void getPresetVariableValueNetworkOfferingTestSetValuesAndReturnObject() {
+ NetworkOfferingVO networkOfferingVoMock = Mockito.mock(NetworkOfferingVO.class);
+ Mockito.doReturn(networkOfferingVoMock).when(networkOfferingDaoMock).findByIdIncludingRemoved(Mockito.anyLong());
+ mockMethodValidateIfObjectIsNull();
+
+ GenericPresetVariable expected = getGenericPresetVariableForTests();
+ Mockito.doReturn(expected.getId()).when(networkOfferingVoMock).getUuid();
+ Mockito.doReturn(expected.getName()).when(networkOfferingVoMock).getName();
+
+ GenericPresetVariable result = presetVariableHelperSpy.getPresetVariableValueNetworkOffering(1L);
+
+ assertPresetVariableIdAndName(expected, result);
+ validateFieldNamesToIncludeInToString(Arrays.asList("id", "name"), result);
Review Comment:
```suggestion
```
This method does not exist anymore.
##########
framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelperTest.java:
##########
@@ -1289,4 +1294,36 @@ public void testGetSnapshotImageStoreRefNotNull() {
Mockito.when(imageStoreDaoMock.findById(1L)).thenReturn(store);
Assert.assertNotNull(presetVariableHelperSpy.getSnapshotImageStoreRef(1L, 1L));
}
+
+ @Test
+ public void getPresetVariableValueNetworkOfferingTestSetValuesAndReturnObject() {
+ NetworkOfferingVO networkOfferingVoMock = Mockito.mock(NetworkOfferingVO.class);
+ Mockito.doReturn(networkOfferingVoMock).when(networkOfferingDaoMock).findByIdIncludingRemoved(Mockito.anyLong());
+ mockMethodValidateIfObjectIsNull();
+
+ GenericPresetVariable expected = getGenericPresetVariableForTests();
+ Mockito.doReturn(expected.getId()).when(networkOfferingVoMock).getUuid();
+ Mockito.doReturn(expected.getName()).when(networkOfferingVoMock).getName();
+
+ GenericPresetVariable result = presetVariableHelperSpy.getPresetVariableValueNetworkOffering(1L);
+
+ assertPresetVariableIdAndName(expected, result);
+ validateFieldNamesToIncludeInToString(Arrays.asList("id", "name"), result);
+ }
+
+ @Test
+ public void getPresetVariableValueVpcOfferingTestSetValuesAndReturnObject() {
+ VpcOfferingVO vpcOfferingVoMock = Mockito.mock(VpcOfferingVO.class);
+ Mockito.doReturn(vpcOfferingVoMock).when(vpcOfferingDao).findByIdIncludingRemoved(Mockito.anyLong());
+ mockMethodValidateIfObjectIsNull();
+
+ GenericPresetVariable expected = getGenericPresetVariableForTests();
+ Mockito.doReturn(expected.getId()).when(vpcOfferingVoMock).getUuid();
+ Mockito.doReturn(expected.getName()).when(vpcOfferingVoMock).getName();
+
+ GenericPresetVariable result = presetVariableHelperSpy.getPresetVariableValueVpcOffering(1L);
+
+ assertPresetVariableIdAndName(expected, result);
+ validateFieldNamesToIncludeInToString(Arrays.asList("id", "name"), result);
Review Comment:
```suggestion
```
Same as above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #11810:
URL: https://github.com/apache/cloudstack/pull/11810#issuecomment-3383354773
## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/11810?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 3.60%. Comparing base ([`e6c7a71`](https://app.codecov.io/gh/apache/cloudstack/commit/e6c7a71d40e5dd0272df718f9b2231e6f9461876?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`e443959`](https://app.codecov.io/gh/apache/cloudstack/commit/e44395915b4a981723d7dba6cdd7a41c5ec7b7f5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 58 commits behind head on main.
> :exclamation: There is a different number of reports uploaded between BASE (e6c7a71) and HEAD (e443959). Click for more details.
>
> <details><summary>HEAD has 1 upload less than BASE</summary>
>
>| Flag | BASE (e6c7a71) | HEAD (e443959) |
>|------|------|------|
>|unittests|1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #11810 +/- ##
=============================================
- Coverage 17.39% 3.60% -13.79%
=============================================
Files 5889 442 -5447
Lines 526184 37279 -488905
Branches 64242 6840 -57402
=============================================
- Hits 91542 1345 -90197
+ Misses 424298 35771 -388527
+ Partials 10344 163 -10181
```
| [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/11810/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/11810/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `3.60% <ø> (-0.02%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/cloudstack/pull/11810/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/11810?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #11810:
URL: https://github.com/apache/cloudstack/pull/11810#issuecomment-3769982141
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
Posted by "winterhazel (via GitHub)" <gi...@apache.org>.
winterhazel commented on code in PR #11810:
URL: https://github.com/apache/cloudstack/pull/11810#discussion_r3005386183
##########
framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelperTest.java:
##########
@@ -1289,4 +1294,36 @@ public void testGetSnapshotImageStoreRefNotNull() {
Mockito.when(imageStoreDaoMock.findById(1L)).thenReturn(store);
Assert.assertNotNull(presetVariableHelperSpy.getSnapshotImageStoreRef(1L, 1L));
}
+
+ @Test
+ public void getPresetVariableValueNetworkOfferingTestSetValuesAndReturnObject() {
+ NetworkOfferingVO networkOfferingVoMock = Mockito.mock(NetworkOfferingVO.class);
+ Mockito.doReturn(networkOfferingVoMock).when(networkOfferingDaoMock).findByIdIncludingRemoved(Mockito.anyLong());
+ mockMethodValidateIfObjectIsNull();
+
+ GenericPresetVariable expected = getGenericPresetVariableForTests();
+ Mockito.doReturn(expected.getId()).when(networkOfferingVoMock).getUuid();
+ Mockito.doReturn(expected.getName()).when(networkOfferingVoMock).getName();
+
+ GenericPresetVariable result = presetVariableHelperSpy.getPresetVariableValueNetworkOffering(1L);
+
+ assertPresetVariableIdAndName(expected, result);
+ validateFieldNamesToIncludeInToString(Arrays.asList("id", "name"), result);
+ }
+
+ @Test
+ public void getPresetVariableValueVpcOfferingTestSetValuesAndReturnObject() {
+ VpcOfferingVO vpcOfferingVoMock = Mockito.mock(VpcOfferingVO.class);
+ Mockito.doReturn(vpcOfferingVoMock).when(vpcOfferingDao).findByIdIncludingRemoved(Mockito.anyLong());
+ mockMethodValidateIfObjectIsNull();
+
+ GenericPresetVariable expected = getGenericPresetVariableForTests();
+ Mockito.doReturn(expected.getId()).when(vpcOfferingVoMock).getUuid();
+ Mockito.doReturn(expected.getName()).when(vpcOfferingVoMock).getName();
+
+ GenericPresetVariable result = presetVariableHelperSpy.getPresetVariableValueVpcOffering(1L);
+
+ assertPresetVariableIdAndName(expected, result);
+ validateFieldNamesToIncludeInToString(Arrays.asList("id", "name"), result);
Review Comment:
Same as above.
##########
framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Value.java:
##########
@@ -255,4 +261,22 @@ public String getState() {
public void setState(String state) {
this.state = state;
}
+
+ public GenericPresetVariable getNetworkOffering() {
+ return networkOffering;
+ }
+
+ public void setNetworkOffering(GenericPresetVariable networkOffering) {
+ this.networkOffering = networkOffering;
+ fieldNamesToIncludeInToString.add("networkOffering");
Review Comment:
```suggestion
```
This is not used anymore.
##########
framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelperTest.java:
##########
@@ -1289,4 +1294,36 @@ public void testGetSnapshotImageStoreRefNotNull() {
Mockito.when(imageStoreDaoMock.findById(1L)).thenReturn(store);
Assert.assertNotNull(presetVariableHelperSpy.getSnapshotImageStoreRef(1L, 1L));
}
+
+ @Test
+ public void getPresetVariableValueNetworkOfferingTestSetValuesAndReturnObject() {
+ NetworkOfferingVO networkOfferingVoMock = Mockito.mock(NetworkOfferingVO.class);
+ Mockito.doReturn(networkOfferingVoMock).when(networkOfferingDaoMock).findByIdIncludingRemoved(Mockito.anyLong());
+ mockMethodValidateIfObjectIsNull();
+
+ GenericPresetVariable expected = getGenericPresetVariableForTests();
+ Mockito.doReturn(expected.getId()).when(networkOfferingVoMock).getUuid();
+ Mockito.doReturn(expected.getName()).when(networkOfferingVoMock).getName();
+
+ GenericPresetVariable result = presetVariableHelperSpy.getPresetVariableValueNetworkOffering(1L);
+
+ assertPresetVariableIdAndName(expected, result);
+ validateFieldNamesToIncludeInToString(Arrays.asList("id", "name"), result);
Review Comment:
This method does not exist anymore.
##########
framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Value.java:
##########
@@ -255,4 +261,22 @@ public String getState() {
public void setState(String state) {
this.state = state;
}
+
+ public GenericPresetVariable getNetworkOffering() {
+ return networkOffering;
+ }
+
+ public void setNetworkOffering(GenericPresetVariable networkOffering) {
+ this.networkOffering = networkOffering;
+ fieldNamesToIncludeInToString.add("networkOffering");
+ }
+
+ public GenericPresetVariable getVpcOffering() {
+ return vpcOffering;
+ }
+
+ public void setVpcOffering(GenericPresetVariable vpcOffering) {
+ this.vpcOffering = vpcOffering;
+ fieldNamesToIncludeInToString.add("vpcOffering");
Review Comment:
```suggestion
```
Same as above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
Posted by "winterhazel (via GitHub)" <gi...@apache.org>.
winterhazel merged PR #11810:
URL: https://github.com/apache/cloudstack/pull/11810
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
Posted by "winterhazel (via GitHub)" <gi...@apache.org>.
winterhazel commented on PR #11810:
URL: https://github.com/apache/cloudstack/pull/11810#issuecomment-3781185961
> @winterhazel , will you review?
@DaanHoogland yes, I should be able to review and test this one by the end of the week.
> @hsato03 , I feel a bit out of my depth on these preset variables and they only appear once in the documentation when it comes to storage; i.e. https://docs.cloudstack.apache.org/en/4.22.0.0/adminguide/storage.html#direct-resources-to-a-specific-secondary-storage. Is there some wiki page I can read to be able to concoct sensible tests for this?
The Quota documentation needs some updating on how to use the activation rules and preset variables. There is no wiki on how to use them at the moment. We hope to address it in version 23/24 along with the pending code changes.
Here's a basic guide on how to test the changes in this PR:
1. Install Usage and ensure it is running. Also, have a look at the configured execution time, which is defined through the global settings `usage.stats.job.exec.time` and `usage.execution.timezone`.
2. Enable Quota and JS interpretation. This can be done by toggling `quota.enable.service` and `js.interpretation.enabled` on and restarting the Management Server.
3. Create different network and VPC offerings to be used for the tests.
4. Create some tariffs (through `quotaTariffCreate`) using the preset variables in the activation rule.
For the network usage type tariff, you can use the following activation rule:
```js
if (value.networkOffering.id === '<uuid-of-network-offering-X>') {
200
} else {
100
}
```
This tariff will charge users 200 credits in a month for each network using network offering X, and 100 for all other offerings. As this rule already returns a value, the tariff value can be left as 0 (it will be ignored). Another option is returning a true/false instead in the rule to inform whether or not the tariff value should be applied. The other non-required fields may be left as blank to use default values.
```sh
(localcloud) 🐱 quota tariffcreate name="network tariff" usagetype=30 activationrule="if (value.networkOffering.id === '<uuid-of-network-offering-X>') { 200 } else { 100 }" value=0
```
For the VPC usage type tariff, the following activation rule can be used:
```js
if (value.vpcOffering.id === '<uuid-of-VPC-offering-Y>') {
300
} else {
200
}
```
This one will charge users 300 credits in a month for each VPC using the VPC offering Y, and 200 for all others.
```sh
(localcloud) 🐱 quota tariffcreate name="vpc tariff" usagetype=31 activationrule="if (value.vpcOffering.id === '<uuid-of-VPC-offering-Y>') { 300 } else { 200 }" value=0
```
5. Create and implement some networks and VPCs with the different offerings for this so that usage records are later generated.
6. Run the Usage job. I usually do this by manually setting the date of the VM running the Usage server to some minutes before the execution of the next day and restarting the service.
After the Usage job finishes generating all usage records, Quota will process them and charge credits from users based on the tariffs. You can follow Usage's logs during the job's execution to see which values are being charged for that execution, debug it to see which values are being injected as preset variables (I usually place my breakpoint in org.apache.cloudstack.quota.QuotaManagerImpl#injectPresetVariablesIntoJsInterpreter), and see the new balances in the Quota summary page.
The idea for debugging other PRs that add preset variables is the same.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #11810:
URL: https://github.com/apache/cloudstack/pull/11810#issuecomment-4241960228
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #11810:
URL: https://github.com/apache/cloudstack/pull/11810#issuecomment-4247816696
<b>[SF] Trillian test result (tid-15867)</b>
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 54049 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr11810-t15867-kvm-ol8.zip
Smoke tests completed. 149 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
ContextSuite context=TestResetVmOnReboot>:setup | `Error` | 0.00 | test_reset_vm_on_reboot.py
ContextSuite context=TestRAMCPUResourceAccounting>:setup | `Error` | 0.00 | test_resource_accounting.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #11810:
URL: https://github.com/apache/cloudstack/pull/11810#issuecomment-3383492206
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15349
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #11810:
URL: https://github.com/apache/cloudstack/pull/11810#issuecomment-4156382216
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17293
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #11810:
URL: https://github.com/apache/cloudstack/pull/11810#issuecomment-3777035297
@hsato03 , I feel a bit out of my depth on these preset variables and they only appear once in the documentation when it comes to storage; i.e. https://docs.cloudstack.apache.org/en/4.22.0.0/adminguide/storage.html#direct-resources-to-a-specific-secondary-storage. Is there some wiki page I can read to be able to concoct sensible tests for this?
@winterhazel , will you review?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #11810:
URL: https://github.com/apache/cloudstack/pull/11810#issuecomment-3769664646
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16429
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #11810:
URL: https://github.com/apache/cloudstack/pull/11810#issuecomment-4156057837
@hsato03 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
Posted by "hsato03 (via GitHub)" <gi...@apache.org>.
hsato03 commented on PR #11810:
URL: https://github.com/apache/cloudstack/pull/11810#issuecomment-3383334315
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #11810:
URL: https://github.com/apache/cloudstack/pull/11810#discussion_r2708484020
##########
framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelper.java:
##########
@@ -778,6 +783,19 @@ protected void loadPresetVariableValueForNetwork(UsageVO usageRecord, Value valu
value.setId(network.getUuid());
value.setName(network.getName());
value.setState(usageRecord.getState());
+
+ value.setNetworkOffering(getPresetVariableValueNetworkOffering(network.getNetworkOfferingId()));
Review Comment:
The new networkOffering field is being set in loadPresetVariableValueForNetwork, but there are no tests verifying this integration. Similar methods like loadPresetVariableValueForVolume and loadPresetVariableValueForBackup have comprehensive tests that verify all fields are properly set. Consider adding a test similar to loadPresetVariableValueForBackupTestRecordIsBackupSetAllFields that verifies the networkOffering field is correctly populated.
##########
framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelper.java:
##########
@@ -793,6 +811,18 @@ protected void loadPresetVariableValueForVpc(UsageVO usageRecord, Value value) {
value.setId(vpc.getUuid());
value.setName(vpc.getName());
+ value.setVpcOffering(getPresetVariableValueVpcOffering(vpc.getVpcOfferingId()));
Review Comment:
The new vpcOffering field is being set in loadPresetVariableValueForVpc, but there are no tests verifying this integration. Similar methods like loadPresetVariableValueForVolume and loadPresetVariableValueForBackup have comprehensive tests that verify all fields are properly set. Consider adding a test similar to loadPresetVariableValueForBackupTestRecordIsBackupSetAllFields that verifies the vpcOffering field is correctly populated.
##########
framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelper.java:
##########
@@ -793,6 +811,18 @@ protected void loadPresetVariableValueForVpc(UsageVO usageRecord, Value value) {
value.setId(vpc.getUuid());
value.setName(vpc.getName());
+ value.setVpcOffering(getPresetVariableValueVpcOffering(vpc.getVpcOfferingId()));
+ }
+
+ protected GenericPresetVariable getPresetVariableValueVpcOffering(long vpcOfferingId) {
Review Comment:
The parameter type should be `Long` instead of `long` to maintain consistency with similar methods in this class. Other methods like `getPresetVariableValueNetworkOffering`, `getPresetVariableZone`, and `getPresetVariableValueTemplate` all use `Long` as the parameter type.
```suggestion
protected GenericPresetVariable getPresetVariableValueVpcOffering(Long vpcOfferingId) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
Posted by "winterhazel (via GitHub)" <gi...@apache.org>.
winterhazel commented on PR #11810:
URL: https://github.com/apache/cloudstack/pull/11810#issuecomment-3797262807
While testing this, I found that preset variables are not being injected correctly into the JS interpreter anymore, which prevents this change from working as expected. #12515 needs to be merged first.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #11810:
URL: https://github.com/apache/cloudstack/pull/11810#issuecomment-4241966125
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
Posted by "winterhazel (via GitHub)" <gi...@apache.org>.
winterhazel commented on PR #11810:
URL: https://github.com/apache/cloudstack/pull/11810#issuecomment-4238788537
@DaanHoogland can we run the CI here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #11810:
URL: https://github.com/apache/cloudstack/pull/11810#issuecomment-3769507696
@DaanHoogland 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #11810:
URL: https://github.com/apache/cloudstack/pull/11810#issuecomment-3772787288
<b>[SF] Trillian test result (tid-15218)</b>
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 57521 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr11810-t15218-kvm-ol8.zip
Smoke tests completed. 149 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_03_deploy_and_scale_kubernetes_cluster | `Failure` | 28.78 | test_kubernetes_clusters.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #11810:
URL: https://github.com/apache/cloudstack/pull/11810#issuecomment-3769979183
@blueorangutan test keepEnv
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add offering preset variables for `Network` and `VPC` Quota tariffs [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11810:
URL: https://github.com/apache/cloudstack/pull/11810#issuecomment-3768255663
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org