You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "shwstppr (via GitHub)" <gi...@apache.org> on 2023/12/15 11:49:50 UTC
[PR] api,server,ui: tagged resource limits [cloudstack]
shwstppr opened a new pull request, #8362:
URL: https://github.com/apache/cloudstack/pull/8362
### Description
Introduces the concept of tagged resource limits. Limits can be enforced on accounts and domains for the deployment of entities for a tagged resource. Current tagged resource limits can be used for the following resource types,
Host limits
- user_vm
- cpu
- memory
Storage limits
- volume
- primary_storage
Following global settings can used to specify tags for which limit needs to be enforced,
Host: `resource.limit.host.tags`
Storage: `resource.limit.storage.tags`
Option for specifying tagged resource limits and viewing tagged resource usage are made available in the UI.
Enhances the use of templatetag for VM deployment and template creation
Adds option to list disk offering with suitability flag for a virtual machine. A new parameter named `virtualmachineid` has been added to the listDiskOfferings API which when passed returns `suitableforvirtualmachine` param in the response.
**ToDo: Schema changes to be moved in 4.20 upgrade path**
### 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
### 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?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
#### 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] api,server,ui: granular resource limit management [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1951962966
Merging based on approvals, manual QA and smoke test results
--
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] api,server,ui: tagged resource limits [cloudstack]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1857770303
## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/8362?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: `111 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`de095ba`)](https://app.codecov.io/gh/apache/cloudstack/commit/de095ba70d2a2261ffb2db7f3b8f29544939028e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 30.43% compared to head [(`fcaa40a`)](https://app.codecov.io/gh/apache/cloudstack/pull/8362?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 4.39%.
> Report is 1 commits behind head on main.
| [Files](https://app.codecov.io/gh/apache/cloudstack/pull/8362?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [ui/src/components/view/ResourceLimitTab.vue](https://app.codecov.io/gh/apache/cloudstack/pull/8362?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-dWkvc3JjL2NvbXBvbmVudHMvdmlldy9SZXNvdXJjZUxpbWl0VGFiLnZ1ZQ==) | 0.00% | [41 Missing :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/8362?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [ui/src/components/view/ResourceCountUsage.vue](https://app.codecov.io/gh/apache/cloudstack/pull/8362?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-dWkvc3JjL2NvbXBvbmVudHMvdmlldy9SZXNvdXJjZUNvdW50VXNhZ2UudnVl) | 0.00% | [35 Missing :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/8362?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [ui/src/views/compute/DeployVM.vue](https://app.codecov.io/gh/apache/cloudstack/pull/8362?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-dWkvc3JjL3ZpZXdzL2NvbXB1dGUvRGVwbG95Vk0udnVl) | 0.00% | [13 Missing :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/8362?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [ui/src/utils/plugins.js](https://app.codecov.io/gh/apache/cloudstack/pull/8362?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-dWkvc3JjL3V0aWxzL3BsdWdpbnMuanM=) | 31.25% | [11 Missing :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/8362?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [ui/src/views/storage/CreateVolume.vue](https://app.codecov.io/gh/apache/cloudstack/pull/8362?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-dWkvc3JjL3ZpZXdzL3N0b3JhZ2UvQ3JlYXRlVm9sdW1lLnZ1ZQ==) | 0.00% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/8362?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [ui/src/components/view/TreeView.vue](https://app.codecov.io/gh/apache/cloudstack/pull/8362?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-dWkvc3JjL2NvbXBvbmVudHMvdmlldy9UcmVlVmlldy52dWU=) | 0.00% | [4 Missing :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/8362?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [ui/src/config/section/image.js](https://app.codecov.io/gh/apache/cloudstack/pull/8362?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-dWkvc3JjL2NvbmZpZy9zZWN0aW9uL2ltYWdlLmpz) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/8362?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #8362 +/- ##
============================================
- Coverage 30.43% 4.39% -26.04%
============================================
Files 5341 361 -4980
Lines 374775 28648 -346127
Branches 54515 5014 -49501
============================================
- Hits 114060 1259 -112801
+ Misses 245575 27251 -218324
+ Partials 15140 138 -15002
```
| [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/8362/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [simulator-marvin-tests](https://app.codecov.io/gh/apache/cloudstack/pull/8362/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/8362/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `4.39% <4.31%> (-0.02%)` | :arrow_down: |
| [unit-tests](https://app.codecov.io/gh/apache/cloudstack/pull/8362/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/8362?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).
--
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] api,server,ui: tagged resource limits [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1859705369
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8075
--
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] api,server,ui: tagged resource limits [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1863314278
@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] api,server,ui: granular resource limit management [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on code in PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#discussion_r1489245402
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -1732,8 +1736,8 @@ public Volume destroyVolume(long volumeId, Account caller, boolean expunge, bool
logger.debug("Failed to destroy volume" + volume.getId(), e);
return null;
}
- _resourceLimitMgr.decrementResourceCount(volume.getAccountId(), ResourceType.volume, volume.isDisplay());
- _resourceLimitMgr.decrementResourceCount(volume.getAccountId(), ResourceType.primary_storage, volume.isDisplay(), new Long(volume.getSize()));
+ _resourceLimitMgr.decrementVolumeResourceCount(volume.getAccountId(), volume.isDisplay(),
+ volume.getSize(), _diskOfferingDao.findById(volume.getDiskOfferingId()));
Review Comment:
We should use `findByIdIncludingRemoved` 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] api,server,ui: granular resource limit management [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1945373800
@shwstppr a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) 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] api,server,ui: tagged resource limits [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1882871037
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
Re: [PR] api,server,ui: tagged resource limits [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1873468516
<b>[SF] Trillian test result (tid-8702)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 48924 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8362-t8702-kvm-centos7.zip
Smoke tests completed. 120 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_07_deploy_kubernetes_ha_cluster | `Failure` | 3623.14 | test_kubernetes_clusters.py
test_08_upgrade_kubernetes_ha_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
test_10_vpc_tier_kubernetes_cluster | `Failure` | 46.93 | test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 99.38 | 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] api,server,ui: tagged resource limits [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1914005955
@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] api,server,ui: granular resource limit management [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1938298769
<b>[SF] Trillian test result (tid-9164)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 1530 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8362-t9164-kvm-centos7.zip
Smoke tests completed. 7 look OK, 122 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
runTest | `Error` | 0.00 | test_2fa.py
runTest | `Error` | 0.00 | test_diagnostics.py
runTest | `Error` | 0.00 | test_accounts.py
runTest | `Error` | 0.00 | test_deploy_vm_extra_config_data.py
runTest | `Error` | 0.00 | test_affinity_groups_projects.py
runTest | `Error` | 0.00 | test_deploy_vms_with_varied_deploymentplanners.py
runTest | `Error` | 0.00 | test_affinity_groups.py
runTest | `Error` | 0.00 | test_annotations.py
runTest | `Error` | 0.00 | test_domain_disk_offerings.py
runTest | `Error` | 0.00 | test_async_job.py
runTest | `Error` | 0.00 | test_attach_multiple_volumes.py
runTest | `Error` | 0.00 | test_direct_download.py
runTest | `Error` | 0.00 | test_backup_recovery_dummy.py
runTest | `Error` | 0.00 | test_events_resource.py
runTest | `Error` | 0.00 | test_backup_recovery_veeam.py
runTest | `Error` | 0.00 | test_deploy_vm_iso.py
runTest | `Error` | 0.00 | test_certauthority_root.py
runTest | `Error` | 0.00 | test_disk_provisioning_types.py
runTest | `Error` | 0.00 | test_cluster_drs.py
runTest | `Error` | 0.00 | test_global_acls.py
runTest | `Error` | 0.00 | test_console_endpoint.py
runTest | `Error` | 0.00 | test_deploy_vm_with_userdata.py
runTest | `Error` | 0.00 | test_create_list_domain_account_project.py
runTest | `Error` | 0.00 | test_global_settings.py
runTest | `Error` | 0.00 | test_create_network.py
runTest | `Error` | 0.00 | test_deploy_vgpu_enabled_vm.py
runTest | `Error` | 0.00 | test_disk_offerings.py
runTest | `Error` | 0.00 | test_guest_os.py
runTest | `Error` | 0.00 | test_deploy_virtio_scsi_vm.py
runTest | `Error` | 0.00 | test_deploy_vm_iso_uefi.py
runTest | `Error` | 0.00 | test_guest_vlan_range.py
runTest | `Error` | 0.00 | test_deploy_vm_root_resize.py
runTest | `Error` | 0.00 | test_deploy_vms_in_parallel.py
runTest | `Error` | 0.00 | test_domain_network_offerings.py
runTest | `Error` | 0.00 | test_domain_service_offerings.py
runTest | `Error` | 0.00 | test_domain_vpc_offerings.py
runTest | `Error` | 0.00 | test_enable_account_settings_for_domain.py
runTest | `Error` | 0.00 | test_gateway_on_shared_networks.py
runTest | `Error` | 0.00 | test_host_control_state.py
runTest | `Error` | 0.00 | test_hostha_simulator.py
runTest | `Error` | 0.00 | test_host_ping.py
runTest | `Error` | 0.00 | test_image_store_object_migration.py
runTest | `Error` | 0.00 | test_internal_lb.py
runTest | `Error` | 0.00 | test_ipv6_infra.py
runTest | `Error` | 0.00 | test_iso.py
runTest | `Error` | 0.00 | test_kubernetes_clusters.py
runTest | `Error` | 0.00 | test_kubernetes_supported_versions.py
runTest | `Error` | 0.00 | test_list_accounts.py
runTest | `Error` | 0.00 | test_list_disk_offerings.py
runTest | `Error` | 0.00 | test_over_provisioning.py
runTest | `Error` | 0.00 | test_list_domains.py
runTest | `Error` | 0.00 | test_list_hosts.py
runTest | `Error` | 0.00 | test_nonstrict_affinity_group.py
runTest | `Error` | 0.00 | test_list_ids_parameter.py
runTest | `Error` | 0.00 | test_list_service_offerings.py
runTest | `Error` | 0.00 | test_list_storage_pools.py
runTest | `Error` | 0.00 | test_list_volumes.py
runTest | `Error` | 0.00 | test_password_server.py
runTest | `Error` | 0.00 | test_loadbalance.py
runTest | `Error` | 0.00 | test_login.py
runTest | `Error` | 0.00 | test_metrics_api.py
runTest | `Error` | 0.00 | test_migration.py
runTest | `Error` | 0.00 | test_multipleips_per_nic.py
runTest | `Error` | 0.00 | test_nested_virtualization.py
runTest | `Error` | 0.00 | test_network_acl.py
runTest | `Error` | 0.00 | test_portable_publicip.py
runTest | `Error` | 0.00 | test_network_ipv6.py
runTest | `Error` | 0.00 | test_outofbandmanagement.py
runTest | `Error` | 0.00 | test_network_permissions.py
runTest | `Error` | 0.00 | test_network.py
runTest | `Error` | 0.00 | test_nic_adapter_type.py
runTest | `Error` | 0.00 | test_nic.py
runTest | `Error` | 0.00 | test_non_contigiousvlan.py
runTest | `Error` | 0.00 | test_outofbandmanagement_nestedplugin.py
runTest | `Error` | 0.00 | test_persistent_network.py
runTest | `Error` | 0.00 | test_portforwardingrules.py
runTest | `Error` | 0.00 | test_primary_storage.py
runTest | `Error` | 0.00 | test_privategw_acl_ovs_gre.py
runTest | `Error` | 0.00 | test_privategw_acl.py
runTest | `Error` | 0.00 | test_projects.py
runTest | `Error` | 0.00 | test_public_ip_range.py
runTest | `Error` | 0.00 | test_pvlan.py
runTest | `Error` | 0.00 | test_quarantined_ips.py
runTest | `Error` | 0.00 | test_regions.py
runTest | `Error` | 0.00 | test_register_userdata.py
runTest | `Error` | 0.00 | test_volumes.py
runTest | `Error` | 0.00 | test_reset_configuration_settings.py
runTest | `Error` | 0.00 | test_reset_vm_on_reboot.py
runTest | `Error` | 0.00 | test_vm_life_cycle.py
runTest | `Error` | 0.00 | test_resource_accounting.py
runTest | `Error` | 0.00 | test_resource_detail.py
runTest | `Error` | 0.00 | test_vm_schedule.py
runTest | `Error` | 0.00 | test_router_dhcphosts.py
runTest | `Error` | 0.00 | test_router_dns.py
runTest | `Error` | 0.00 | test_router_dnsservice.py
runTest | `Error` | 0.00 | test_vm_deployment_planner.py
runTest | `Error` | 0.00 | test_routers_iptables_default_policy.py
runTest | `Error` | 0.00 | test_routers_network_ops.py
runTest | `Error` | 0.00 | test_routers.py
runTest | `Error` | 0.00 | test_vm_snapshot_kvm.py
runTest | `Error` | 0.00 | test_safe_shutdown.py
runTest | `Error` | 0.00 | test_scale_vm.py
runTest | `Error` | 0.00 | test_secondary_storage.py
runTest | `Error` | 0.00 | test_service_offerings.py
runTest | `Error` | 0.00 | test_vm_snapshots.py
runTest | `Error` | 0.00 | test_set_sourcenat.py
runTest | `Error` | 0.00 | test_vpc_ipv6.py
runTest | `Error` | 0.00 | test_snapshots.py
runTest | `Error` | 0.00 | test_ssvm.py
runTest | `Error` | 0.00 | test_storage_policy.py
runTest | `Error` | 0.00 | test_templates.py
runTest | `Error` | 0.00 | test_update_security_group.py
runTest | `Error` | 0.00 | test_usage_events.py
runTest | `Error` | 0.00 | test_usage.py
runTest | `Error` | 0.00 | test_vnf_templates.py
runTest | `Error` | 0.00 | test_vm_autoscaling.py
runTest | `Error` | 0.00 | test_vm_lifecycle_unmanage_import.py
runTest | `Error` | 0.00 | test_vpc_redundant.py
runTest | `Error` | 0.00 | test_vpc_router_nics.py
runTest | `Error` | 0.00 | test_vpc_vpn.py
runTest | `Error` | 0.00 | test_host_maintenance.py
runTest | `Error` | 0.00 | test_hostha_kvm.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] api,server,ui: tagged resource limits [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1926450274
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
Re: [PR] api,server,ui: granular resource limit management [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on code in PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#discussion_r1490013340
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -8487,10 +8536,15 @@ public Boolean getDestroyRootVolumeOnVmDestruction(Long domainId){
return DestroyRootVolumeOnVmDestruction.valueIn(domainId);
}
- private void setVncPasswordForKvmIfAvailable(Map<String, String> customParameters, UserVmVO vm){
+ private void setVncPasswordForKvmIfAvailable(Map<String, String> customParameters, UserVmVO vm) {
if (customParameters.containsKey(VmDetailConstants.KVM_VNC_PASSWORD)
&& StringUtils.isNotEmpty(customParameters.get(VmDetailConstants.KVM_VNC_PASSWORD))) {
vm.setVncPassword(customParameters.get(VmDetailConstants.KVM_VNC_PASSWORD));
}
}
+
+ @Override
+ public Map<Long, Boolean> getDiskOfferingSuitabilityForVm(long vmId, List<Long> diskOfferingIds) {
+ return _itMgr.getDiskOfferingSuitabilityForVm(vmId, diskOfferingIds);
Review Comment:
StoragePoolAllocators were not accessible in `UserVmManagerImpl` so it calls VitualMachineManager. I'll check using VirtualMachineManager itself in ApiDBUtils
--
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] api,server,ui: tagged resource limits [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1863315731
@shwstppr 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] api,server,ui: tagged resource limits [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1884398452
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
Re: [PR] api,server,ui: granular resource limit management [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1940367524
@shwstppr a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) 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] api,server,ui: granular resource limit management [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on code in PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#discussion_r1488958984
##########
api/src/main/java/org/apache/cloudstack/api/command/user/resource/UpdateResourceCountCmd.java:
##########
@@ -117,15 +124,14 @@ public long getEntityOwnerId() {
@Override
public void execute() {
List<? extends ResourceCount> result =
- _resourceLimitService.recalculateResourceCount(_accountService.finalyzeAccountId(accountName, domainId, projectId, true), getDomainId(), getResourceType());
+ _resourceLimitService.recalculateResourceCount(_accountService.finalyzeAccountId(accountName, domainId, projectId, true), getDomainId(), getResourceType(), getTag());
if ((result != null) && (result.size() > 0)) {
ListResponse<ResourceCountResponse> response = new ListResponse<ResourceCountResponse>();
List<ResourceCountResponse> countResponses = new ArrayList<ResourceCountResponse>();
for (ResourceCount count : result) {
ResourceCountResponse resourceCountResponse = _responseGenerator.createResourceCountResponse(count);
- resourceCountResponse.setObjectName("resourcecount");
Review Comment:
Would this change the json key in the output?
--
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] api,server,ui: granular resource limit management [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on code in PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#discussion_r1489142003
##########
engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDaoImpl.java:
##########
@@ -157,4 +157,23 @@ public boolean remove(Long id) {
return update(id, diskOffering);
}
+
+ @Override
+ public List<DiskOfferingVO> listByStorageTag(String tag) {
+ SearchBuilder<DiskOfferingVO> sb = createSearchBuilder();
+ sb.and("s", sb.entity().getTags(), SearchCriteria.Op.NNULL);
+ sb.and("tagNotNull", sb.entity().getTags(), SearchCriteria.Op.NNULL);
Review Comment:
```suggestion
sb.and("tagNotNull", sb.entity().getTags(), SearchCriteria.Op.NNULL);
```
--
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] api,server,ui: granular resource limit management [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1948390617
@JoaoJandre @GutoVeronezi , this has been reviewed by several and has gone through our QA procedure (manual and automated testing. Do you guys hace any concerns with merging this?
--
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] api,server,ui: tagged resource limits [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1882890151
@shwstppr 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] api,server,ui: tagged resource limits [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1903548990
@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] api,server,ui: tagged resource limits [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1923039715
Test results from backend,
```
[SF] Trillian test result (tid-9040)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 43368 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8362-t9040-kvm-centos7.zip
Smoke tests completed. 120 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_02_trigger_shutdown | `Failure` | 341.70 | test_safe_shutdown.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] api,server,ui: tagged resource limits [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1870124634
@shwstppr 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] api,server,ui: tagged resource limits [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1873181955
@shwstppr a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) 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] api,server,ui: granular resource limit management [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1945717349
@shwstppr is this ready/can it go out of draft ?
--
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] api,server,ui: granular resource limit management [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on code in PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#discussion_r1489083226
##########
api/src/test/java/org/apache/cloudstack/api/command/admin/resource/ListCapacityCmdTest.java:
##########
@@ -0,0 +1,34 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.resource;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.springframework.test.util.ReflectionTestUtils;
+
+public class ListCapacityCmdTest {
Review Comment:
@vishesh92 if this helps then will add it to this and other added test files. I didn't know. Thanks.
--
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] api,server,ui: granular resource limit management [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1944540128
@shwstppr 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] api,server,ui: granular resource limit management [cloudstack]
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1938159665
@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] api,server,ui: tagged resource limits [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1859637989
@shwstppr 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] api,server,ui: tagged resource limits [cloudstack]
Posted by "gogast (via GitHub)" <gi...@apache.org>.
gogast commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1924362015
Now there is one global parameter for maximum volume size. With this new feature will it be possible to specify different maximum volume sizes for different storage pools?
--
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] api,server,ui: tagged resource limits [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1914007770
@shwstppr 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] api,server,ui: granular resource limit management [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on code in PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#discussion_r1488998059
##########
api/src/test/java/org/apache/cloudstack/api/command/admin/resource/ListCapacityCmdTest.java:
##########
@@ -0,0 +1,34 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.resource;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.springframework.test.util.ReflectionTestUtils;
+
+public class ListCapacityCmdTest {
Review Comment:
@DaanHoogland do we need to add this to make this test considered for coverage?
```java
@RunWith(MockitoJUnitRunner.class)
```
--
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] api,server,ui: granular resource limit management [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1946850401
<b>[SF] Trillian test result (tid-9213)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 46491 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8362-t9213-kvm-centos7.zip
Smoke tests completed. 129 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
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] api,server,ui: granular resource limit management [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on code in PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#discussion_r1489240381
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -978,8 +981,8 @@ public VolumeVO doInTransaction(TransactionStatus status) {
CallContext.current().putContextParameter(Volume.class, volume.getId());
// Increment resource count during allocation; if actual creation fails,
// decrement it
- _resourceLimitMgr.incrementResourceCount(volume.getAccountId(), ResourceType.volume, displayVolume);
- _resourceLimitMgr.incrementResourceCount(volume.getAccountId(), ResourceType.primary_storage, displayVolume, new Long(volume.getSize()));
+ _resourceLimitMgr.incrementVolumeResourceCount(volume.getAccountId(), displayVolume, volume.getSize(),
+ _diskOfferingDao.findById(volume.getDiskOfferingId()));
Review Comment:
Should we use `findByIdIncludingRemoved` instead?
--
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] api,server,ui: granular resource limit management [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1945500200
@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] api,server,ui: granular resource limit management [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on code in PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#discussion_r1489995602
##########
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##########
@@ -2414,25 +2415,24 @@ public CapacityVO getSecondaryStorageUsedStats(Long hostId, Long zoneId) {
return capacity;
}
- @Override
- public CapacityVO getStoragePoolUsedStats(Long poolId, Long clusterId, Long podId, Long zoneId) {
+ private CapacityVO getStoragePoolUsedStatsInternal(Long zoneId, Long podId, Long clusterId, List<Long> poolIds, Long poolId) {
SearchCriteria<StoragePoolVO> sc = _storagePoolDao.createSearchCriteria();
- List<StoragePoolVO> pools = new ArrayList<StoragePoolVO>();
+ List<StoragePoolVO> pools = new ArrayList<>();
if (zoneId != null) {
sc.addAnd("dataCenterId", SearchCriteria.Op.EQ, zoneId);
}
-
if (podId != null) {
sc.addAnd("podId", SearchCriteria.Op.EQ, podId);
}
-
if (clusterId != null) {
sc.addAnd("clusterId", SearchCriteria.Op.EQ, clusterId);
}
-
+ if (CollectionUtils.isNotEmpty(poolIds)) {
+ sc.addAnd("id", SearchCriteria.Op.IN, poolIds.toArray());
+ }
if (poolId != null) {
- sc.addAnd("hostOrPoolId", SearchCriteria.Op.EQ, poolId);
+ sc.addAnd("id", SearchCriteria.Op.EQ, poolId);
Review Comment:
Not directly but this was causing an issue with listCapacity API. SearchCriteria here is directly created from dao and there is no `hostOrPoolId` member in `StoragePoolVO` class
--
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] api,server,ui: granular resource limit management [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1938228906
@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] api,server,ui: tagged resource limits [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1927096763
<b>[SF] Trillian test result (tid-9076)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8
Total time taken: 54968 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8362-t9076-vmware-67u3.zip
Smoke tests completed. 119 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_02_balanced_drs_algorithm | `Error` | 430.45 | test_cluster_drs.py
test_08_upgrade_kubernetes_ha_cluster | `Failure` | 1037.44 | 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] api,server,ui: granular resource limit management [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1941443571
> > @JoaoJandre Description in the PR specifically mentions limits for tagged resources. I'll review and make further changes to make it clearer. Regarding listDiskOfferings change I would prefer it in the same PR as it has already been tested internally. We will add relevant test reports. Also, diskoffering suitability for a VM is quite related to offering and storage pool tags. I'll update the title and description for the PR if needed.
>
> @shwstppr, does the change on the list disk offering depends on the other change you are proposing? Because if one does not affect or depends on the other, they are different contexts, even if related to the same set of functionalities (storage's and host's tags, in this case).
>
> Mixing contexts in a PR is not encouraged. It makes it difficult for others to review the changes and understand what is being done, especially in these cases with thousands of changes. Furthermore, unrelated changes being tested internally should not be used as reason to open a PR with all of them. While contributing to a community, we should structure our changes and proposals in a way that others can easily understand what is being done/proposed; thus, communicating without noise.
@GutoVeronezi (cc @JoaoJandre), I agree with the principal of submitting small isolated chunks of functionality as much as possible. In this case, it is work done for a customer now given back to the upstream source tree. It is indeed slightly different pieces of code and could be separated out, but this is not work we (as in @shwstppr) will do now. It has been tested and any comments on either of the pieces that will lead to further changes, will then have to be tested on the integrated version as well. the QA work is what is leading in this case. I don't like reviewing 7000 lines of code in one go either, sorry.
--
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] api,server,ui: tagged resource limits [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1859637758
@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] api,server,ui: tagged resource limits [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1873181595
@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] api,server,ui: tagged resource limits [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1882888655
@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] api,server,ui: granular resource limit management [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1941989230
<b>[SF] Trillian test result (tid-9184)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 46393 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8362-t9184-kvm-centos7.zip
Smoke tests completed. 129 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
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] api,server,ui: granular resource limit management [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on code in PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#discussion_r1489242168
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -978,8 +981,8 @@ public VolumeVO doInTransaction(TransactionStatus status) {
CallContext.current().putContextParameter(Volume.class, volume.getId());
// Increment resource count during allocation; if actual creation fails,
// decrement it
- _resourceLimitMgr.incrementResourceCount(volume.getAccountId(), ResourceType.volume, displayVolume);
- _resourceLimitMgr.incrementResourceCount(volume.getAccountId(), ResourceType.primary_storage, displayVolume, new Long(volume.getSize()));
+ _resourceLimitMgr.incrementVolumeResourceCount(volume.getAccountId(), displayVolume, volume.getSize(),
+ _diskOfferingDao.findById(volume.getDiskOfferingId()));
Review Comment:
nvm. It's during volume creation. So, it should be fine.
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -1038,8 +1041,8 @@ public VolumeVO createVolume(CreateVolumeCmd cmd) {
} finally {
if (!created) {
logger.trace("Decrementing volume resource count for account id=" + volume.getAccountId() + " as volume failed to create on the backend");
- _resourceLimitMgr.decrementResourceCount(volume.getAccountId(), ResourceType.volume, cmd.getDisplayVolume());
- _resourceLimitMgr.decrementResourceCount(volume.getAccountId(), ResourceType.primary_storage, cmd.getDisplayVolume(), new Long(volume.getSize()));
+ _resourceLimitMgr.decrementVolumeResourceCount(volume.getAccountId(), cmd.getDisplayVolume(),
+ volume.getSize(), _diskOfferingDao.findById(volume.getDiskOfferingId()));
Review Comment:
Should we use `findByIdIncludingRemoved` instead?
--
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] api,server,ui: granular resource limit management [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on code in PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#discussion_r1489212377
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -2595,11 +2591,12 @@ private void updateVmStateForFailedVmCreation(Long vmId, Long hostId) {
String msg = "Failed to deploy Vm with Id: " + vmId + ", on Host with Id: " + hostId;
_alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_USERVM, vm.getDataCenterId(), vm.getPodIdToDeployIn(), msg, msg);
- // Get serviceOffering for Virtual Machine
+ // Get serviceOffering and template for Virtual Machine
ServiceOfferingVO offering = serviceOfferingDao.findById(vm.getId(), vm.getServiceOfferingId());
+ VMTemplateVO tempalte = _templateDao.findByIdIncludingRemoved(vm.getTemplateId());
Review Comment:
tempalte -> template
rename this please
##########
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##########
@@ -2414,25 +2415,24 @@ public CapacityVO getSecondaryStorageUsedStats(Long hostId, Long zoneId) {
return capacity;
}
- @Override
- public CapacityVO getStoragePoolUsedStats(Long poolId, Long clusterId, Long podId, Long zoneId) {
+ private CapacityVO getStoragePoolUsedStatsInternal(Long zoneId, Long podId, Long clusterId, List<Long> poolIds, Long poolId) {
SearchCriteria<StoragePoolVO> sc = _storagePoolDao.createSearchCriteria();
- List<StoragePoolVO> pools = new ArrayList<StoragePoolVO>();
+ List<StoragePoolVO> pools = new ArrayList<>();
if (zoneId != null) {
sc.addAnd("dataCenterId", SearchCriteria.Op.EQ, zoneId);
}
-
if (podId != null) {
sc.addAnd("podId", SearchCriteria.Op.EQ, podId);
}
-
if (clusterId != null) {
sc.addAnd("clusterId", SearchCriteria.Op.EQ, clusterId);
}
-
+ if (CollectionUtils.isNotEmpty(poolIds)) {
+ sc.addAnd("id", SearchCriteria.Op.IN, poolIds.toArray());
+ }
if (poolId != null) {
- sc.addAnd("hostOrPoolId", SearchCriteria.Op.EQ, poolId);
+ sc.addAnd("id", SearchCriteria.Op.EQ, poolId);
Review Comment:
is this change related to this PR ?
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -8487,10 +8536,15 @@ public Boolean getDestroyRootVolumeOnVmDestruction(Long domainId){
return DestroyRootVolumeOnVmDestruction.valueIn(domainId);
}
- private void setVncPasswordForKvmIfAvailable(Map<String, String> customParameters, UserVmVO vm){
+ private void setVncPasswordForKvmIfAvailable(Map<String, String> customParameters, UserVmVO vm) {
if (customParameters.containsKey(VmDetailConstants.KVM_VNC_PASSWORD)
&& StringUtils.isNotEmpty(customParameters.get(VmDetailConstants.KVM_VNC_PASSWORD))) {
vm.setVncPassword(customParameters.get(VmDetailConstants.KVM_VNC_PASSWORD));
}
}
+
+ @Override
+ public Map<Long, Boolean> getDiskOfferingSuitabilityForVm(long vmId, List<Long> diskOfferingIds) {
+ return _itMgr.getDiskOfferingSuitabilityForVm(vmId, diskOfferingIds);
Review Comment:
why did we need this in both uservmmanager and virtualmachinemanager ? We can keep it in one place right!
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -2278,11 +2273,12 @@ public UserVm recoverVirtualMachine(RecoverVMCmd cmd) throws ResourceAllocationE
// Get serviceOffering for Virtual Machine
ServiceOfferingVO serviceOffering = serviceOfferingDao.findById(vm.getId(), vm.getServiceOfferingId());
+ VMTemplateVO tempalte = _templateDao.findByIdIncludingRemoved(vm.getTemplateId());
Review Comment:
tempalte -> template
rename this please
##########
server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java:
##########
@@ -1152,11 +1459,365 @@ public void changeResourceCount(long accountId, ResourceType type, Boolean displ
}
@Override
- public ResourceReservation getReservation(final Account account, final Boolean displayResource, final Resource.ResourceType type, final Long delta) throws ResourceAllocationException {
- if (! Boolean.FALSE.equals(displayResource)) {
- return new CheckedReservation(account, type, delta, reservationDao, resourceLimitService);
+ public List<String> getResourceLimitHostTags() {
+ if (StringUtils.isEmpty(ResourceLimitService.ResourceLimitHostTags.value())) {
+ return new ArrayList<>();
+ }
+ return Arrays.asList(ResourceLimitService.ResourceLimitHostTags.value().split(","));
Review Comment:
can we use trim() here something like
List<String> ResourceLimitHostTags = Stream.of(ResourceLimitService.ResourceLimitHostTags.value().split(","))
.map(item -> (item).trim())
.collect(Collectors.toList());
--
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] api,server,ui: tagged resource limits [cloudstack]
Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1924266633
@shwstppr from your description, it seems like you're proposing something that already exists, we already have limits for all resources you listed (and a few more) on an account/domain scope, I feel like I'm missing something here, could you maybe describe a little further what you're proposing?
Also, about this:
> Adds option to list disk offering with suitability flag for a virtual machine. A new parameter named virtualmachineid has been added to the listDiskOfferings API which when passed returns suitableforvirtualmachine param in the response.
To me this looks like an unrelated change, this PR is already big, so adding more context will make it a little too complex for reviewing. It would be better to separate different changes in different PRs so that we can properly review the code.
--
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] api,server,ui: tagged resource limits [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1925947598
@vladimirpetrov a [SL] Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) 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] api,server,ui: tagged resource limits [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#discussion_r1436843312
##########
api/src/test/java/org/apache/cloudstack/api/command/user/template/UpdateTemplateCmdTest.java:
##########
@@ -0,0 +1,33 @@
+package org.apache.cloudstack.api.command.user.template;
Review Comment:
```suggestion
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
package org.apache.cloudstack.api.command.user.template;
```
--
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] api,server,ui: tagged resource limits [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1870070103
@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] api,server,ui: tagged resource limits [cloudstack]
Posted by "GutoVeronezi (via GitHub)" <gi...@apache.org>.
GutoVeronezi commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1924491548
> @JoaoJandre Description in the PR specifically mentions limits for tagged resources. I'll review and make further changes to make it clearer. Regarding listDiskOfferings change I would prefer it in the same PR as it has already been tested internally. We will add relevant test reports. Also, diskoffering suitability for a VM is quite related to offering and storage pool tags. I'll update the title and description for the PR if needed.
@shwstppr, does the change on the list disk offering depends on the other change you are proposing? Because if one does not affect or depends on the other, they are different contexts, even if related to the same set of functionalities (storage's and host's tags, in this case).
Mixing contexts in a PR is not encouraged. It makes it difficult for others to review the changes and understand what is being done, especially in these cases with thousands of changes. Furthermore, unrelated changes being tested internally should not be used as reason to open a PR with all of them. While contributing to a community, we should structure our changes and proposals in a way that others can easily understand what is being done/proposed; thus, communicating without noise.
--
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] api,server,ui: tagged resource limits [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1924394430
> Now there is one global parameter for maximum volume size. With this new feature will it be possible to specify different maximum volume sizes for different storage pools?
@gogast no, this feature will not add that functionality. You may add an improvement request for this if needed.
However, with this feature, operators will be able to specify host and storage tags as resource limit tags. Once these tags are specified in the new global config, operators will be able to set limits (VM, cpu, memory, volumes and primary storage) for account and domains.
This can be useful when you have some specialized hardware on which you would want to enforce some extra limits.
Eg, For an account you've set volumes limit as 10. But now you want the account to deploy only 5 volumes on SSD storage. To facilitate this you can tag your SSD storage with tag - `SSD`. Create a corresponding disk offering with this tag. Specify this tags resource.limit.storage.tags. With UI and API now you can set limits for volume resource type with tag `SSD` as 5.
In short, it gives functionality to set certain resource limits on a granular level for accounts and domains.
--
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] api,server,ui: granular resource limit management [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on code in PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#discussion_r1489238493
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -707,7 +710,7 @@ public VolumeVO allocVolume(CreateVolumeCmd cmd) throws ResourceAllocationExcept
}
// Check that the resource limit for volumes won't be exceeded
- _resourceLimitMgr.checkResourceLimit(owner, ResourceType.volume, displayVolume);
+// _resourceLimitMgr.checkResourceLimit(owner, ResourceType.volume, displayVolume);
Review Comment:
uncomment or remove commented code.
--
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] api,server,ui: granular resource limit management [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on code in PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#discussion_r1489105719
##########
api/src/test/java/org/apache/cloudstack/api/command/admin/resource/ListCapacityCmdTest.java:
##########
@@ -0,0 +1,34 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.resource;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.springframework.test.util.ReflectionTestUtils;
+
+public class ListCapacityCmdTest {
Review Comment:
Looks like I confused it with something else. I can see coverage for getTags in codecov. No need to add RunWith
--
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] api,server,ui: granular resource limit management [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1945449749
@harikrishna-patnala @vishesh92 I've addressed your comments. Can you please check?
--
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] api,server,ui: granular resource limit management [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1938230614
@shwstppr a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) 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] api,server,ui: tagged resource limits [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1903675458
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8406
--
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] api,server,ui: granular resource limit management [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr merged PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362
--
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] api,server,ui: tagged resource limits [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on code in PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#discussion_r1436876322
##########
api/src/test/java/org/apache/cloudstack/api/command/user/template/UpdateTemplateCmdTest.java:
##########
@@ -0,0 +1,33 @@
+package org.apache.cloudstack.api.command.user.template;
Review Comment:
License added in commit [06ef971](https://github.com/apache/cloudstack/pull/8362/commits/06ef97158a0d4e544ba5131072e95404e8dee956)
--
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] api,server,ui: tagged resource limits [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1870177278
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8148
--
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] api,server,ui: granular resource limit management [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#discussion_r1489188356
##########
api/src/test/java/org/apache/cloudstack/api/command/admin/resource/ListCapacityCmdTest.java:
##########
@@ -0,0 +1,34 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.resource;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.springframework.test.util.ReflectionTestUtils;
+
+public class ListCapacityCmdTest {
Review Comment:
only if you don't go for the default runner as i always understood it (which means not much btw)
--
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] api,server,ui: granular resource limit management [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1945788799
> @shwstppr is this ready/can it go out of draft ?
Waiting for @vladimirpetrov to finish some manual QA
--
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] api,server,ui: granular resource limit management [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1938160606
@rohityadavcloud 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] api,server,ui: granular resource limit management [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1938225194
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8626
--
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] api,server,ui: granular resource limit management [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1944537655
@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] api,server,ui: granular resource limit management [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1940366223
@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] api,server,ui: granular resource limit management [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on code in PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#discussion_r1489082303
##########
api/src/main/java/org/apache/cloudstack/api/command/user/resource/UpdateResourceCountCmd.java:
##########
@@ -117,15 +124,14 @@ public long getEntityOwnerId() {
@Override
public void execute() {
List<? extends ResourceCount> result =
- _resourceLimitService.recalculateResourceCount(_accountService.finalyzeAccountId(accountName, domainId, projectId, true), getDomainId(), getResourceType());
+ _resourceLimitService.recalculateResourceCount(_accountService.finalyzeAccountId(accountName, domainId, projectId, true), getDomainId(), getResourceType(), getTag());
if ((result != null) && (result.size() > 0)) {
ListResponse<ResourceCountResponse> response = new ListResponse<ResourceCountResponse>();
List<ResourceCountResponse> countResponses = new ArrayList<ResourceCountResponse>();
for (ResourceCount count : result) {
ResourceCountResponse resourceCountResponse = _responseGenerator.createResourceCountResponse(count);
- resourceCountResponse.setObjectName("resourcecount");
Review Comment:
@vishesh92 removed this as ApiResponseHelper already sets the same thing in `createResourceCountResponse`
--
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] api,server,ui: tagged resource limits [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1920966272
@shwstppr 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] api,server,ui: tagged resource limits [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1921189812
@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] api,server,ui: tagged resource limits [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1924373348
@JoaoJandre Description in the PR specifically mentions limits for tagged resources. I'll review and make further changes to make it clearer.
Regarding listDiskOfferings change I would prefer it in the same PR as it has already been tested internally. We will add relevant test reports. Also, diskoffering suitability for a VM is quite related to offering and storage pool tags. I'll update the title and description for the PR if needed.
--
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] api,server,ui: tagged resource limits [cloudstack]
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1903541161
@shwstppr can you check/address conflict?
--
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] api,server,ui: granular resource limit management [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1944667315
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8654
--
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] api,server,ui: granular resource limit management [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on code in PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#discussion_r1489156863
##########
plugins/hypervisors/simulator/src/main/java/com/cloud/agent/manager/SimulatorManagerImpl.java:
##########
@@ -439,7 +440,9 @@ public Answer simulate(final Command cmd, final String hostGuid) {
} else if (cmd instanceof FenceCommand) {
answer = _mockVmMgr.fence((FenceCommand)cmd);
} else if (cmd instanceof HandleConfigDriveIsoCommand) {
- answer = _mockStorageMgr.handleConfigDriveIso((HandleConfigDriveIsoCommand)cmd);
+ answer = _mockStorageMgr.handleConfigDriveIso((HandleConfigDriveIsoCommand)cmd);;
Review Comment:
```suggestion
answer = _mockStorageMgr.handleConfigDriveIso((HandleConfigDriveIsoCommand)cmd);
```
--
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] api,server,ui: tagged resource limits [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1863399357
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8092
--
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] api,server,ui: tagged resource limits [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1870114725
@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] api,server,ui: tagged resource limits [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1903552227
@shwstppr 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] api,server,ui: tagged resource limits [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1914068784
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8458
--
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] api,server,ui: tagged resource limits [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1921190973
@shwstppr a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) 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] api,server,ui: tagged resource limits [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1926636830
<b>[SF] Trillian test result (tid-9075)</b>
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 40792 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8362-t9075-xenserver-71.zip
Smoke tests completed. 121 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
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] api,server,ui: tagged resource limits [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1934110467
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
Re: [PR] api,server,ui: tagged resource limits [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1920965262
@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] api,server,ui: tagged resource limits [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1882979161
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8249
--
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] api,server,ui: tagged resource limits [cloudstack]
Posted by "vladimirpetrov (via GitHub)" <gi...@apache.org>.
vladimirpetrov commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1925946021
@blueorangutan test matrix
--
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] api,server,ui: tagged resource limits [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1921084111
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8488
--
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] api,server,ui: tagged resource limits [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1925195584
Not interested at all in unnecessary noise like some, I'll move some of the offerings API related change to separate PRs. cc @vladimirpetrov
--
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] api,server,ui: tagged resource limits [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1859726157
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
Re: [PR] api,server,ui: granular resource limit management [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on code in PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#discussion_r1489990527
##########
server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java:
##########
@@ -679,4 +680,30 @@ public List<UserVmJoinVO> newUserVmView(VirtualMachine... vms) {
return searchByIds(vmIdSet.toArray(new Long[vmIdSet.size()]));
}
+ @Override
+ public List<UserVmJoinVO> listByAccountServiceOfferingTemplateAndNotInState(long accountId, List<State> states,
+ List<Long> offeringIds, List<Long> templateIds) {
+ SearchBuilder<UserVmJoinVO> userVmSearch = createSearchBuilder();
+ userVmSearch.and("accountId", userVmSearch.entity().getAccountId(), Op.EQ);
+ userVmSearch.and("serviceOfferingId", userVmSearch.entity().getServiceOfferingId(), Op.IN);
+ userVmSearch.and("templateId", userVmSearch.entity().getTemplateId(), Op.IN);
+ userVmSearch.and("state", userVmSearch.entity().getState(), SearchCriteria.Op.NIN);
+ userVmSearch.and("displayVm", userVmSearch.entity().isDisplayVm(), Op.EQ);
+ userVmSearch.groupBy(userVmSearch.entity().getId()); // select distinct
Review Comment:
Without group by it will return same VM entry for different nics, etc and may need handling in java
--
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] api,server,ui: granular resource limit management [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1945373391
@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] api,server,ui: tagged resource limits [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1900044614
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
Re: [PR] api,server,ui: granular resource limit management [cloudstack]
Posted by "GutoVeronezi (via GitHub)" <gi...@apache.org>.
GutoVeronezi commented on PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1948864377
> @JoaoJandre @GutoVeronezi , this has been reviewed by several and has gone through our QA procedure (manual and automated testing. Do you guys hace any concerns with merging this?
@DaanHoogland, my concerns are regarding the structure of the proposal, as I mentioned [here](https://github.com/apache/cloudstack/pull/8362#issuecomment-1924491548). Regarding the code itself, I did not have time to review 7,000+ lines of changes; therefore, I do not have an opinion on it. I am -0.
--
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] api,server,ui: granular resource limit management [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on code in PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#discussion_r1489187638
##########
server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java:
##########
@@ -679,4 +680,30 @@ public List<UserVmJoinVO> newUserVmView(VirtualMachine... vms) {
return searchByIds(vmIdSet.toArray(new Long[vmIdSet.size()]));
}
+ @Override
+ public List<UserVmJoinVO> listByAccountServiceOfferingTemplateAndNotInState(long accountId, List<State> states,
+ List<Long> offeringIds, List<Long> templateIds) {
+ SearchBuilder<UserVmJoinVO> userVmSearch = createSearchBuilder();
+ userVmSearch.and("accountId", userVmSearch.entity().getAccountId(), Op.EQ);
+ userVmSearch.and("serviceOfferingId", userVmSearch.entity().getServiceOfferingId(), Op.IN);
+ userVmSearch.and("templateId", userVmSearch.entity().getTemplateId(), Op.IN);
+ userVmSearch.and("state", userVmSearch.entity().getState(), SearchCriteria.Op.NIN);
+ userVmSearch.and("displayVm", userVmSearch.entity().isDisplayVm(), Op.EQ);
+ userVmSearch.groupBy(userVmSearch.entity().getId()); // select distinct
Review Comment:
Do we need **group by** 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] api,server,ui: granular resource limit management [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on code in PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#discussion_r1489240730
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -1038,8 +1041,8 @@ public VolumeVO createVolume(CreateVolumeCmd cmd) {
} finally {
if (!created) {
logger.trace("Decrementing volume resource count for account id=" + volume.getAccountId() + " as volume failed to create on the backend");
- _resourceLimitMgr.decrementResourceCount(volume.getAccountId(), ResourceType.volume, cmd.getDisplayVolume());
- _resourceLimitMgr.decrementResourceCount(volume.getAccountId(), ResourceType.primary_storage, cmd.getDisplayVolume(), new Long(volume.getSize()));
+ _resourceLimitMgr.decrementVolumeResourceCount(volume.getAccountId(), cmd.getDisplayVolume(),
+ volume.getSize(), _diskOfferingDao.findById(volume.getDiskOfferingId()));
Review Comment:
Should we use `findByIdIncludingRemoved` instead?
--
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] api,server,ui: granular resource limit management [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on code in PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#discussion_r1489243943
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -1527,10 +1530,11 @@ private VolumeVO orchestrateResizeVolume(long volumeId, long currentSize, long n
}
/* Update resource count for the account on primary storage resource */
+ DiskOffering diskOffering = _diskOfferingDao.findById(volume.getDiskOfferingId());
Review Comment:
Should we use `findByIdIncludingRemoved` instead?
--
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] api,server,ui: granular resource limit management [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on code in PR #8362:
URL: https://github.com/apache/cloudstack/pull/8362#discussion_r1489252719
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -8402,8 +8452,7 @@ private void postProcessingUnmanageVMVolumes(List<VolumeVO> volumes, UserVmVO vm
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_DELETE, volume.getAccountId(), volume.getDataCenterId(), volume.getId(), volume.getName(),
Volume.class.getName(), volume.getUuid(), volume.isDisplayVolume());
}
- _resourceLimitMgr.decrementResourceCount(vm.getAccountId(), ResourceType.volume);
- _resourceLimitMgr.decrementResourceCount(vm.getAccountId(), ResourceType.primary_storage, new Long(volume.getSize()));
+ _resourceLimitMgr.decrementVolumeResourceCount(vm.getAccountId(), volume.isDisplayVolume(), volume.getSize(), _diskOfferingDao.findById(volume.getDiskOfferingId()));
Review Comment:
findById or findByIdIncludingRemoved?
--
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