You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/02/22 06:56:00 UTC

[GitHub] [cloudstack] Spaceman1984 opened a new pull request #4717: Added recursive fetch of domains for listUsageRecords API call

Spaceman1984 opened a new pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717


   ### Description
   
   When calling the listUasageRecords API records per domain are fetched recursively. This is not the case if you specify a domain id.
   
   This PR adds a new parameter to enable fetching records recursively (isRecursive) when passing the domain id.
   
   <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
   
   <!-- For new features, provide link to FS, dev ML discussion etc. -->
   <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
   
   <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
   <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
   <!-- Fixes: # -->
   
   <!--- ********************************************************************************* -->
   <!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
   <!--- PLEASE PUT AN 'X' in only **ONE** box -->
   <!--- ********************************************************************************* -->
   
   ### 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)
   
   ### Feature/Enhancement Scale or Bug Severity
   
   #### Feature/Enhancement Scale
   
   - [ ] Major
   - [x] Minor
   
   ### 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 -->
   <!-- see how your change affects other areas of the code, etc. -->
   
   This has been tested by calling the API with different parameters. With/without a domain id and with/without isRecursive.
   
   <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 removed a comment on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 removed a comment on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-790408046


   > Why not create a `sb` @Spaceman1984 and then use it to create a `sc`?
   
   That would require a rewrite of the 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 removed a comment on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 removed a comment on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-783416729


   @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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 edited a comment on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 edited a comment on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-789597444


   > @Spaceman1984 can you test if it works as before if the domain admin can list usage records for an account within the (a) same domain, (b) a sub-domain, and (c) a different domain (the domain admin is not admin of that domain).
   
   @rhtyd I have already tested these scenarios. The (c) case should fail.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-790390637


   > Consider if you can reuse existing isrecursive/domainid handler code (see service layer that handles listVolumes, listVMs... etc).
   
   @rhtyd the _accountMgr.buildACLViewSearchBuilder(sb, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria);
   
   method is not compatible with how the records are fetched in the listUsageRecords class. 
   
   In getUsageRecords() in the UsageServiceImpl class file the result set is built using 
   `SearchCriteria<UsageVO> sc = _usageDao.createSearchCriteria();`
   , we don't have a SearchBuilder object to pass to the method.
   
   The code is not reusable.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r581825811



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/usage/ListUsageRecordsCmd.java
##########
@@ -85,6 +85,10 @@
     @Parameter(name = ApiConstants.OLD_FORMAT, type = CommandType.BOOLEAN, description = "Flag to enable description rendered in old format which uses internal database IDs instead of UUIDs. False by default.")
     private Boolean oldFormat;
 
+    @Parameter(name = ApiConstants.IS_RECURSIVE, type = CommandType.BOOLEAN,
+            description = "Specify if usage records should be fetched recursively per domain.")

Review comment:
       @Spaceman1984 you may add the `since` key here

##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -216,6 +216,31 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
             s_logger.debug("Account details not available. Using userContext accountId: " + accountId);
         }
 
+        // Check if a domain admin is allowed to access the requested account info.
+        if (_accountService.isDomainAdmin(caller.getId()) && accountId != null){
+            long accountDomainId = _accountDao.getDomainIdForGivenAccountId(accountId);
+            long callerDomainId = caller.getDomainId();
+            boolean matchFound = false;
+
+            if (callerDomainId == accountDomainId) {
+                matchFound = true;
+            } else {
+                // Check if the account is in a child domain of this domain admin.
+                List<DomainVO> childDomains = _domainDao.findAllChildren(_domainDao.findById(caller.getDomainId()).getPath(), caller.getDomainId());
+
+                for (DomainVO domainVO: childDomains) {
+                    if (accountDomainId == domainVO.getId()) {

Review comment:
       @Spaceman1984 can we use checkAccess method to simplify checking accountId domain access?
   `void checkAccess(Account account, Domain domain) throws PermissionDeniedException;`

##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -248,7 +273,17 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
             sc.addAnd("domainId", SearchCriteria.Op.IN, domainIds.toArray());
         }
 
-        if (domainId != null) {
+        if (cmd.isRecursive() && domainId != null){
+            SearchCriteria<DomainVO> sdc = _domainDao.createSearchCriteria();
+            sdc.addOr("path", SearchCriteria.Op.LIKE, _domainDao.findById(domainId).getPath() + "%");
+            List<DomainVO> domains = _domainDao.search(sdc, null);
+            List<Long> domainIds = new ArrayList<Long>();
+            for (DomainVO domain : domains)
+                domainIds.add(domain.getId());
+            sc.addAnd("domainId", SearchCriteria.Op.IN, domainIds.toArray());
+        }
+
+        if (!cmd.isRecursive() && domainId != null) {

Review comment:
       Will this add two different AND conditions when,
   Caller = Domain Admin
   isRecursive = False
   domainId = ID of a child domain




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-805746942


   @rhtyd a 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-788394775






----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 commented on pull request #4717: Added recursive fetch of domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-783137592


   @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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r580265549



##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -248,7 +248,17 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
             sc.addAnd("domainId", SearchCriteria.Op.IN, domainIds.toArray());
         }
 
-        if (domainId != null) {
+        if (cmd.isRecursive() && domainId != null){

Review comment:
       While testing I found that users cannot call this API, but domain admins can. I have added a check, if an account id is specified, if it belongs to any of the domains for the calling domain admin.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-801802154


   @rhtyd I don't care if it goes in 4.15 or not, as it is a minor enhancement. I wouldn't compare listUsageRecords to other list APIs though. Most testing done and ok. I just want to see another test run and am busy with that.
   clgtm 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-805352608


   <b>Trillian test result (tid-232)</b>
   Environment: vmware-60u2 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 34736 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4717-t232-vmware-60u2.zip
   Intermittent failure detected: /marvin/tests/smoke/test_accounts.py
   Intermittent failure detected: /marvin/tests/smoke/test_affinity_groups_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_async_job.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vgpu_enabled_vm.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vms_with_varied_deploymentplanners.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_with_userdata.py
   Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
   Intermittent failure detected: /marvin/tests/smoke/test_domain_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_list_ids_parameter.py
   Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py
   Intermittent failure detected: /marvin/tests/smoke/test_metrics_api.py
   Intermittent failure detected: /marvin/tests/smoke/test_multipleips_per_nic.py
   Intermittent failure detected: /marvin/tests/smoke/test_nested_virtualization.py
   Intermittent failure detected: /marvin/tests/smoke/test_network_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_network.py
   Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
   Intermittent failure detected: /marvin/tests/smoke/test_portforwardingrules.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
   Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers.py
   Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Smoke tests completed. 51 look OK, 35 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   ContextSuite context=TestAddVmToSubDomain>:setup | `Error` | 90.46 | test_accounts.py
   test_DeleteDomain | `Error` | 85.69 | test_accounts.py
   test_forceDeleteDomain | `Failure` | 94.10 | test_accounts.py
   test_forceDeleteDomain | `Error` | 119.74 | test_accounts.py
   test_01_user_remove_VM_running | `Error` | 85.67 | test_accounts.py
   test_DeployVmAntiAffinityGroup_in_project | `Error` | 88.26 | test_affinity_groups_projects.py
   test_query_async_job_result | `Error` | 141.68 | test_async_job.py
   ContextSuite context=TestLoadBalance>:setup | `Error` | 0.00 | test_loadbalance.py
   test_3d_gpu_support | `Error` | 126.90 | test_deploy_vgpu_enabled_vm.py
   test_list_vms_metrics | `Error` | 90.12 | test_metrics_api.py
   test_deployvm_firstfit | `Error` | 111.93 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userconcentrated | `Error` | 15.79 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userdispersing | `Error` | 22.67 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userdata | `Error` | 80.06 | test_deploy_vm_with_userdata.py
   test_deployvm_userdata_post | `Error` | 16.62 | test_deploy_vm_with_userdata.py
   test_network_acl | `Error` | 97.39 | test_network_acl.py
   ContextSuite context=TestRemoteDiagnostics>:setup | `Error` | 0.00 | test_diagnostics.py
   test_delete_account | `Error` | 98.64 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 11.49 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 12.56 | test_network.py
   test_deploy_vm_l2network | `Error` | 10.44 | test_network.py
   test_deploy_vm_l2network | `Error` | 11.50 | test_network.py
   test_l2network_restart | `Error` | 10.54 | test_network.py
   test_l2network_restart | `Error` | 11.61 | test_network.py
   ContextSuite context=TestL2Networks>:teardown | `Error` | 12.69 | test_network.py
   ContextSuite context=TestPortForwarding>:setup | `Error` | 88.89 | test_network.py
   ContextSuite context=TestPublicIP>:setup | `Error` | 83.78 | test_network.py
   test_reboot_router | `Error` | 91.48 | test_network.py
   test_releaseIP | `Error` | 77.14 | test_network.py
   ContextSuite context=TestRouterRules>:setup | `Error` | 152.24 | test_network.py
   test_03_deploy_vm_domain_service_offering | `Error` | 119.23 | test_domain_service_offerings.py
   ContextSuite context=TestIsolatedNetworksPasswdServer>:setup | `Error` | 0.00 | test_password_server.py
   test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | `Error` | 124.28 | test_internal_lb.py
   test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | `Error` | 126.42 | test_internal_lb.py
   test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Error` | 228.99 | test_internal_lb.py
   test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Error` | 231.44 | test_internal_lb.py
   test_03_vpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 137.10 | test_internal_lb.py
   test_03_vpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 138.51 | test_internal_lb.py
   test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 181.52 | test_internal_lb.py
   test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 182.89 | test_internal_lb.py
   test_01_deploy_kubernetes_cluster | `Failure` | 94.53 | test_kubernetes_clusters.py
   test_02_invalid_upgrade_kubernetes_cluster | `Failure` | 98.19 | test_kubernetes_clusters.py
   test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 97.05 | test_kubernetes_clusters.py
   test_04_deploy_and_scale_kubernetes_cluster | `Failure` | 87.81 | test_kubernetes_clusters.py
   test_05_delete_kubernetes_cluster | `Failure` | 85.75 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 77.69 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 79.80 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 83.83 | test_kubernetes_clusters.py
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 321.50 | test_kubernetes_clusters.py
   ContextSuite context=TestListIdsParams>:setup | `Error` | 0.00 | test_list_ids_parameter.py
   test_nic_secondaryip_add_remove | `Error` | 87.34 | test_multipleips_per_nic.py
   test_nested_virtualization_vmware | `Error` | 81.99 | test_nested_virtualization.py
   test_01_create_delete_portforwarding_fornonvpc | `Error` | 83.59 | test_portforwardingrules.py
   test_02_vpc_privategw_static_routes | `Failure` | 172.36 | test_privategw_acl.py
   test_02_vpc_privategw_static_routes | `Error` | 176.18 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 166.00 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Error` | 168.16 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Failure` | 278.25 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Error` | 279.42 | test_privategw_acl.py
   test_09_project_suspend | `Error` | 79.97 | test_projects.py
   test_10_project_activation | `Error` | 17.57 | test_projects.py
   ContextSuite context=TestResetVmOnReboot>:setup | `Error` | 0.00 | test_reset_vm_on_reboot.py
   test_01_so_removal_resource_update | `Error` | 79.75 | test_resource_accounting.py
   ContextSuite context=TestRouterDHCPHosts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDHCPOpts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDns>:setup | `Error` | 0.00 | test_router_dns.py
   ContextSuite context=TestRouterDnsService>:setup | `Error` | 0.00 | test_router_dnsservice.py
   test_02_routervm_iptables_policies | `Error` | 79.70 | test_routers_iptables_default_policy.py
   test_01_single_VPC_iptables_policies | `Error` | 80.06 | test_routers_iptables_default_policy.py
   test_01_single_VPC_iptables_policies | `Error` | 99.52 | test_routers_iptables_default_policy.py
   test_01_isolate_network_FW_PF_default_routes_egress_true | `Error` | 78.78 | test_routers_network_ops.py
   test_02_isolate_network_FW_PF_default_routes_egress_false | `Error` | 88.95 | test_routers_network_ops.py
   test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Error` | 160.47 | test_routers_network_ops.py
   test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Error` | 157.36 | test_routers_network_ops.py
   test_03_RVR_Network_check_router_state | `Error` | 153.58 | test_routers_network_ops.py
   ContextSuite context=TestRouterServices>:setup | `Error` | 0.00 | test_routers.py
   ContextSuite context=TestServiceOfferings>:setup | `Error` | 82.25 | test_service_offerings.py
   ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py
   ContextSuite context=TestVAppsVM>:setup | `Error` | 44.49 | test_vm_life_cycle.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 156.72 | test_vpc_redundant.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Error` | 172.21 | test_vpc_redundant.py
   test_02_redundant_VPC_default_routes | `Failure` | 157.83 | test_vpc_redundant.py
   test_02_redundant_VPC_default_routes | `Error` | 176.38 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 167.94 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Error` | 183.44 | test_vpc_redundant.py
   test_04_rvpc_network_garbage_collector_nics | `Failure` | 154.81 | test_vpc_redundant.py
   test_04_rvpc_network_garbage_collector_nics | `Error` | 172.34 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Failure` | 154.89 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Error` | 170.40 | test_vpc_redundant.py
   test_01_VPC_nics_after_destroy | `Failure` | 87.50 | test_vpc_router_nics.py
   test_02_VPC_default_routes | `Failure` | 87.53 | test_vpc_router_nics.py
   test_01_redundant_vpc_site2site_vpn | `Failure` | 272.62 | test_vpc_vpn.py
   test_01_redundant_vpc_site2site_vpn | `Error` | 274.76 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn_multiple_options | `Failure` | 143.23 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn_multiple_options | `Error` | 145.37 | test_vpc_vpn.py
   test_01_vpc_remote_access_vpn | `Failure` | 76.82 | test_vpc_vpn.py
   test_01_vpc_remote_access_vpn | `Error` | 77.89 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn | `Failure` | 132.07 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn | `Error` | 134.22 | test_vpc_vpn.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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r586256562



##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -234,22 +286,27 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
 
         SearchCriteria<UsageVO> sc = _usageDao.createSearchCriteria();
 
-        if (accountId != -1 && accountId != Account.ACCOUNT_ID_SYSTEM && !isAdmin && !isDomainAdmin) {
-            sc.addAnd("accountId", SearchCriteria.Op.EQ, accountId);
-        }
-
-        if (isDomainAdmin) {
-            SearchCriteria<DomainVO> sdc = _domainDao.createSearchCriteria();
-            sdc.addOr("path", SearchCriteria.Op.LIKE, _domainDao.findById(caller.getDomainId()).getPath() + "%");
-            List<DomainVO> domains = _domainDao.search(sdc, null);
-            List<Long> domainIds = new ArrayList<Long>();
-            for (DomainVO domain : domains)
-                domainIds.add(domain.getId());
-            sc.addAnd("domainId", SearchCriteria.Op.IN, domainIds.toArray());
+        if (accountId != -1 && accountId != Account.ACCOUNT_ID_SYSTEM && !isAdmin) {
+            // account exists and either domain on user role
+            // If not recursive and the account belongs to the user/domain admin, or the account was passed in, filter
+            if ((accountId == caller.getId() && !cmd.isRecursive()) || cmd.getAccountId() != null){
+                sc.addAnd("accountId", SearchCriteria.Op.EQ, accountId);
+            }
         }
 
         if (domainId != null) {
-            sc.addAnd("domainId", SearchCriteria.Op.EQ, domainId);
+            if (cmd.isRecursive()) {
+                SearchCriteria<DomainVO> sdc = _domainDao.createSearchCriteria();
+                sdc.addOr("path", SearchCriteria.Op.LIKE, _domainDao.findById(domainId).getPath() + "%");
+                List<DomainVO> domains = _domainDao.search(sdc, null);

Review comment:
       Simplify, reuse existing DomainDao::findAllChildren




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-801730644


   capacity problems, dismissing all blueorangutan 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-801076183


   @blueorangutan test centos7 vmware-60u2


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-800349101


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


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-806233575


   <b>Trillian test result (tid-251)</b>
   Environment: vmware-60u2 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 33701 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4717-t251-vmware-60u2.zip
   Intermittent failure detected: /marvin/tests/smoke/test_accounts.py
   Intermittent failure detected: /marvin/tests/smoke/test_affinity_groups_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_async_job.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vgpu_enabled_vm.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vms_with_varied_deploymentplanners.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_with_userdata.py
   Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
   Intermittent failure detected: /marvin/tests/smoke/test_domain_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_list_ids_parameter.py
   Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py
   Intermittent failure detected: /marvin/tests/smoke/test_metrics_api.py
   Intermittent failure detected: /marvin/tests/smoke/test_multipleips_per_nic.py
   Intermittent failure detected: /marvin/tests/smoke/test_nested_virtualization.py
   Intermittent failure detected: /marvin/tests/smoke/test_network_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_network.py
   Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
   Intermittent failure detected: /marvin/tests/smoke/test_portforwardingrules.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
   Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers.py
   Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Smoke tests completed. 50 look OK, 36 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   ContextSuite context=TestAddVmToSubDomain>:setup | `Error` | 110.53 | test_accounts.py
   test_DeleteDomain | `Error` | 103.07 | test_accounts.py
   test_forceDeleteDomain | `Failure` | 118.90 | test_accounts.py
   test_forceDeleteDomain | `Error` | 159.29 | test_accounts.py
   test_01_user_remove_VM_running | `Error` | 99.91 | test_accounts.py
   test_DeployVmAntiAffinityGroup_in_project | `Error` | 112.57 | test_affinity_groups_projects.py
   test_query_async_job_result | `Error` | 105.66 | test_async_job.py
   ContextSuite context=TestLoadBalance>:setup | `Error` | 0.00 | test_loadbalance.py
   test_3d_gpu_support | `Error` | 123.28 | test_deploy_vgpu_enabled_vm.py
   test_list_vms_metrics | `Error` | 77.84 | test_metrics_api.py
   test_deployvm_firstfit | `Error` | 79.72 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userconcentrated | `Error` | 15.56 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userdispersing | `Error` | 16.52 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userdata | `Error` | 81.99 | test_deploy_vm_with_userdata.py
   test_deployvm_userdata_post | `Error` | 16.54 | test_deploy_vm_with_userdata.py
   test_network_acl | `Error` | 87.10 | test_network_acl.py
   ContextSuite context=TestRemoteDiagnostics>:setup | `Error` | 0.00 | test_diagnostics.py
   test_delete_account | `Error` | 78.08 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 10.43 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 11.49 | test_network.py
   test_deploy_vm_l2network | `Error` | 11.44 | test_network.py
   test_deploy_vm_l2network | `Error` | 12.50 | test_network.py
   test_l2network_restart | `Error` | 11.48 | test_network.py
   test_l2network_restart | `Error` | 12.54 | test_network.py
   ContextSuite context=TestL2Networks>:teardown | `Error` | 13.63 | test_network.py
   ContextSuite context=TestPortForwarding>:setup | `Error` | 89.73 | test_network.py
   ContextSuite context=TestPublicIP>:setup | `Error` | 71.51 | test_network.py
   test_reboot_router | `Error` | 81.22 | test_network.py
   test_releaseIP | `Error` | 78.22 | test_network.py
   ContextSuite context=TestRouterRules>:setup | `Error` | 152.37 | test_network.py
   test_03_deploy_vm_domain_service_offering | `Error` | 77.44 | test_domain_service_offerings.py
   ContextSuite context=TestIsolatedNetworksPasswdServer>:setup | `Error` | 0.00 | test_password_server.py
   test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | `Error` | 86.75 | test_internal_lb.py
   test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | `Error` | 88.89 | test_internal_lb.py
   test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Error` | 164.03 | test_internal_lb.py
   test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Error` | 166.18 | test_internal_lb.py
   test_03_vpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 88.37 | test_internal_lb.py
   test_03_vpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 89.46 | test_internal_lb.py
   test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 154.64 | test_internal_lb.py
   test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 155.71 | test_internal_lb.py
   test_01_deploy_kubernetes_cluster | `Failure` | 100.70 | test_kubernetes_clusters.py
   test_02_invalid_upgrade_kubernetes_cluster | `Failure` | 95.16 | test_kubernetes_clusters.py
   test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 78.74 | test_kubernetes_clusters.py
   test_04_deploy_and_scale_kubernetes_cluster | `Failure` | 78.74 | test_kubernetes_clusters.py
   test_05_delete_kubernetes_cluster | `Failure` | 80.75 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 85.79 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 78.62 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 77.60 | test_kubernetes_clusters.py
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 115.82 | test_kubernetes_clusters.py
   ContextSuite context=TestListIdsParams>:setup | `Error` | 0.00 | test_list_ids_parameter.py
   test_nic_secondaryip_add_remove | `Error` | 76.14 | test_multipleips_per_nic.py
   test_nested_virtualization_vmware | `Error` | 82.87 | test_nested_virtualization.py
   test_01_create_delete_portforwarding_fornonvpc | `Error` | 73.25 | test_portforwardingrules.py
   test_02_vpc_privategw_static_routes | `Failure` | 141.70 | test_privategw_acl.py
   test_02_vpc_privategw_static_routes | `Error` | 143.86 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 139.67 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Error` | 141.82 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Failure` | 268.02 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Error` | 270.18 | test_privategw_acl.py
   test_09_project_suspend | `Error` | 84.93 | test_projects.py
   test_10_project_activation | `Error` | 18.64 | test_projects.py
   ContextSuite context=TestResetVmOnReboot>:setup | `Error` | 0.00 | test_reset_vm_on_reboot.py
   test_01_so_removal_resource_update | `Error` | 69.58 | test_resource_accounting.py
   ContextSuite context=TestRouterDHCPHosts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDHCPOpts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDns>:setup | `Error` | 0.00 | test_router_dns.py
   ContextSuite context=TestRouterDnsService>:setup | `Error` | 0.00 | test_router_dnsservice.py
   test_02_routervm_iptables_policies | `Error` | 84.82 | test_routers_iptables_default_policy.py
   test_01_single_VPC_iptables_policies | `Error` | 81.05 | test_routers_iptables_default_policy.py
   test_01_single_VPC_iptables_policies | `Error` | 99.52 | test_routers_iptables_default_policy.py
   test_01_isolate_network_FW_PF_default_routes_egress_true | `Error` | 66.67 | test_routers_network_ops.py
   test_02_isolate_network_FW_PF_default_routes_egress_false | `Error` | 63.50 | test_routers_network_ops.py
   test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Error` | 149.51 | test_routers_network_ops.py
   test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Error` | 148.99 | test_routers_network_ops.py
   test_03_RVR_Network_check_router_state | `Error` | 158.38 | test_routers_network_ops.py
   ContextSuite context=TestRouterServices>:setup | `Error` | 0.00 | test_routers.py
   ContextSuite context=TestServiceOfferings>:setup | `Error` | 77.12 | test_service_offerings.py
   ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py
   test_08_reboot_cpvm | `Failure` | 26.73 | test_ssvm.py
   ContextSuite context=TestVAppsVM>:setup | `Error` | 45.44 | test_vm_life_cycle.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 177.17 | test_vpc_redundant.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Error` | 195.70 | test_vpc_redundant.py
   test_02_redundant_VPC_default_routes | `Failure` | 165.92 | test_vpc_redundant.py
   test_02_redundant_VPC_default_routes | `Error` | 181.40 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 161.93 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Error` | 179.45 | test_vpc_redundant.py
   test_04_rvpc_network_garbage_collector_nics | `Failure` | 156.86 | test_vpc_redundant.py
   test_04_rvpc_network_garbage_collector_nics | `Error` | 173.37 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Failure` | 149.90 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Error` | 167.44 | test_vpc_redundant.py
   test_01_VPC_nics_after_destroy | `Failure` | 77.38 | test_vpc_router_nics.py
   test_02_VPC_default_routes | `Failure` | 88.57 | test_vpc_router_nics.py
   test_01_redundant_vpc_site2site_vpn | `Failure` | 294.01 | test_vpc_vpn.py
   test_01_redundant_vpc_site2site_vpn | `Error` | 296.17 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn_multiple_options | `Failure` | 149.32 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn_multiple_options | `Error` | 151.46 | test_vpc_vpn.py
   test_01_vpc_remote_access_vpn | `Failure` | 85.04 | test_vpc_vpn.py
   test_01_vpc_remote_access_vpn | `Error` | 86.11 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn | `Failure` | 136.08 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn | `Error` | 138.22 | test_vpc_vpn.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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-800349101


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


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-805569574


   @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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-805566257


   Packaging result: :heavy_multiplication_x: centos7 :heavy_multiplication_x: centos8 :heavy_multiplication_x: debian. SL-JID 224


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-801619693






----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 removed a comment on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 removed a comment on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-801018985


   @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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-787712983


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


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-793481767


   > Why not create a `sb` @Spaceman1984 and then use it to create a `sc`?
   
   @rhtyd the sb object needed here needs a VO class that implements ControlledEntity. UsageVO does not implement ControlledEntity.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-809280179


   Is this ready for merging @DaanHoogland @Spaceman1984 ?


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-789597444


   > @Spaceman1984 can you test if it works as before if the domain admin can list usage records for an account within the (a) same domain, (b) a sub-domain, and (c) a different domain (the domain admin is not admin of that domain).
   
   I have already tested these scenarios. The (c) case should fail.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-787688652


   @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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r586290505



##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -210,12 +212,62 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
             //If account_id or account_name is explicitly mentioned, list records for the specified account only even if the caller is of type admin
             if (_accountService.isRootAdmin(caller.getId())) {
                 isAdmin = true;
-            } else if (_accountService.isDomainAdmin(caller.getId())) {
-                isDomainAdmin = true;
             }
             s_logger.debug("Account details not available. Using userContext accountId: " + accountId);
         }
 
+        // Check if a domain admin is allowed to access the requested domain id
+        if (isDomainAdmin) {
+            if (domainId != null) {
+                Account callerAccount = _accountService.getAccount(caller.getId());
+                Domain domain = _domainDao.findById(domainId);
+                _accountService.checkAccess(callerAccount, domain);
+            } else {
+                // Domain admins can only access their own domain's usage records.
+                // Set the domain if not specified.
+                domainId = caller.getDomainId();
+            }
+
+            // Check if a domain admin is allowed to access the requested account info.
+            Account account = _accountService.getAccount(accountId);

Review comment:
       @rhtyd Using checkAccess() here will result in querying the database for every child domain. I'm querying the database once by asking for all the child domains of the current domain and checking if the requested account belongs to anyone of them.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland removed a comment on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
DaanHoogland removed a comment on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-801076183






----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-806233575


   <b>Trillian test result (tid-251)</b>
   Environment: vmware-60u2 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 33701 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4717-t251-vmware-60u2.zip
   Intermittent failure detected: /marvin/tests/smoke/test_accounts.py
   Intermittent failure detected: /marvin/tests/smoke/test_affinity_groups_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_async_job.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vgpu_enabled_vm.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vms_with_varied_deploymentplanners.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_with_userdata.py
   Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
   Intermittent failure detected: /marvin/tests/smoke/test_domain_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_list_ids_parameter.py
   Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py
   Intermittent failure detected: /marvin/tests/smoke/test_metrics_api.py
   Intermittent failure detected: /marvin/tests/smoke/test_multipleips_per_nic.py
   Intermittent failure detected: /marvin/tests/smoke/test_nested_virtualization.py
   Intermittent failure detected: /marvin/tests/smoke/test_network_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_network.py
   Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
   Intermittent failure detected: /marvin/tests/smoke/test_portforwardingrules.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
   Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers.py
   Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Smoke tests completed. 50 look OK, 36 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   ContextSuite context=TestAddVmToSubDomain>:setup | `Error` | 110.53 | test_accounts.py
   test_DeleteDomain | `Error` | 103.07 | test_accounts.py
   test_forceDeleteDomain | `Failure` | 118.90 | test_accounts.py
   test_forceDeleteDomain | `Error` | 159.29 | test_accounts.py
   test_01_user_remove_VM_running | `Error` | 99.91 | test_accounts.py
   test_DeployVmAntiAffinityGroup_in_project | `Error` | 112.57 | test_affinity_groups_projects.py
   test_query_async_job_result | `Error` | 105.66 | test_async_job.py
   ContextSuite context=TestLoadBalance>:setup | `Error` | 0.00 | test_loadbalance.py
   test_3d_gpu_support | `Error` | 123.28 | test_deploy_vgpu_enabled_vm.py
   test_list_vms_metrics | `Error` | 77.84 | test_metrics_api.py
   test_deployvm_firstfit | `Error` | 79.72 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userconcentrated | `Error` | 15.56 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userdispersing | `Error` | 16.52 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userdata | `Error` | 81.99 | test_deploy_vm_with_userdata.py
   test_deployvm_userdata_post | `Error` | 16.54 | test_deploy_vm_with_userdata.py
   test_network_acl | `Error` | 87.10 | test_network_acl.py
   ContextSuite context=TestRemoteDiagnostics>:setup | `Error` | 0.00 | test_diagnostics.py
   test_delete_account | `Error` | 78.08 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 10.43 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 11.49 | test_network.py
   test_deploy_vm_l2network | `Error` | 11.44 | test_network.py
   test_deploy_vm_l2network | `Error` | 12.50 | test_network.py
   test_l2network_restart | `Error` | 11.48 | test_network.py
   test_l2network_restart | `Error` | 12.54 | test_network.py
   ContextSuite context=TestL2Networks>:teardown | `Error` | 13.63 | test_network.py
   ContextSuite context=TestPortForwarding>:setup | `Error` | 89.73 | test_network.py
   ContextSuite context=TestPublicIP>:setup | `Error` | 71.51 | test_network.py
   test_reboot_router | `Error` | 81.22 | test_network.py
   test_releaseIP | `Error` | 78.22 | test_network.py
   ContextSuite context=TestRouterRules>:setup | `Error` | 152.37 | test_network.py
   test_03_deploy_vm_domain_service_offering | `Error` | 77.44 | test_domain_service_offerings.py
   ContextSuite context=TestIsolatedNetworksPasswdServer>:setup | `Error` | 0.00 | test_password_server.py
   test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | `Error` | 86.75 | test_internal_lb.py
   test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | `Error` | 88.89 | test_internal_lb.py
   test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Error` | 164.03 | test_internal_lb.py
   test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Error` | 166.18 | test_internal_lb.py
   test_03_vpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 88.37 | test_internal_lb.py
   test_03_vpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 89.46 | test_internal_lb.py
   test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 154.64 | test_internal_lb.py
   test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 155.71 | test_internal_lb.py
   test_01_deploy_kubernetes_cluster | `Failure` | 100.70 | test_kubernetes_clusters.py
   test_02_invalid_upgrade_kubernetes_cluster | `Failure` | 95.16 | test_kubernetes_clusters.py
   test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 78.74 | test_kubernetes_clusters.py
   test_04_deploy_and_scale_kubernetes_cluster | `Failure` | 78.74 | test_kubernetes_clusters.py
   test_05_delete_kubernetes_cluster | `Failure` | 80.75 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 85.79 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 78.62 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 77.60 | test_kubernetes_clusters.py
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 115.82 | test_kubernetes_clusters.py
   ContextSuite context=TestListIdsParams>:setup | `Error` | 0.00 | test_list_ids_parameter.py
   test_nic_secondaryip_add_remove | `Error` | 76.14 | test_multipleips_per_nic.py
   test_nested_virtualization_vmware | `Error` | 82.87 | test_nested_virtualization.py
   test_01_create_delete_portforwarding_fornonvpc | `Error` | 73.25 | test_portforwardingrules.py
   test_02_vpc_privategw_static_routes | `Failure` | 141.70 | test_privategw_acl.py
   test_02_vpc_privategw_static_routes | `Error` | 143.86 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 139.67 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Error` | 141.82 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Failure` | 268.02 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Error` | 270.18 | test_privategw_acl.py
   test_09_project_suspend | `Error` | 84.93 | test_projects.py
   test_10_project_activation | `Error` | 18.64 | test_projects.py
   ContextSuite context=TestResetVmOnReboot>:setup | `Error` | 0.00 | test_reset_vm_on_reboot.py
   test_01_so_removal_resource_update | `Error` | 69.58 | test_resource_accounting.py
   ContextSuite context=TestRouterDHCPHosts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDHCPOpts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDns>:setup | `Error` | 0.00 | test_router_dns.py
   ContextSuite context=TestRouterDnsService>:setup | `Error` | 0.00 | test_router_dnsservice.py
   test_02_routervm_iptables_policies | `Error` | 84.82 | test_routers_iptables_default_policy.py
   test_01_single_VPC_iptables_policies | `Error` | 81.05 | test_routers_iptables_default_policy.py
   test_01_single_VPC_iptables_policies | `Error` | 99.52 | test_routers_iptables_default_policy.py
   test_01_isolate_network_FW_PF_default_routes_egress_true | `Error` | 66.67 | test_routers_network_ops.py
   test_02_isolate_network_FW_PF_default_routes_egress_false | `Error` | 63.50 | test_routers_network_ops.py
   test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Error` | 149.51 | test_routers_network_ops.py
   test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Error` | 148.99 | test_routers_network_ops.py
   test_03_RVR_Network_check_router_state | `Error` | 158.38 | test_routers_network_ops.py
   ContextSuite context=TestRouterServices>:setup | `Error` | 0.00 | test_routers.py
   ContextSuite context=TestServiceOfferings>:setup | `Error` | 77.12 | test_service_offerings.py
   ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py
   test_08_reboot_cpvm | `Failure` | 26.73 | test_ssvm.py
   ContextSuite context=TestVAppsVM>:setup | `Error` | 45.44 | test_vm_life_cycle.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 177.17 | test_vpc_redundant.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Error` | 195.70 | test_vpc_redundant.py
   test_02_redundant_VPC_default_routes | `Failure` | 165.92 | test_vpc_redundant.py
   test_02_redundant_VPC_default_routes | `Error` | 181.40 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 161.93 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Error` | 179.45 | test_vpc_redundant.py
   test_04_rvpc_network_garbage_collector_nics | `Failure` | 156.86 | test_vpc_redundant.py
   test_04_rvpc_network_garbage_collector_nics | `Error` | 173.37 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Failure` | 149.90 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Error` | 167.44 | test_vpc_redundant.py
   test_01_VPC_nics_after_destroy | `Failure` | 77.38 | test_vpc_router_nics.py
   test_02_VPC_default_routes | `Failure` | 88.57 | test_vpc_router_nics.py
   test_01_redundant_vpc_site2site_vpn | `Failure` | 294.01 | test_vpc_vpn.py
   test_01_redundant_vpc_site2site_vpn | `Error` | 296.17 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn_multiple_options | `Failure` | 149.32 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn_multiple_options | `Error` | 151.46 | test_vpc_vpn.py
   test_01_vpc_remote_access_vpn | `Failure` | 85.04 | test_vpc_vpn.py
   test_01_vpc_remote_access_vpn | `Error` | 86.11 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn | `Failure` | 136.08 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn | `Error` | 138.22 | test_vpc_vpn.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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-783137976


   @Spaceman1984 a Jenkins job has been kicked to build packages. 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-801076266


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


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-793640938


   Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 56


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-812710394


   <b>Trillian test result (tid-331)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 38841 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4717-t331-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
   Smoke tests completed. 84 look OK, 2 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 3615.24 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 42.91 | test_kubernetes_clusters.py
   test_01_migrate_VM_and_root_volume | `Error` | 67.22 | test_vm_life_cycle.py
   test_02_migrate_VM_with_two_data_disks | `Error` | 50.09 | test_vm_life_cycle.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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd merged pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
rhtyd merged pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717


   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r584468447



##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -234,22 +269,23 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
 
         SearchCriteria<UsageVO> sc = _usageDao.createSearchCriteria();
 
-        if (accountId != -1 && accountId != Account.ACCOUNT_ID_SYSTEM && !isAdmin && !isDomainAdmin) {
+        if (accountId != -1 && accountId != Account.ACCOUNT_ID_SYSTEM && !isAdmin && !cmd.isRecursive()) {

Review comment:
       @Spaceman1984 is this change needed?

##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -210,12 +212,45 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
             //If account_id or account_name is explicitly mentioned, list records for the specified account only even if the caller is of type admin
             if (_accountService.isRootAdmin(caller.getId())) {
                 isAdmin = true;
-            } else if (_accountService.isDomainAdmin(caller.getId())) {
-                isDomainAdmin = true;
             }
             s_logger.debug("Account details not available. Using userContext accountId: " + accountId);
         }
 
+        // Check if a domain admin is allowed to access the requested domain id
+        if (isDomainAdmin) {
+            if (domainId != null) {
+                Account callerAccount = _accountService.getAccount(caller.getId());
+                Domain domain = _domainDao.findById(domainId);
+                _accountService.checkAccess(callerAccount, domain);
+            } else {
+                // Domain admins can only access their own domain's usage records.
+                // Set the domain if not specified.
+                domainId = caller.getDomainId();
+            }
+
+            // Check if a domain admin is allowed to access the requested account info.
+            Account account = _accountService.getAccount(accountId);
+            Domain domain = _domainDao.findById(caller.getDomainId());
+            _accountService.checkAccess(account, domain);

Review comment:
       Not sure if this check is correct. Probably what you want to check is if the account belongs to the same domain or subdomain as the caller but this is checking opposite.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] andrijapanicsb commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
andrijapanicsb commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-809301419


   Not until IR is done.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r581736881



##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -216,6 +216,31 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
             s_logger.debug("Account details not available. Using userContext accountId: " + accountId);
         }
 
+        // Check if a domain admin is allowed to access the requested account info.
+        if (_accountService.isDomainAdmin(caller.getId()) && accountId != null){

Review comment:
       A normal user doesn't have access to this API, this check is only applicable to domain admin.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-793966231


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


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-787803120


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


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r583592713



##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -216,6 +216,31 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
             s_logger.debug("Account details not available. Using userContext accountId: " + accountId);
         }
 
+        // Check if a domain admin is allowed to access the requested account info.
+        if (_accountService.isDomainAdmin(caller.getId()) && accountId != null){
+            long accountDomainId = _accountDao.getDomainIdForGivenAccountId(accountId);
+            long callerDomainId = caller.getDomainId();
+            boolean matchFound = false;
+
+            if (callerDomainId == accountDomainId) {
+                matchFound = true;
+            } else {
+                // Check if the account is in a child domain of this domain admin.
+                List<DomainVO> childDomains = _domainDao.findAllChildren(_domainDao.findById(caller.getDomainId()).getPath(), caller.getDomainId());
+
+                for (DomainVO domainVO: childDomains) {
+                    if (accountDomainId == domainVO.getId()) {

Review comment:
       Addressed




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r580265549



##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -248,7 +248,17 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
             sc.addAnd("domainId", SearchCriteria.Op.IN, domainIds.toArray());
         }
 
-        if (domainId != null) {
+        if (cmd.isRecursive() && domainId != null){

Review comment:
       While testing I found that users cannot call this API, but domain admins can. I have added a check, if an account id is specified, if it belongs to any of the domains of the calling domain admin.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-801050535


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


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-801605157


   <b>Trillian test result (tid-3687)</b>
   Environment: vmware-60u2 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 34547 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4717-t3687-vmware-60u2.zip
   Intermittent failure detected: /marvin/tests/smoke/test_accounts.py
   Intermittent failure detected: /marvin/tests/smoke/test_affinity_groups_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_async_job.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vgpu_enabled_vm.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vms_with_varied_deploymentplanners.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_with_userdata.py
   Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
   Intermittent failure detected: /marvin/tests/smoke/test_domain_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_list_ids_parameter.py
   Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py
   Intermittent failure detected: /marvin/tests/smoke/test_metrics_api.py
   Intermittent failure detected: /marvin/tests/smoke/test_multipleips_per_nic.py
   Intermittent failure detected: /marvin/tests/smoke/test_nested_virtualization.py
   Intermittent failure detected: /marvin/tests/smoke/test_network_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_network.py
   Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
   Intermittent failure detected: /marvin/tests/smoke/test_portforwardingrules.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
   Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers.py
   Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Smoke tests completed. 51 look OK, 35 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   ContextSuite context=TestAddVmToSubDomain>:setup | `Error` | 115.78 | test_accounts.py
   test_DeleteDomain | `Error` | 102.77 | test_accounts.py
   test_forceDeleteDomain | `Failure` | 95.75 | test_accounts.py
   test_forceDeleteDomain | `Error` | 119.25 | test_accounts.py
   test_01_user_remove_VM_running | `Error` | 112.86 | test_accounts.py
   test_DeployVmAntiAffinityGroup_in_project | `Error` | 115.39 | test_affinity_groups_projects.py
   test_query_async_job_result | `Error` | 100.45 | test_async_job.py
   ContextSuite context=TestLoadBalance>:setup | `Error` | 0.00 | test_loadbalance.py
   test_3d_gpu_support | `Error` | 115.84 | test_deploy_vgpu_enabled_vm.py
   test_list_vms_metrics | `Error` | 93.31 | test_metrics_api.py
   test_deployvm_firstfit | `Error` | 92.95 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userconcentrated | `Error` | 15.51 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userdispersing | `Error` | 12.47 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userdata | `Error` | 111.32 | test_deploy_vm_with_userdata.py
   test_deployvm_userdata_post | `Error` | 15.53 | test_deploy_vm_with_userdata.py
   test_network_acl | `Error` | 71.88 | test_network_acl.py
   ContextSuite context=TestRemoteDiagnostics>:setup | `Error` | 0.00 | test_diagnostics.py
   test_delete_account | `Error` | 103.60 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 10.41 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 11.48 | test_network.py
   test_deploy_vm_l2network | `Error` | 9.43 | test_network.py
   test_deploy_vm_l2network | `Error` | 10.49 | test_network.py
   test_l2network_restart | `Error` | 11.48 | test_network.py
   test_l2network_restart | `Error` | 12.54 | test_network.py
   ContextSuite context=TestL2Networks>:teardown | `Error` | 13.62 | test_network.py
   ContextSuite context=TestPortForwarding>:setup | `Error` | 112.16 | test_network.py
   ContextSuite context=TestPublicIP>:setup | `Error` | 99.15 | test_network.py
   test_reboot_router | `Error` | 98.56 | test_network.py
   test_releaseIP | `Error` | 94.44 | test_network.py
   ContextSuite context=TestRouterRules>:setup | `Error` | 189.94 | test_network.py
   test_03_deploy_vm_domain_service_offering | `Error` | 110.86 | test_domain_service_offerings.py
   ContextSuite context=TestIsolatedNetworksPasswdServer>:setup | `Error` | 0.00 | test_password_server.py
   test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | `Error` | 68.12 | test_internal_lb.py
   test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | `Error` | 70.24 | test_internal_lb.py
   test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Error` | 136.30 | test_internal_lb.py
   test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Error` | 138.44 | test_internal_lb.py
   test_03_vpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 89.36 | test_internal_lb.py
   test_03_vpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 90.43 | test_internal_lb.py
   test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 128.00 | test_internal_lb.py
   test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 129.06 | test_internal_lb.py
   test_01_deploy_kubernetes_cluster | `Failure` | 111.89 | test_kubernetes_clusters.py
   test_02_invalid_upgrade_kubernetes_cluster | `Failure` | 104.24 | test_kubernetes_clusters.py
   test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 95.03 | test_kubernetes_clusters.py
   test_04_deploy_and_scale_kubernetes_cluster | `Failure` | 90.92 | test_kubernetes_clusters.py
   test_05_delete_kubernetes_cluster | `Failure` | 89.84 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 82.72 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 91.88 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 90.82 | test_kubernetes_clusters.py
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 312.94 | test_kubernetes_clusters.py
   ContextSuite context=TestListIdsParams>:setup | `Error` | 0.00 | test_list_ids_parameter.py
   test_nic_secondaryip_add_remove | `Error` | 100.52 | test_multipleips_per_nic.py
   test_nested_virtualization_vmware | `Error` | 93.14 | test_nested_virtualization.py
   test_01_create_delete_portforwarding_fornonvpc | `Error` | 111.02 | test_portforwardingrules.py
   test_02_vpc_privategw_static_routes | `Failure` | 122.31 | test_privategw_acl.py
   test_02_vpc_privategw_static_routes | `Error` | 124.46 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 111.07 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Error` | 113.23 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Failure` | 246.60 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Error` | 248.76 | test_privategw_acl.py
   test_09_project_suspend | `Error` | 95.08 | test_projects.py
   test_10_project_activation | `Error` | 13.52 | test_projects.py
   ContextSuite context=TestResetVmOnReboot>:setup | `Error` | 0.00 | test_reset_vm_on_reboot.py
   test_01_so_removal_resource_update | `Error` | 95.13 | test_resource_accounting.py
   ContextSuite context=TestRouterDHCPHosts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDHCPOpts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDns>:setup | `Error` | 0.00 | test_router_dns.py
   ContextSuite context=TestRouterDnsService>:setup | `Error` | 0.00 | test_router_dnsservice.py
   test_02_routervm_iptables_policies | `Error` | 89.99 | test_routers_iptables_default_policy.py
   test_01_single_VPC_iptables_policies | `Error` | 67.82 | test_routers_iptables_default_policy.py
   test_01_single_VPC_iptables_policies | `Error` | 82.22 | test_routers_iptables_default_policy.py
   test_01_isolate_network_FW_PF_default_routes_egress_true | `Error` | 99.20 | test_routers_network_ops.py
   test_02_isolate_network_FW_PF_default_routes_egress_false | `Error` | 88.95 | test_routers_network_ops.py
   test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Error` | 183.19 | test_routers_network_ops.py
   test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Error` | 190.32 | test_routers_network_ops.py
   test_03_RVR_Network_check_router_state | `Error` | 179.67 | test_routers_network_ops.py
   ContextSuite context=TestRouterServices>:setup | `Error` | 0.00 | test_routers.py
   ContextSuite context=TestServiceOfferings>:setup | `Error` | 99.64 | test_service_offerings.py
   ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py
   ContextSuite context=TestVAppsVM>:setup | `Error` | 44.39 | test_vm_life_cycle.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 116.98 | test_vpc_redundant.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Error` | 132.45 | test_vpc_redundant.py
   test_02_redundant_VPC_default_routes | `Failure` | 109.01 | test_vpc_redundant.py
   test_02_redundant_VPC_default_routes | `Error` | 126.54 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 122.09 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Error` | 135.50 | test_vpc_redundant.py
   test_04_rvpc_network_garbage_collector_nics | `Failure` | 125.35 | test_vpc_redundant.py
   test_04_rvpc_network_garbage_collector_nics | `Error` | 141.82 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Failure` | 121.07 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Error` | 138.55 | test_vpc_redundant.py
   test_01_VPC_nics_after_destroy | `Failure` | 66.08 | test_vpc_router_nics.py
   test_02_VPC_default_routes | `Failure` | 73.10 | test_vpc_router_nics.py
   test_01_redundant_vpc_site2site_vpn | `Failure` | 208.56 | test_vpc_vpn.py
   test_01_redundant_vpc_site2site_vpn | `Error` | 210.69 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn_multiple_options | `Failure` | 109.68 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn_multiple_options | `Error` | 111.79 | test_vpc_vpn.py
   test_01_vpc_remote_access_vpn | `Failure` | 68.63 | test_vpc_vpn.py
   test_01_vpc_remote_access_vpn | `Error` | 69.69 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn | `Failure` | 103.48 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn | `Error` | 105.59 | test_vpc_vpn.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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-789637962


   Consider if you can reuse existing isrecursive/domainid handler code (see service layer that handles listVolumes, listVMs... etc).


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r581718683



##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -216,6 +216,31 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
             s_logger.debug("Account details not available. Using userContext accountId: " + accountId);
         }
 
+        // Check if a domain admin is allowed to access the requested account info.
+        if (_accountService.isDomainAdmin(caller.getId()) && accountId != null){
+            long accountDomainId = _accountDao.getDomainIdForGivenAccountId(accountId);
+            long callerDomainId = caller.getDomainId();
+            boolean matchFound = false;
+
+            if (callerDomainId == accountDomainId) {
+                matchFound = true;
+            } else {
+                // Check if the account is in a child domain of this domain admin.
+                List<DomainVO> childDomains = _domainDao.findAllChildren(_domainDao.findById(caller.getDomainId()).getPath(), caller.getDomainId());
+
+                for (DomainVO domainVO: childDomains) {
+                    if (accountDomainId == domainVO.getId()) {

Review comment:
       Why not use the existing `_accountMgr.buildACLSearchParameters`?
   And should the be done for all caller types, as: (this is just suggestion, I've not investigated this)
   ```
   Root admin - allow recursive usage records for all domains
   Domain admin - only allow recursive usage records for its own domain
   User - don't allow recursive domain ID (it should be only allowed to see their own usage records)
   ```




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r586255367



##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -234,22 +286,27 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
 
         SearchCriteria<UsageVO> sc = _usageDao.createSearchCriteria();
 
-        if (accountId != -1 && accountId != Account.ACCOUNT_ID_SYSTEM && !isAdmin && !isDomainAdmin) {
-            sc.addAnd("accountId", SearchCriteria.Op.EQ, accountId);
-        }
-
-        if (isDomainAdmin) {
-            SearchCriteria<DomainVO> sdc = _domainDao.createSearchCriteria();
-            sdc.addOr("path", SearchCriteria.Op.LIKE, _domainDao.findById(caller.getDomainId()).getPath() + "%");
-            List<DomainVO> domains = _domainDao.search(sdc, null);
-            List<Long> domainIds = new ArrayList<Long>();
-            for (DomainVO domain : domains)
-                domainIds.add(domain.getId());
-            sc.addAnd("domainId", SearchCriteria.Op.IN, domainIds.toArray());
+        if (accountId != -1 && accountId != Account.ACCOUNT_ID_SYSTEM && !isAdmin) {

Review comment:
       Please simplify 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-803345361


   <b>[S] Trillian test result (tid-179)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 101325 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4717-t179-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_virtio_scsi_vm.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_extra_config_data.py
   Intermittent failure detected: /marvin/tests/smoke/test_human_readable_logs.py
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_supported_versions.py
   Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py
   Intermittent failure detected: /marvin/tests/smoke/test_network.py
   Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers.py
   Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Intermittent failure detected: /marvin/tests/smoke/test_host_maintenance.py
   Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
   Smoke tests completed. 60 look OK, 26 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_create_lb_rule_src_nat | `Failure` | 308.28 | test_loadbalance.py
   test_02_create_lb_rule_non_nat | `Failure` | 34.55 | test_loadbalance.py
   test_assign_and_removal_lb | `Failure` | 34.53 | test_loadbalance.py
   test_01_verify_libvirt | `Error` | 602.78 | test_deploy_virtio_scsi_vm.py
   test_02_verify_libvirt_after_restart | `Error` | 610.07 | test_deploy_virtio_scsi_vm.py
   test_03_verify_libvirt_attach_disk | `Error` | 605.85 | test_deploy_virtio_scsi_vm.py
   test_04_verify_guest_lspci | `Error` | 602.00 | test_deploy_virtio_scsi_vm.py
   test_05_change_vm_ostype_restart | `Error` | 610.12 | test_deploy_virtio_scsi_vm.py
   test_06_verify_guest_lspci_again | `Error` | 602.01 | test_deploy_virtio_scsi_vm.py
   ContextSuite context=TestAddConfigtoDeployVM>:setup | `Error` | 0.00 | test_deploy_vm_extra_config_data.py
   test_01_port_fwd_on_src_nat | `Failure` | 605.06 | test_network.py
   test_02_port_fwd_on_non_src_nat | `Failure` | 607.95 | test_network.py
   ContextSuite context=TestPrivateVlansL2Networks>:setup | `Error` | 1229.18 | test_network.py
   test_reboot_router | `Failure` | 358.76 | test_network.py
   test_network_rules_acquired_public_ip_1_static_nat_rule | `Error` | 607.45 | test_network.py
   test_network_rules_acquired_public_ip_2_nat_rule | `Error` | 609.00 | test_network.py
   test_network_rules_acquired_public_ip_3_Load_Balancer_Rule | `Error` | 613.02 | test_network.py
   test_isolate_network_password_server | `Failure` | 157.40 | test_password_server.py
   test_02_vpc_privategw_static_routes | `Failure` | 759.00 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 755.42 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Failure` | 846.72 | test_privategw_acl.py
   test_01_disableHumanReadableLogs | `Error` | 602.78 | test_human_readable_logs.py
   test_02_enableHumanReadableLogs | `Error` | 602.89 | test_human_readable_logs.py
   test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | `Failure` | 270.13 | test_internal_lb.py
   test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Failure` | 316.97 | test_internal_lb.py
   test_03_vpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 137.47 | test_internal_lb.py
   test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 170.93 | test_internal_lb.py
   test_01_deploy_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_02_invalid_upgrade_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_04_deploy_and_scale_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_05_delete_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_06_deploy_invalid_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_01_add_delete_kubernetes_supported_version | `Error` | 0.01 | test_kubernetes_supported_versions.py
   test_router_dhcphosts | `Failure` | 156.92 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDHCPHosts>:teardown | `Error` | 167.22 | test_router_dhcphosts.py
   test_router_dhcp_opts | `Error` | 609.17 | test_router_dhcphosts.py
   test_router_dns_guestipquery | `Failure` | 454.70 | test_router_dns.py
   test_router_dns_guestipquery | `Failure` | 454.81 | test_router_dnsservice.py
   test_02_routervm_iptables_policies | `Error` | 660.47 | test_routers_iptables_default_policy.py
   test_01_single_VPC_iptables_policies | `Error` | 724.74 | test_routers_iptables_default_policy.py
   test_01_isolate_network_FW_PF_default_routes_egress_true | `Failure` | 209.59 | test_routers_network_ops.py
   test_02_isolate_network_FW_PF_default_routes_egress_false | `Failure` | 211.04 | test_routers_network_ops.py
   test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Failure` | 246.44 | test_routers_network_ops.py
   test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Failure` | 246.00 | test_routers_network_ops.py
   test_03_RVR_Network_check_router_state | `Error` | 688.98 | test_routers_network_ops.py
   test_01_router_internal_basic | `Error` | 602.74 | test_routers.py
   test_02_router_internal_adv | `Error` | 602.71 | test_routers.py
   test_04_restart_network_wo_cleanup | `Error` | 604.85 | test_routers.py
   test_01_service_offering_cpu_limit_use | `Error` | 100.51 | test_service_offerings.py
   test_04_change_offering_small | `Failure` | 713.52 | test_service_offerings.py
   test_01_snapshot_root_disk | `Error` | 605.85 | test_snapshots.py
   test_03_ssvm_internals | `Error` | 602.78 | test_ssvm.py
   test_04_cpvm_internals | `Error` | 602.85 | test_ssvm.py
   test_05_stop_ssvm | `Error` | 645.20 | test_ssvm.py
   test_06_stop_cpvm | `Error` | 655.34 | test_ssvm.py
   test_07_reboot_ssvm | `Error` | 720.11 | test_ssvm.py
   test_08_reboot_cpvm | `Error` | 627.06 | test_ssvm.py
   test_09_destroy_ssvm | `Error` | 671.36 | test_ssvm.py
   test_10_destroy_cpvm | `Error` | 659.17 | test_ssvm.py
   test_01_migrate_VM_and_root_volume | `Error` | 66.19 | test_vm_life_cycle.py
   test_02_migrate_VM_with_two_data_disks | `Error` | 49.00 | test_vm_life_cycle.py
   test_01_secure_vm_migration | `Error` | 700.17 | test_vm_life_cycle.py
   test_02_unsecure_vm_migration | `Error` | 637.21 | test_vm_life_cycle.py
   test_03_secured_to_nonsecured_vm_migration | `Error` | 637.17 | test_vm_life_cycle.py
   test_04_nonsecured_to_secured_vm_migration | `Error` | 637.14 | test_vm_life_cycle.py
   test_10_attachAndDetach_iso | `Failure` | 607.80 | test_vm_life_cycle.py
   test_01_create_vm_snapshots | `Failure` | 604.48 | test_vm_snapshots.py
   test_02_revert_vm_snapshots | `Failure` | 601.46 | test_vm_snapshots.py
   test_03_delete_vm_snapshots | `Failure` | 0.01 | test_vm_snapshots.py
   test_01_create_volume | `Failure` | 610.81 | test_volumes.py
   test_02_attach_volume | `Failure` | 606.71 | test_volumes.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Error` | 829.17 | test_vpc_redundant.py
   test_02_redundant_VPC_default_routes | `Error` | 830.12 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Error` | 745.29 | test_vpc_redundant.py
   test_04_rvpc_network_garbage_collector_nics | `Error` | 708.01 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Error` | 802.33 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Error` | 828.25 | test_vpc_redundant.py
   test_01_VPC_nics_after_destroy | `Failure` | 721.46 | test_vpc_router_nics.py
   test_02_VPC_default_routes | `Failure` | 726.52 | test_vpc_router_nics.py
   test_01_redundant_vpc_site2site_vpn | `Failure` | 357.48 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn_multiple_options | `Error` | 249.15 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn | `Error` | 287.13 | test_vpc_vpn.py
   test_hostha_enable_ha_when_host_disabled | `Error` | 0.41 | test_hostha_kvm.py
   test_hostha_enable_ha_when_host_disconected | `Error` | 605.08 | test_hostha_kvm.py
   test_hostha_enable_ha_when_host_in_maintenance | `Error` | 301.53 | 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-805575897


   Packaging result: :heavy_multiplication_x: centos7 :heavy_multiplication_x: centos8 :heavy_multiplication_x: debian. SL-JID 228


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-802411702


   <b>[S] Trillian test result (tid-167)</b>
   Environment: vmware-60u2 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 56205 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4717-t167-vmware-60u2.zip
   Intermittent failure detected: /marvin/tests/smoke/test_accounts.py
   Intermittent failure detected: /marvin/tests/smoke/test_affinity_groups_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_async_job.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vgpu_enabled_vm.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_extra_config_data.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vms_with_varied_deploymentplanners.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_with_userdata.py
   Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
   Intermittent failure detected: /marvin/tests/smoke/test_domain_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_human_readable_logs.py
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_supported_versions.py
   Intermittent failure detected: /marvin/tests/smoke/test_list_ids_parameter.py
   Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py
   Intermittent failure detected: /marvin/tests/smoke/test_metrics_api.py
   Intermittent failure detected: /marvin/tests/smoke/test_multipleips_per_nic.py
   Intermittent failure detected: /marvin/tests/smoke/test_nested_virtualization.py
   Intermittent failure detected: /marvin/tests/smoke/test_network_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_network.py
   Intermittent failure detected: /marvin/tests/smoke/test_nic.py
   Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
   Intermittent failure detected: /marvin/tests/smoke/test_portforwardingrules.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
   Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers.py
   Intermittent failure detected: /marvin/tests/smoke/test_scale_vm.py
   Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Smoke tests completed. 43 look OK, 43 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   ContextSuite context=TestAddVmToSubDomain>:setup | `Error` | 90.97 | test_accounts.py
   test_DeleteDomain | `Error` | 69.43 | test_accounts.py
   test_forceDeleteDomain | `Failure` | 86.07 | test_accounts.py
   test_forceDeleteDomain | `Error` | 108.61 | test_accounts.py
   test_01_user_remove_VM_running | `Error` | 95.88 | test_accounts.py
   test_DeployVmAntiAffinityGroup_in_project | `Error` | 99.03 | test_affinity_groups_projects.py
   test_query_async_job_result | `Error` | 76.29 | test_async_job.py
   ContextSuite context=TestLoadBalance>:setup | `Error` | 0.00 | test_loadbalance.py
   test_3d_gpu_support | `Error` | 91.59 | test_deploy_vgpu_enabled_vm.py
   test_list_vms_metrics | `Error` | 73.68 | test_metrics_api.py
   test_05_deploy_vm_with_extraconfig_vmware | `Error` | 699.43 | test_deploy_vm_extra_config_data.py
   test_network_acl | `Error` | 83.03 | test_network_acl.py
   test_deployvm_firstfit | `Error` | 77.88 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userconcentrated | `Error` | 16.63 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userdispersing | `Error` | 15.52 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userdata | `Error` | 70.60 | test_deploy_vm_with_userdata.py
   test_deployvm_userdata_post | `Error` | 16.56 | test_deploy_vm_with_userdata.py
   ContextSuite context=TestRemoteDiagnostics>:setup | `Error` | 0.00 | test_diagnostics.py
   test_delete_account | `Error` | 76.22 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 11.57 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 12.63 | test_network.py
   test_deploy_vm_l2network | `Error` | 9.42 | test_network.py
   test_deploy_vm_l2network | `Error` | 9.46 | test_network.py
   test_l2network_restart | `Error` | 9.49 | test_network.py
   test_l2network_restart | `Error` | 10.55 | test_network.py
   ContextSuite context=TestL2Networks>:teardown | `Error` | 11.63 | test_network.py
   ContextSuite context=TestPortForwarding>:setup | `Error` | 79.48 | test_network.py
   ContextSuite context=TestPublicIP>:setup | `Error` | 64.44 | test_network.py
   test_reboot_router | `Error` | 69.92 | test_network.py
   test_releaseIP | `Error` | 62.97 | test_network.py
   ContextSuite context=TestRouterRules>:setup | `Error` | 133.91 | test_network.py
   test_01_nic | `Error` | 762.33 | test_nic.py
   test_03_deploy_vm_domain_service_offering | `Error` | 83.48 | test_domain_service_offerings.py
   ContextSuite context=TestIsolatedNetworksPasswdServer>:setup | `Error` | 0.00 | test_password_server.py
   test_01_disableHumanReadableLogs | `Error` | 602.64 | test_human_readable_logs.py
   test_02_enableHumanReadableLogs | `Error` | 602.72 | test_human_readable_logs.py
   test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | `Error` | 99.50 | test_internal_lb.py
   test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | `Error` | 101.63 | test_internal_lb.py
   test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Error` | 153.55 | test_internal_lb.py
   test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Error` | 155.69 | test_internal_lb.py
   test_03_vpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 80.01 | test_internal_lb.py
   test_03_vpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 81.08 | test_internal_lb.py
   test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 153.39 | test_internal_lb.py
   test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 154.46 | test_internal_lb.py
   test_01_deploy_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_02_invalid_upgrade_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_04_deploy_and_scale_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_05_delete_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_06_deploy_invalid_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   ContextSuite context=TestProjectSuspendActivate>:setup | `Error` | 1518.51 | test_projects.py
   test_01_add_delete_kubernetes_supported_version | `Error` | 0.01 | test_kubernetes_supported_versions.py
   ContextSuite context=TestListIdsParams>:setup | `Error` | 0.00 | test_list_ids_parameter.py
   test_nic_secondaryip_add_remove | `Error` | 77.09 | test_multipleips_per_nic.py
   test_nested_virtualization_vmware | `Error` | 70.58 | test_nested_virtualization.py
   test_01_create_delete_portforwarding_fornonvpc | `Error` | 70.17 | test_portforwardingrules.py
   test_02_vpc_privategw_static_routes | `Failure` | 146.77 | test_privategw_acl.py
   test_02_vpc_privategw_static_routes | `Error` | 148.93 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 142.75 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Error` | 144.89 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Failure` | 255.87 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Error` | 258.03 | test_privategw_acl.py
   ContextSuite context=TestResetVmOnReboot>:setup | `Error` | 0.00 | test_reset_vm_on_reboot.py
   test_01_so_removal_resource_update | `Error` | 77.74 | test_resource_accounting.py
   ContextSuite context=TestRouterDHCPHosts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDHCPOpts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDns>:setup | `Error` | 0.00 | test_router_dns.py
   ContextSuite context=TestRouterDnsService>:setup | `Error` | 0.00 | test_router_dnsservice.py
   test_02_routervm_iptables_policies | `Error` | 72.59 | test_routers_iptables_default_policy.py
   test_01_single_VPC_iptables_policies | `Error` | 91.17 | test_routers_iptables_default_policy.py
   test_01_single_VPC_iptables_policies | `Error` | 109.63 | test_routers_iptables_default_policy.py
   test_01_isolate_network_FW_PF_default_routes_egress_true | `Error` | 67.65 | test_routers_network_ops.py
   test_02_isolate_network_FW_PF_default_routes_egress_false | `Error` | 72.73 | test_routers_network_ops.py
   test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Error` | 149.49 | test_routers_network_ops.py
   test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Error` | 142.01 | test_routers_network_ops.py
   test_03_RVR_Network_check_router_state | `Error` | 135.87 | test_routers_network_ops.py
   ContextSuite context=TestRouterServices>:setup | `Error` | 0.00 | test_routers.py
   test_01_scale_vm | `Error` | 604.61 | test_scale_vm.py
   ContextSuite context=TestServiceOfferings>:setup | `Error` | 75.13 | test_service_offerings.py
   ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py
   test_03_ssvm_internals | `Error` | 602.69 | test_ssvm.py
   test_04_cpvm_internals | `Error` | 602.72 | test_ssvm.py
   test_05_stop_ssvm | `Error` | 685.73 | test_ssvm.py
   test_06_stop_cpvm | `Error` | 706.95 | test_ssvm.py
   test_07_reboot_ssvm | `Error` | 701.11 | test_ssvm.py
   test_08_reboot_cpvm | `Error` | 610.87 | test_ssvm.py
   test_09_destroy_ssvm | `Error` | 684.48 | test_ssvm.py
   test_10_destroy_cpvm | `Error` | 674.54 | test_ssvm.py
   ContextSuite context=TestVAppsVM>:setup | `Error` | 44.33 | test_vm_life_cycle.py
   test_10_attachAndDetach_iso | `Failure` | 607.54 | test_vm_life_cycle.py
   test_change_service_offering_for_vm_with_snapshots | `Failure` | 705.31 | test_vm_snapshots.py
   test_01_create_vm_snapshots | `Failure` | 604.71 | test_vm_snapshots.py
   test_02_revert_vm_snapshots | `Failure` | 601.54 | test_vm_snapshots.py
   test_03_delete_vm_snapshots | `Failure` | 0.02 | test_vm_snapshots.py
   test_01_create_volume | `Failure` | 614.91 | test_volumes.py
   test_02_attach_volume | `Failure` | 611.73 | test_volumes.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 146.72 | test_vpc_redundant.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Error` | 165.25 | test_vpc_redundant.py
   test_02_redundant_VPC_default_routes | `Failure` | 146.67 | test_vpc_redundant.py
   test_02_redundant_VPC_default_routes | `Error` | 164.18 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 146.68 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Error` | 165.22 | test_vpc_redundant.py
   test_04_rvpc_network_garbage_collector_nics | `Failure` | 142.55 | test_vpc_redundant.py
   test_04_rvpc_network_garbage_collector_nics | `Error` | 157.02 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Failure` | 150.68 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Error` | 167.18 | test_vpc_redundant.py
   test_01_VPC_nics_after_destroy | `Failure` | 80.40 | test_vpc_router_nics.py
   test_02_VPC_default_routes | `Failure` | 77.36 | test_vpc_router_nics.py
   test_01_redundant_vpc_site2site_vpn | `Failure` | 265.25 | test_vpc_vpn.py
   test_01_redundant_vpc_site2site_vpn | `Error` | 267.40 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn_multiple_options | `Failure` | 137.20 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn_multiple_options | `Error` | 139.33 | test_vpc_vpn.py
   test_01_vpc_remote_access_vpn | `Failure` | 78.83 | test_vpc_vpn.py
   test_01_vpc_remote_access_vpn | `Error` | 79.89 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn | `Failure` | 131.10 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn | `Error` | 133.25 | test_vpc_vpn.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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r586328615



##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -210,12 +212,62 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
             //If account_id or account_name is explicitly mentioned, list records for the specified account only even if the caller is of type admin
             if (_accountService.isRootAdmin(caller.getId())) {
                 isAdmin = true;
-            } else if (_accountService.isDomainAdmin(caller.getId())) {
-                isDomainAdmin = true;
             }
             s_logger.debug("Account details not available. Using userContext accountId: " + accountId);
         }
 
+        // Check if a domain admin is allowed to access the requested domain id
+        if (isDomainAdmin) {
+            if (domainId != null) {
+                Account callerAccount = _accountService.getAccount(caller.getId());
+                Domain domain = _domainDao.findById(domainId);
+                _accountService.checkAccess(callerAccount, domain);
+            } else {
+                // Domain admins can only access their own domain's usage records.
+                // Set the domain if not specified.
+                domainId = caller.getDomainId();
+            }
+
+            // Check if a domain admin is allowed to access the requested account info.
+            Account account = _accountService.getAccount(accountId);
+            boolean matchFound = false;
+
+            if (account.getDomainId() == domainId) {
+                matchFound = true;
+            } else {
+
+                // Check if the account is in a child domain of this domain admin.
+                List<DomainVO> childDomains = _domainDao.findAllChildren(_domainDao.findById(domainId).getPath(), domainId);
+
+                for (DomainVO domainVO : childDomains) {
+                    if (account.getDomainId() == domainVO.getId()) {
+                        matchFound = true;
+                        break;
+                    }
+                }
+            }
+            if (!matchFound) {
+                    throw new PermissionDeniedException("Domain admins may only retrieve usage records for accounts in their own domain and child domains.");
+            }
+        }
+
+        // By default users do not have access to this API.
+        // Adding checks here in case someone changes the default access.
+        if (isNormalUser) {
+            // A user can only access their own account records
+            if (caller.getId() != accountId) {

Review comment:
       caller.getAccountId() (in case getAccountId is not same as getId)




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r595746183



##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -210,12 +212,65 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
             //If account_id or account_name is explicitly mentioned, list records for the specified account only even if the caller is of type admin
             if (_accountService.isRootAdmin(caller.getId())) {
                 isAdmin = true;
-            } else if (_accountService.isDomainAdmin(caller.getId())) {
-                isDomainAdmin = true;
             }
             s_logger.debug("Account details not available. Using userContext accountId: " + accountId);
         }
 
+        // Check if a domain admin is allowed to access the requested domain id
+        if (isDomainAdmin) {
+            if (domainId != null) {
+                Account callerAccount = _accountService.getAccount(caller.getId());
+                Domain domain = _domainDao.findById(domainId);
+                _accountService.checkAccess(callerAccount, domain);
+            } else {
+                // Domain admins can only access their own domain's usage records.
+                // Set the domain if not specified.
+                domainId = caller.getDomainId();
+            }
+
+            if (cmd.getAccountId() != null) {
+
+                // Check if a domain admin is allowed to access the requested account info.
+                Account account = _accountService.getAccount(accountId);
+                boolean matchFound = false;
+
+                if (account.getDomainId() == domainId) {
+                    matchFound = true;
+                } else {
+
+                    // Check if the account is in a child domain of this domain admin.
+                    List<DomainVO> childDomains = _domainDao.findAllChildren(_domainDao.findById(domainId).getPath(), domainId);
+
+                    for (DomainVO domainVO : childDomains) {
+                        if (account.getDomainId() == domainVO.getId()) {
+                            matchFound = true;
+                            break;
+                        }
+                    }
+                }
+                if (!matchFound) {
+                    throw new PermissionDeniedException("Domain admins may only retrieve usage records for accounts in their own domain and child domains.");
+                }
+            }
+        }
+

Review comment:
       @Spaceman1984 is it possible to move the domain admin / normal user validations to separate private methods ?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-789569465


   @Spaceman1984 can you test if it works as before if the domain admin can list usage records for an account within the (a) same domain, (b) a sub-domain, and (c) a different domain (the domain admin is not admin of that domain).


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-787689491


   @shwstppr a Jenkins job has been kicked to build packages. 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r586293953



##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -234,22 +286,27 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
 
         SearchCriteria<UsageVO> sc = _usageDao.createSearchCriteria();
 
-        if (accountId != -1 && accountId != Account.ACCOUNT_ID_SYSTEM && !isAdmin && !isDomainAdmin) {
-            sc.addAnd("accountId", SearchCriteria.Op.EQ, accountId);
-        }
-
-        if (isDomainAdmin) {
-            SearchCriteria<DomainVO> sdc = _domainDao.createSearchCriteria();
-            sdc.addOr("path", SearchCriteria.Op.LIKE, _domainDao.findById(caller.getDomainId()).getPath() + "%");
-            List<DomainVO> domains = _domainDao.search(sdc, null);
-            List<Long> domainIds = new ArrayList<Long>();
-            for (DomainVO domain : domains)
-                domainIds.add(domain.getId());
-            sc.addAnd("domainId", SearchCriteria.Op.IN, domainIds.toArray());
+        if (accountId != -1 && accountId != Account.ACCOUNT_ID_SYSTEM && !isAdmin) {

Review comment:
       I don't understand, how do you mean?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-800376245


   Packaging result: :heavy_check_mark: centos7 :heavy_multiplication_x: centos8 :heavy_check_mark: debian. SL-JID 139


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r586297186



##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -234,22 +286,27 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
 
         SearchCriteria<UsageVO> sc = _usageDao.createSearchCriteria();
 
-        if (accountId != -1 && accountId != Account.ACCOUNT_ID_SYSTEM && !isAdmin && !isDomainAdmin) {
-            sc.addAnd("accountId", SearchCriteria.Op.EQ, accountId);
-        }
-
-        if (isDomainAdmin) {
-            SearchCriteria<DomainVO> sdc = _domainDao.createSearchCriteria();
-            sdc.addOr("path", SearchCriteria.Op.LIKE, _domainDao.findById(caller.getDomainId()).getPath() + "%");
-            List<DomainVO> domains = _domainDao.search(sdc, null);
-            List<Long> domainIds = new ArrayList<Long>();
-            for (DomainVO domain : domains)
-                domainIds.add(domain.getId());
-            sc.addAnd("domainId", SearchCriteria.Op.IN, domainIds.toArray());
+        if (accountId != -1 && accountId != Account.ACCOUNT_ID_SYSTEM && !isAdmin) {
+            // account exists and either domain on user role
+            // If not recursive and the account belongs to the user/domain admin, or the account was passed in, filter
+            if ((accountId == caller.getId() && !cmd.isRecursive()) || cmd.getAccountId() != null){
+                sc.addAnd("accountId", SearchCriteria.Op.EQ, accountId);
+            }
         }
 
         if (domainId != null) {
-            sc.addAnd("domainId", SearchCriteria.Op.EQ, domainId);
+            if (cmd.isRecursive()) {
+                SearchCriteria<DomainVO> sdc = _domainDao.createSearchCriteria();
+                sdc.addOr("path", SearchCriteria.Op.LIKE, _domainDao.findById(domainId).getPath() + "%");
+                List<DomainVO> domains = _domainDao.search(sdc, null);

Review comment:
       @rhtyd Using findAllChildren here would not pass the top domain.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-803247477


   <b>[S] Trillian test result (tid-182)</b>
   Environment: vmware-60u2 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 53679 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4717-t182-vmware-60u2.zip
   Intermittent failure detected: /marvin/tests/smoke/test_accounts.py
   Intermittent failure detected: /marvin/tests/smoke/test_affinity_groups_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_async_job.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vgpu_enabled_vm.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_extra_config_data.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vms_with_varied_deploymentplanners.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_with_userdata.py
   Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
   Intermittent failure detected: /marvin/tests/smoke/test_domain_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_human_readable_logs.py
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_supported_versions.py
   Intermittent failure detected: /marvin/tests/smoke/test_list_ids_parameter.py
   Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py
   Intermittent failure detected: /marvin/tests/smoke/test_metrics_api.py
   Intermittent failure detected: /marvin/tests/smoke/test_multipleips_per_nic.py
   Intermittent failure detected: /marvin/tests/smoke/test_nested_virtualization.py
   Intermittent failure detected: /marvin/tests/smoke/test_network_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_network.py
   Intermittent failure detected: /marvin/tests/smoke/test_nic.py
   Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
   Intermittent failure detected: /marvin/tests/smoke/test_portforwardingrules.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
   Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers.py
   Intermittent failure detected: /marvin/tests/smoke/test_scale_vm.py
   Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Smoke tests completed. 43 look OK, 43 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   ContextSuite context=TestAddVmToSubDomain>:setup | `Error` | 88.62 | test_accounts.py
   test_DeleteDomain | `Error` | 70.36 | test_accounts.py
   test_forceDeleteDomain | `Failure` | 72.44 | test_accounts.py
   test_forceDeleteDomain | `Error` | 88.83 | test_accounts.py
   test_01_user_remove_VM_running | `Error` | 70.28 | test_accounts.py
   test_DeployVmAntiAffinityGroup_in_project | `Error` | 76.76 | test_affinity_groups_projects.py
   test_query_async_job_result | `Error` | 93.93 | test_async_job.py
   ContextSuite context=TestLoadBalance>:setup | `Error` | 0.00 | test_loadbalance.py
   test_3d_gpu_support | `Error` | 105.85 | test_deploy_vgpu_enabled_vm.py
   test_list_vms_metrics | `Error` | 70.79 | test_metrics_api.py
   test_05_deploy_vm_with_extraconfig_vmware | `Error` | 701.58 | test_deploy_vm_extra_config_data.py
   test_network_acl | `Error` | 78.03 | test_network_acl.py
   test_deployvm_firstfit | `Error` | 70.57 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userconcentrated | `Error` | 16.56 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userdispersing | `Error` | 15.58 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userdata | `Error` | 80.56 | test_deploy_vm_with_userdata.py
   test_deployvm_userdata_post | `Error` | 27.71 | test_deploy_vm_with_userdata.py
   ContextSuite context=TestRemoteDiagnostics>:setup | `Error` | 0.00 | test_diagnostics.py
   test_delete_account | `Error` | 94.46 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 11.45 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 12.52 | test_network.py
   test_deploy_vm_l2network | `Error` | 9.40 | test_network.py
   test_deploy_vm_l2network | `Error` | 10.47 | test_network.py
   test_l2network_restart | `Error` | 10.58 | test_network.py
   test_l2network_restart | `Error` | 11.66 | test_network.py
   ContextSuite context=TestL2Networks>:teardown | `Error` | 12.76 | test_network.py
   ContextSuite context=TestPortForwarding>:setup | `Error` | 80.74 | test_network.py
   ContextSuite context=TestPublicIP>:setup | `Error` | 64.59 | test_network.py
   test_reboot_router | `Error` | 71.03 | test_network.py
   test_releaseIP | `Error` | 71.00 | test_network.py
   ContextSuite context=TestRouterRules>:setup | `Error` | 140.10 | test_network.py
   test_01_nic | `Error` | 752.64 | test_nic.py
   test_03_deploy_vm_domain_service_offering | `Error` | 72.34 | test_domain_service_offerings.py
   ContextSuite context=TestIsolatedNetworksPasswdServer>:setup | `Error` | 0.00 | test_password_server.py
   test_01_disableHumanReadableLogs | `Error` | 602.69 | test_human_readable_logs.py
   test_02_enableHumanReadableLogs | `Error` | 602.65 | test_human_readable_logs.py
   test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | `Error` | 83.40 | test_internal_lb.py
   test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | `Error` | 85.54 | test_internal_lb.py
   test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Error` | 156.05 | test_internal_lb.py
   test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Error` | 158.19 | test_internal_lb.py
   test_03_vpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 75.13 | test_internal_lb.py
   test_03_vpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 76.21 | test_internal_lb.py
   test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 151.51 | test_internal_lb.py
   test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 152.58 | test_internal_lb.py
   test_01_deploy_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_02_invalid_upgrade_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_04_deploy_and_scale_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_05_delete_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_06_deploy_invalid_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_09_project_suspend | `Error` | 71.75 | test_projects.py
   test_10_project_activation | `Error` | 17.66 | test_projects.py
   test_01_add_delete_kubernetes_supported_version | `Error` | 0.01 | test_kubernetes_supported_versions.py
   ContextSuite context=TestListIdsParams>:setup | `Error` | 0.00 | test_list_ids_parameter.py
   test_nic_secondaryip_add_remove | `Error` | 72.10 | test_multipleips_per_nic.py
   test_nested_virtualization_vmware | `Error` | 70.80 | test_nested_virtualization.py
   test_01_create_delete_portforwarding_fornonvpc | `Error` | 69.33 | test_portforwardingrules.py
   test_02_vpc_privategw_static_routes | `Failure` | 136.85 | test_privategw_acl.py
   test_02_vpc_privategw_static_routes | `Error` | 139.04 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 148.25 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Error` | 150.42 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Failure` | 263.38 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Error` | 265.58 | test_privategw_acl.py
   ContextSuite context=TestResetVmOnReboot>:setup | `Error` | 0.00 | test_reset_vm_on_reboot.py
   test_01_so_removal_resource_update | `Error` | 71.72 | test_resource_accounting.py
   ContextSuite context=TestRouterDHCPHosts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDHCPOpts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDns>:setup | `Error` | 0.00 | test_router_dns.py
   ContextSuite context=TestRouterDnsService>:setup | `Error` | 0.00 | test_router_dnsservice.py
   test_02_routervm_iptables_policies | `Error` | 76.73 | test_routers_iptables_default_policy.py
   test_01_single_VPC_iptables_policies | `Error` | 90.26 | test_routers_iptables_default_policy.py
   test_01_single_VPC_iptables_policies | `Error` | 109.78 | test_routers_iptables_default_policy.py
   test_01_isolate_network_FW_PF_default_routes_egress_true | `Error` | 64.68 | test_routers_network_ops.py
   test_02_isolate_network_FW_PF_default_routes_egress_false | `Error` | 80.90 | test_routers_network_ops.py
   test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Error` | 144.32 | test_routers_network_ops.py
   test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Error` | 148.34 | test_routers_network_ops.py
   test_03_RVR_Network_check_router_state | `Error` | 150.94 | test_routers_network_ops.py
   ContextSuite context=TestRouterServices>:setup | `Error` | 0.00 | test_routers.py
   test_01_scale_vm | `Error` | 604.68 | test_scale_vm.py
   ContextSuite context=TestServiceOfferings>:setup | `Error` | 79.31 | test_service_offerings.py
   ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py
   test_03_ssvm_internals | `Error` | 602.90 | test_ssvm.py
   test_04_cpvm_internals | `Error` | 602.87 | test_ssvm.py
   test_05_stop_ssvm | `Error` | 687.05 | test_ssvm.py
   test_06_stop_cpvm | `Error` | 708.26 | test_ssvm.py
   test_07_reboot_ssvm | `Error` | 702.15 | test_ssvm.py
   test_08_reboot_cpvm | `Error` | 611.20 | test_ssvm.py
   test_09_destroy_ssvm | `Error` | 682.57 | test_ssvm.py
   test_10_destroy_cpvm | `Error` | 675.68 | test_ssvm.py
   ContextSuite context=TestVAppsVM>:setup | `Error` | 44.25 | test_vm_life_cycle.py
   test_10_attachAndDetach_iso | `Failure` | 607.75 | test_vm_life_cycle.py
   test_change_service_offering_for_vm_with_snapshots | `Failure` | 710.59 | test_vm_snapshots.py
   test_01_create_vm_snapshots | `Failure` | 604.82 | test_vm_snapshots.py
   test_02_revert_vm_snapshots | `Failure` | 601.70 | test_vm_snapshots.py
   test_03_delete_vm_snapshots | `Failure` | 0.02 | test_vm_snapshots.py
   test_01_create_volume | `Failure` | 615.08 | test_volumes.py
   test_02_attach_volume | `Failure` | 612.84 | test_volumes.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 151.94 | test_vpc_redundant.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Error` | 167.46 | test_vpc_redundant.py
   test_02_redundant_VPC_default_routes | `Failure` | 148.81 | test_vpc_redundant.py
   test_02_redundant_VPC_default_routes | `Error` | 166.41 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 144.98 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Error` | 162.57 | test_vpc_redundant.py
   test_04_rvpc_network_garbage_collector_nics | `Failure` | 149.88 | test_vpc_redundant.py
   test_04_rvpc_network_garbage_collector_nics | `Error` | 167.44 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Failure` | 153.06 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Error` | 168.56 | test_vpc_redundant.py
   test_01_VPC_nics_after_destroy | `Failure` | 82.58 | test_vpc_router_nics.py
   test_02_VPC_default_routes | `Failure` | 77.52 | test_vpc_router_nics.py
   test_01_redundant_vpc_site2site_vpn | `Failure` | 270.05 | test_vpc_vpn.py
   test_01_redundant_vpc_site2site_vpn | `Error` | 272.24 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn_multiple_options | `Failure` | 131.32 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn_multiple_options | `Error` | 133.48 | test_vpc_vpn.py
   test_01_vpc_remote_access_vpn | `Failure` | 77.92 | test_vpc_vpn.py
   test_01_vpc_remote_access_vpn | `Error` | 78.99 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn | `Failure` | 133.21 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn | `Error` | 135.36 | test_vpc_vpn.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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-805569905


   @rhtyd a Jenkins job has been kicked to build packages. 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-794622939


   <b>Trillian test result (tid-3681)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 31927 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4717-t3681-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Smoke tests completed. 85 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_migrate_VM_and_root_volume | `Error` | 69.23 | test_vm_life_cycle.py
   test_02_migrate_VM_with_two_data_disks | `Error` | 48.03 | test_vm_life_cycle.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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-803345361


   <b>[S] Trillian test result (tid-179)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 101325 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4717-t179-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_virtio_scsi_vm.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_extra_config_data.py
   Intermittent failure detected: /marvin/tests/smoke/test_human_readable_logs.py
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_supported_versions.py
   Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py
   Intermittent failure detected: /marvin/tests/smoke/test_network.py
   Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers.py
   Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Intermittent failure detected: /marvin/tests/smoke/test_host_maintenance.py
   Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
   Smoke tests completed. 60 look OK, 26 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_create_lb_rule_src_nat | `Failure` | 308.28 | test_loadbalance.py
   test_02_create_lb_rule_non_nat | `Failure` | 34.55 | test_loadbalance.py
   test_assign_and_removal_lb | `Failure` | 34.53 | test_loadbalance.py
   test_01_verify_libvirt | `Error` | 602.78 | test_deploy_virtio_scsi_vm.py
   test_02_verify_libvirt_after_restart | `Error` | 610.07 | test_deploy_virtio_scsi_vm.py
   test_03_verify_libvirt_attach_disk | `Error` | 605.85 | test_deploy_virtio_scsi_vm.py
   test_04_verify_guest_lspci | `Error` | 602.00 | test_deploy_virtio_scsi_vm.py
   test_05_change_vm_ostype_restart | `Error` | 610.12 | test_deploy_virtio_scsi_vm.py
   test_06_verify_guest_lspci_again | `Error` | 602.01 | test_deploy_virtio_scsi_vm.py
   ContextSuite context=TestAddConfigtoDeployVM>:setup | `Error` | 0.00 | test_deploy_vm_extra_config_data.py
   test_01_port_fwd_on_src_nat | `Failure` | 605.06 | test_network.py
   test_02_port_fwd_on_non_src_nat | `Failure` | 607.95 | test_network.py
   ContextSuite context=TestPrivateVlansL2Networks>:setup | `Error` | 1229.18 | test_network.py
   test_reboot_router | `Failure` | 358.76 | test_network.py
   test_network_rules_acquired_public_ip_1_static_nat_rule | `Error` | 607.45 | test_network.py
   test_network_rules_acquired_public_ip_2_nat_rule | `Error` | 609.00 | test_network.py
   test_network_rules_acquired_public_ip_3_Load_Balancer_Rule | `Error` | 613.02 | test_network.py
   test_isolate_network_password_server | `Failure` | 157.40 | test_password_server.py
   test_02_vpc_privategw_static_routes | `Failure` | 759.00 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 755.42 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Failure` | 846.72 | test_privategw_acl.py
   test_01_disableHumanReadableLogs | `Error` | 602.78 | test_human_readable_logs.py
   test_02_enableHumanReadableLogs | `Error` | 602.89 | test_human_readable_logs.py
   test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | `Failure` | 270.13 | test_internal_lb.py
   test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Failure` | 316.97 | test_internal_lb.py
   test_03_vpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 137.47 | test_internal_lb.py
   test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 170.93 | test_internal_lb.py
   test_01_deploy_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_02_invalid_upgrade_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_04_deploy_and_scale_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_05_delete_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_06_deploy_invalid_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_01_add_delete_kubernetes_supported_version | `Error` | 0.01 | test_kubernetes_supported_versions.py
   test_router_dhcphosts | `Failure` | 156.92 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDHCPHosts>:teardown | `Error` | 167.22 | test_router_dhcphosts.py
   test_router_dhcp_opts | `Error` | 609.17 | test_router_dhcphosts.py
   test_router_dns_guestipquery | `Failure` | 454.70 | test_router_dns.py
   test_router_dns_guestipquery | `Failure` | 454.81 | test_router_dnsservice.py
   test_02_routervm_iptables_policies | `Error` | 660.47 | test_routers_iptables_default_policy.py
   test_01_single_VPC_iptables_policies | `Error` | 724.74 | test_routers_iptables_default_policy.py
   test_01_isolate_network_FW_PF_default_routes_egress_true | `Failure` | 209.59 | test_routers_network_ops.py
   test_02_isolate_network_FW_PF_default_routes_egress_false | `Failure` | 211.04 | test_routers_network_ops.py
   test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Failure` | 246.44 | test_routers_network_ops.py
   test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Failure` | 246.00 | test_routers_network_ops.py
   test_03_RVR_Network_check_router_state | `Error` | 688.98 | test_routers_network_ops.py
   test_01_router_internal_basic | `Error` | 602.74 | test_routers.py
   test_02_router_internal_adv | `Error` | 602.71 | test_routers.py
   test_04_restart_network_wo_cleanup | `Error` | 604.85 | test_routers.py
   test_01_service_offering_cpu_limit_use | `Error` | 100.51 | test_service_offerings.py
   test_04_change_offering_small | `Failure` | 713.52 | test_service_offerings.py
   test_01_snapshot_root_disk | `Error` | 605.85 | test_snapshots.py
   test_03_ssvm_internals | `Error` | 602.78 | test_ssvm.py
   test_04_cpvm_internals | `Error` | 602.85 | test_ssvm.py
   test_05_stop_ssvm | `Error` | 645.20 | test_ssvm.py
   test_06_stop_cpvm | `Error` | 655.34 | test_ssvm.py
   test_07_reboot_ssvm | `Error` | 720.11 | test_ssvm.py
   test_08_reboot_cpvm | `Error` | 627.06 | test_ssvm.py
   test_09_destroy_ssvm | `Error` | 671.36 | test_ssvm.py
   test_10_destroy_cpvm | `Error` | 659.17 | test_ssvm.py
   test_01_migrate_VM_and_root_volume | `Error` | 66.19 | test_vm_life_cycle.py
   test_02_migrate_VM_with_two_data_disks | `Error` | 49.00 | test_vm_life_cycle.py
   test_01_secure_vm_migration | `Error` | 700.17 | test_vm_life_cycle.py
   test_02_unsecure_vm_migration | `Error` | 637.21 | test_vm_life_cycle.py
   test_03_secured_to_nonsecured_vm_migration | `Error` | 637.17 | test_vm_life_cycle.py
   test_04_nonsecured_to_secured_vm_migration | `Error` | 637.14 | test_vm_life_cycle.py
   test_10_attachAndDetach_iso | `Failure` | 607.80 | test_vm_life_cycle.py
   test_01_create_vm_snapshots | `Failure` | 604.48 | test_vm_snapshots.py
   test_02_revert_vm_snapshots | `Failure` | 601.46 | test_vm_snapshots.py
   test_03_delete_vm_snapshots | `Failure` | 0.01 | test_vm_snapshots.py
   test_01_create_volume | `Failure` | 610.81 | test_volumes.py
   test_02_attach_volume | `Failure` | 606.71 | test_volumes.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Error` | 829.17 | test_vpc_redundant.py
   test_02_redundant_VPC_default_routes | `Error` | 830.12 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Error` | 745.29 | test_vpc_redundant.py
   test_04_rvpc_network_garbage_collector_nics | `Error` | 708.01 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Error` | 802.33 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Error` | 828.25 | test_vpc_redundant.py
   test_01_VPC_nics_after_destroy | `Failure` | 721.46 | test_vpc_router_nics.py
   test_02_VPC_default_routes | `Failure` | 726.52 | test_vpc_router_nics.py
   test_01_redundant_vpc_site2site_vpn | `Failure` | 357.48 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn_multiple_options | `Error` | 249.15 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn | `Error` | 287.13 | test_vpc_vpn.py
   test_hostha_enable_ha_when_host_disabled | `Error` | 0.41 | test_hostha_kvm.py
   test_hostha_enable_ha_when_host_disconected | `Error` | 605.08 | test_hostha_kvm.py
   test_hostha_enable_ha_when_host_in_maintenance | `Error` | 301.53 | 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r597546783



##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -200,22 +201,41 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
             }
         }
 
-        boolean isAdmin = false;
-        boolean isDomainAdmin = false;
+        boolean ignoreAccountId = false;
+        boolean isDomainAdmin = _accountService.isDomainAdmin(caller.getId());
+        boolean isNormalUser = _accountService.isNormalUser(caller.getId());
 
         //If accountId couldn't be found using accountName and domainId, get it from userContext
         if (accountId == null) {
             accountId = caller.getId();
             //List records for all the accounts if the caller account is of type admin.
             //If account_id or account_name is explicitly mentioned, list records for the specified account only even if the caller is of type admin
-            if (_accountService.isRootAdmin(caller.getId())) {
-                isAdmin = true;
-            } else if (_accountService.isDomainAdmin(caller.getId())) {
-                isDomainAdmin = true;
-            }
+            ignoreAccountId = _accountService.isRootAdmin(caller.getId());
             s_logger.debug("Account details not available. Using userContext accountId: " + accountId);
         }
 
+        // Check if a domain admin is allowed to access the requested domain id
+        if (isDomainAdmin) {
+            if (domainId != null) {
+                Account callerAccount = _accountService.getAccount(caller.getId());
+                Domain domain = _domainDao.findById(domainId);
+                _accountService.checkAccess(callerAccount, domain);
+            } else {
+                // Domain admins can only access their own domain's usage records.
+                // Set the domain if not specified.
+                domainId = caller.getDomainId();
+            }
+
+            if (cmd.getAccountId() != null) {
+                // Check if a domain admin is allowed to access the requested account info.
+                checkDomainAdminAccountAccess(accountId, domainId);
+            }
+        }
+
+        // By default users do not have access to this API.
+        // Adding checks here in case someone changes the default access.
+        checkUserAccess(cmd, accountId, caller, isNormalUser);

Review comment:
       @Spaceman1984 is this method call required for domain admin ?




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-783156453


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


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-787976257


   @Spaceman1984 a 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-801019157


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


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r580036253



##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -248,7 +248,17 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
             sc.addAnd("domainId", SearchCriteria.Op.IN, domainIds.toArray());
         }
 
-        if (domainId != null) {
+        if (cmd.isRecursive() && domainId != null){

Review comment:
       Look at for reference https://github.com/apache/cloudstack/blob/master/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java#L872
   
   Expected test if an account is allowed or disallowed to get usage records of other domains (with/without recursive flag), and allowed to recursively get all usage records for a permitted account/domain.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-793966231


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


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r581712538



##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -216,6 +216,31 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
             s_logger.debug("Account details not available. Using userContext accountId: " + accountId);
         }
 
+        // Check if a domain admin is allowed to access the requested account info.
+        if (_accountService.isDomainAdmin(caller.getId()) && accountId != null){

Review comment:
       or normal user account?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 removed a comment on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 removed a comment on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-800346639


   @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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-788394775


   <b>Trillian test result (tid-3628)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 32087 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4717-t3628-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Smoke tests completed. 85 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_migrate_VM_and_root_volume | `Error` | 69.23 | test_vm_life_cycle.py
   test_02_migrate_VM_with_two_data_disks | `Error` | 49.12 | test_vm_life_cycle.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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-802411702


   <b>[S] Trillian test result (tid-167)</b>
   Environment: vmware-60u2 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 56205 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4717-t167-vmware-60u2.zip
   Intermittent failure detected: /marvin/tests/smoke/test_accounts.py
   Intermittent failure detected: /marvin/tests/smoke/test_affinity_groups_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_async_job.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vgpu_enabled_vm.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_extra_config_data.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vms_with_varied_deploymentplanners.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_with_userdata.py
   Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
   Intermittent failure detected: /marvin/tests/smoke/test_domain_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_human_readable_logs.py
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_supported_versions.py
   Intermittent failure detected: /marvin/tests/smoke/test_list_ids_parameter.py
   Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py
   Intermittent failure detected: /marvin/tests/smoke/test_metrics_api.py
   Intermittent failure detected: /marvin/tests/smoke/test_multipleips_per_nic.py
   Intermittent failure detected: /marvin/tests/smoke/test_nested_virtualization.py
   Intermittent failure detected: /marvin/tests/smoke/test_network_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_network.py
   Intermittent failure detected: /marvin/tests/smoke/test_nic.py
   Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
   Intermittent failure detected: /marvin/tests/smoke/test_portforwardingrules.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
   Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers.py
   Intermittent failure detected: /marvin/tests/smoke/test_scale_vm.py
   Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Smoke tests completed. 43 look OK, 43 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   ContextSuite context=TestAddVmToSubDomain>:setup | `Error` | 90.97 | test_accounts.py
   test_DeleteDomain | `Error` | 69.43 | test_accounts.py
   test_forceDeleteDomain | `Failure` | 86.07 | test_accounts.py
   test_forceDeleteDomain | `Error` | 108.61 | test_accounts.py
   test_01_user_remove_VM_running | `Error` | 95.88 | test_accounts.py
   test_DeployVmAntiAffinityGroup_in_project | `Error` | 99.03 | test_affinity_groups_projects.py
   test_query_async_job_result | `Error` | 76.29 | test_async_job.py
   ContextSuite context=TestLoadBalance>:setup | `Error` | 0.00 | test_loadbalance.py
   test_3d_gpu_support | `Error` | 91.59 | test_deploy_vgpu_enabled_vm.py
   test_list_vms_metrics | `Error` | 73.68 | test_metrics_api.py
   test_05_deploy_vm_with_extraconfig_vmware | `Error` | 699.43 | test_deploy_vm_extra_config_data.py
   test_network_acl | `Error` | 83.03 | test_network_acl.py
   test_deployvm_firstfit | `Error` | 77.88 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userconcentrated | `Error` | 16.63 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userdispersing | `Error` | 15.52 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userdata | `Error` | 70.60 | test_deploy_vm_with_userdata.py
   test_deployvm_userdata_post | `Error` | 16.56 | test_deploy_vm_with_userdata.py
   ContextSuite context=TestRemoteDiagnostics>:setup | `Error` | 0.00 | test_diagnostics.py
   test_delete_account | `Error` | 76.22 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 11.57 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 12.63 | test_network.py
   test_deploy_vm_l2network | `Error` | 9.42 | test_network.py
   test_deploy_vm_l2network | `Error` | 9.46 | test_network.py
   test_l2network_restart | `Error` | 9.49 | test_network.py
   test_l2network_restart | `Error` | 10.55 | test_network.py
   ContextSuite context=TestL2Networks>:teardown | `Error` | 11.63 | test_network.py
   ContextSuite context=TestPortForwarding>:setup | `Error` | 79.48 | test_network.py
   ContextSuite context=TestPublicIP>:setup | `Error` | 64.44 | test_network.py
   test_reboot_router | `Error` | 69.92 | test_network.py
   test_releaseIP | `Error` | 62.97 | test_network.py
   ContextSuite context=TestRouterRules>:setup | `Error` | 133.91 | test_network.py
   test_01_nic | `Error` | 762.33 | test_nic.py
   test_03_deploy_vm_domain_service_offering | `Error` | 83.48 | test_domain_service_offerings.py
   ContextSuite context=TestIsolatedNetworksPasswdServer>:setup | `Error` | 0.00 | test_password_server.py
   test_01_disableHumanReadableLogs | `Error` | 602.64 | test_human_readable_logs.py
   test_02_enableHumanReadableLogs | `Error` | 602.72 | test_human_readable_logs.py
   test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | `Error` | 99.50 | test_internal_lb.py
   test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | `Error` | 101.63 | test_internal_lb.py
   test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Error` | 153.55 | test_internal_lb.py
   test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Error` | 155.69 | test_internal_lb.py
   test_03_vpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 80.01 | test_internal_lb.py
   test_03_vpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 81.08 | test_internal_lb.py
   test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 153.39 | test_internal_lb.py
   test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 154.46 | test_internal_lb.py
   test_01_deploy_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_02_invalid_upgrade_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_04_deploy_and_scale_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_05_delete_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_06_deploy_invalid_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   ContextSuite context=TestProjectSuspendActivate>:setup | `Error` | 1518.51 | test_projects.py
   test_01_add_delete_kubernetes_supported_version | `Error` | 0.01 | test_kubernetes_supported_versions.py
   ContextSuite context=TestListIdsParams>:setup | `Error` | 0.00 | test_list_ids_parameter.py
   test_nic_secondaryip_add_remove | `Error` | 77.09 | test_multipleips_per_nic.py
   test_nested_virtualization_vmware | `Error` | 70.58 | test_nested_virtualization.py
   test_01_create_delete_portforwarding_fornonvpc | `Error` | 70.17 | test_portforwardingrules.py
   test_02_vpc_privategw_static_routes | `Failure` | 146.77 | test_privategw_acl.py
   test_02_vpc_privategw_static_routes | `Error` | 148.93 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 142.75 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Error` | 144.89 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Failure` | 255.87 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Error` | 258.03 | test_privategw_acl.py
   ContextSuite context=TestResetVmOnReboot>:setup | `Error` | 0.00 | test_reset_vm_on_reboot.py
   test_01_so_removal_resource_update | `Error` | 77.74 | test_resource_accounting.py
   ContextSuite context=TestRouterDHCPHosts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDHCPOpts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDns>:setup | `Error` | 0.00 | test_router_dns.py
   ContextSuite context=TestRouterDnsService>:setup | `Error` | 0.00 | test_router_dnsservice.py
   test_02_routervm_iptables_policies | `Error` | 72.59 | test_routers_iptables_default_policy.py
   test_01_single_VPC_iptables_policies | `Error` | 91.17 | test_routers_iptables_default_policy.py
   test_01_single_VPC_iptables_policies | `Error` | 109.63 | test_routers_iptables_default_policy.py
   test_01_isolate_network_FW_PF_default_routes_egress_true | `Error` | 67.65 | test_routers_network_ops.py
   test_02_isolate_network_FW_PF_default_routes_egress_false | `Error` | 72.73 | test_routers_network_ops.py
   test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Error` | 149.49 | test_routers_network_ops.py
   test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Error` | 142.01 | test_routers_network_ops.py
   test_03_RVR_Network_check_router_state | `Error` | 135.87 | test_routers_network_ops.py
   ContextSuite context=TestRouterServices>:setup | `Error` | 0.00 | test_routers.py
   test_01_scale_vm | `Error` | 604.61 | test_scale_vm.py
   ContextSuite context=TestServiceOfferings>:setup | `Error` | 75.13 | test_service_offerings.py
   ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py
   test_03_ssvm_internals | `Error` | 602.69 | test_ssvm.py
   test_04_cpvm_internals | `Error` | 602.72 | test_ssvm.py
   test_05_stop_ssvm | `Error` | 685.73 | test_ssvm.py
   test_06_stop_cpvm | `Error` | 706.95 | test_ssvm.py
   test_07_reboot_ssvm | `Error` | 701.11 | test_ssvm.py
   test_08_reboot_cpvm | `Error` | 610.87 | test_ssvm.py
   test_09_destroy_ssvm | `Error` | 684.48 | test_ssvm.py
   test_10_destroy_cpvm | `Error` | 674.54 | test_ssvm.py
   ContextSuite context=TestVAppsVM>:setup | `Error` | 44.33 | test_vm_life_cycle.py
   test_10_attachAndDetach_iso | `Failure` | 607.54 | test_vm_life_cycle.py
   test_change_service_offering_for_vm_with_snapshots | `Failure` | 705.31 | test_vm_snapshots.py
   test_01_create_vm_snapshots | `Failure` | 604.71 | test_vm_snapshots.py
   test_02_revert_vm_snapshots | `Failure` | 601.54 | test_vm_snapshots.py
   test_03_delete_vm_snapshots | `Failure` | 0.02 | test_vm_snapshots.py
   test_01_create_volume | `Failure` | 614.91 | test_volumes.py
   test_02_attach_volume | `Failure` | 611.73 | test_volumes.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 146.72 | test_vpc_redundant.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Error` | 165.25 | test_vpc_redundant.py
   test_02_redundant_VPC_default_routes | `Failure` | 146.67 | test_vpc_redundant.py
   test_02_redundant_VPC_default_routes | `Error` | 164.18 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 146.68 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Error` | 165.22 | test_vpc_redundant.py
   test_04_rvpc_network_garbage_collector_nics | `Failure` | 142.55 | test_vpc_redundant.py
   test_04_rvpc_network_garbage_collector_nics | `Error` | 157.02 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Failure` | 150.68 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Error` | 167.18 | test_vpc_redundant.py
   test_01_VPC_nics_after_destroy | `Failure` | 80.40 | test_vpc_router_nics.py
   test_02_VPC_default_routes | `Failure` | 77.36 | test_vpc_router_nics.py
   test_01_redundant_vpc_site2site_vpn | `Failure` | 265.25 | test_vpc_vpn.py
   test_01_redundant_vpc_site2site_vpn | `Error` | 267.40 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn_multiple_options | `Failure` | 137.20 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn_multiple_options | `Error` | 139.33 | test_vpc_vpn.py
   test_01_vpc_remote_access_vpn | `Failure` | 78.83 | test_vpc_vpn.py
   test_01_vpc_remote_access_vpn | `Error` | 79.89 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn | `Failure` | 131.10 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn | `Error` | 133.25 | test_vpc_vpn.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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-801060655


   @DaanHoogland unsupported parameters provided. Supported mgmt server os are: `centos6, centos7, centos8, ubuntu`. Supported hypervisors are: `kvm-centos6, kvm-centos7, kvm-centos8, kvm-ubuntu, xenserver-71, xenserver-65sp1, vmware-67u3, vmware-65u2, vmware-60u2, vmware-55u3, xcpng76, xcpng80, xcpng81, xenserver-74, xcpng74`


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-794622939






----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-816483933


   @andrijapanicsb are you lgtm on 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-783156453






----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-790408046


   > Why not create a `sb` @Spaceman1984 and then use it to create a `sc`?
   
   That would require a rewrite of the 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r581711776



##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -216,6 +216,31 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
             s_logger.debug("Account details not available. Using userContext accountId: " + accountId);
         }
 
+        // Check if a domain admin is allowed to access the requested account info.
+        if (_accountService.isDomainAdmin(caller.getId()) && accountId != null){

Review comment:
       @Spaceman1984 What is the caller account is root admin?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r586254479



##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -210,12 +212,62 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
             //If account_id or account_name is explicitly mentioned, list records for the specified account only even if the caller is of type admin
             if (_accountService.isRootAdmin(caller.getId())) {
                 isAdmin = true;
-            } else if (_accountService.isDomainAdmin(caller.getId())) {
-                isDomainAdmin = true;
             }
             s_logger.debug("Account details not available. Using userContext accountId: " + accountId);
         }
 
+        // Check if a domain admin is allowed to access the requested domain id
+        if (isDomainAdmin) {
+            if (domainId != null) {
+                Account callerAccount = _accountService.getAccount(caller.getId());
+                Domain domain = _domainDao.findById(domainId);
+                _accountService.checkAccess(callerAccount, domain);
+            } else {
+                // Domain admins can only access their own domain's usage records.
+                // Set the domain if not specified.
+                domainId = caller.getDomainId();
+            }
+
+            // Check if a domain admin is allowed to access the requested account info.
+            Account account = _accountService.getAccount(accountId);

Review comment:
       @Spaceman1984 should we do a checkAccess() here as well with the provided caller and account?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-805746354


   @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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-783416729


   @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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r584491107



##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -210,12 +212,45 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
             //If account_id or account_name is explicitly mentioned, list records for the specified account only even if the caller is of type admin
             if (_accountService.isRootAdmin(caller.getId())) {
                 isAdmin = true;
-            } else if (_accountService.isDomainAdmin(caller.getId())) {
-                isDomainAdmin = true;
             }
             s_logger.debug("Account details not available. Using userContext accountId: " + accountId);
         }
 
+        // Check if a domain admin is allowed to access the requested domain id
+        if (isDomainAdmin) {
+            if (domainId != null) {
+                Account callerAccount = _accountService.getAccount(caller.getId());
+                Domain domain = _domainDao.findById(domainId);
+                _accountService.checkAccess(callerAccount, domain);
+            } else {
+                // Domain admins can only access their own domain's usage records.
+                // Set the domain if not specified.
+                domainId = caller.getDomainId();
+            }
+
+            // Check if a domain admin is allowed to access the requested account info.
+            Account account = _accountService.getAccount(accountId);
+            Domain domain = _domainDao.findById(caller.getDomainId());
+            _accountService.checkAccess(account, domain);

Review comment:
       This is checking if the requested account exists in the same domain and sub domains as the caller. I don't see anything wrong 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-806213300


   <b>Trillian test result (tid-247)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 36028 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4717-t247-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Smoke tests completed. 84 look OK, 2 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 72.98 | test_kubernetes_clusters.py
   test_01_migrate_VM_and_root_volume | `Error` | 67.12 | test_vm_life_cycle.py
   test_02_migrate_VM_with_two_data_disks | `Error` | 51.10 | test_vm_life_cycle.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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-801619693


   <b>Trillian test result (tid-3686)</b>
   Environment: vmware-60u2 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 37193 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4717-t3686-vmware-60u2.zip
   Intermittent failure detected: /marvin/tests/smoke/test_accounts.py
   Intermittent failure detected: /marvin/tests/smoke/test_affinity_groups_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_async_job.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vgpu_enabled_vm.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vms_with_varied_deploymentplanners.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_with_userdata.py
   Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
   Intermittent failure detected: /marvin/tests/smoke/test_domain_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_list_ids_parameter.py
   Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py
   Intermittent failure detected: /marvin/tests/smoke/test_metrics_api.py
   Intermittent failure detected: /marvin/tests/smoke/test_multipleips_per_nic.py
   Intermittent failure detected: /marvin/tests/smoke/test_nested_virtualization.py
   Intermittent failure detected: /marvin/tests/smoke/test_network_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_network.py
   Intermittent failure detected: /marvin/tests/smoke/test_nic.py
   Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
   Intermittent failure detected: /marvin/tests/smoke/test_portforwardingrules.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
   Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers.py
   Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Smoke tests completed. 50 look OK, 36 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   ContextSuite context=TestAddVmToSubDomain>:setup | `Error` | 104.41 | test_accounts.py
   test_DeleteDomain | `Error` | 107.05 | test_accounts.py
   test_forceDeleteDomain | `Failure` | 107.90 | test_accounts.py
   test_forceDeleteDomain | `Error` | 128.36 | test_accounts.py
   test_01_user_remove_VM_running | `Error` | 123.14 | test_accounts.py
   test_DeployVmAntiAffinityGroup_in_project | `Error` | 116.46 | test_affinity_groups_projects.py
   test_query_async_job_result | `Error` | 118.72 | test_async_job.py
   ContextSuite context=TestLoadBalance>:setup | `Error` | 0.00 | test_loadbalance.py
   test_3d_gpu_support | `Error` | 125.02 | test_deploy_vgpu_enabled_vm.py
   test_list_vms_metrics | `Error` | 104.42 | test_metrics_api.py
   test_deployvm_firstfit | `Error` | 94.13 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userconcentrated | `Error` | 17.53 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userdispersing | `Error` | 14.48 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userdata | `Error` | 95.08 | test_deploy_vm_with_userdata.py
   test_deployvm_userdata_post | `Error` | 16.53 | test_deploy_vm_with_userdata.py
   test_network_acl | `Error` | 76.97 | test_network_acl.py
   ContextSuite context=TestRemoteDiagnostics>:setup | `Error` | 0.00 | test_diagnostics.py
   test_delete_account | `Error` | 107.65 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 15.57 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 16.63 | test_network.py
   test_deploy_vm_l2network | `Error` | 10.42 | test_network.py
   test_deploy_vm_l2network | `Error` | 11.48 | test_network.py
   test_l2network_restart | `Error` | 11.50 | test_network.py
   test_l2network_restart | `Error` | 12.56 | test_network.py
   ContextSuite context=TestL2Networks>:teardown | `Error` | 13.65 | test_network.py
   ContextSuite context=TestPortForwarding>:setup | `Error` | 114.18 | test_network.py
   ContextSuite context=TestPublicIP>:setup | `Error` | 95.96 | test_network.py
   test_reboot_router | `Error` | 96.45 | test_network.py
   test_releaseIP | `Error` | 92.36 | test_network.py
   ContextSuite context=TestRouterRules>:setup | `Error` | 189.82 | test_network.py
   test_01_nic | `Error` | 131.78 | test_nic.py
   test_03_deploy_vm_domain_service_offering | `Error` | 101.82 | test_domain_service_offerings.py
   ContextSuite context=TestIsolatedNetworksPasswdServer>:setup | `Error` | 0.00 | test_password_server.py
   test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | `Error` | 73.19 | test_internal_lb.py
   test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | `Error` | 75.33 | test_internal_lb.py
   test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Error` | 135.45 | test_internal_lb.py
   test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Error` | 137.58 | test_internal_lb.py
   test_03_vpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 74.05 | test_internal_lb.py
   test_03_vpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 75.11 | test_internal_lb.py
   test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 129.16 | test_internal_lb.py
   test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 130.24 | test_internal_lb.py
   test_01_deploy_kubernetes_cluster | `Failure` | 111.78 | test_kubernetes_clusters.py
   test_02_invalid_upgrade_kubernetes_cluster | `Failure` | 99.19 | test_kubernetes_clusters.py
   test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 117.58 | test_kubernetes_clusters.py
   test_04_deploy_and_scale_kubernetes_cluster | `Failure` | 95.02 | test_kubernetes_clusters.py
   test_05_delete_kubernetes_cluster | `Failure` | 93.94 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 97.08 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 94.96 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 95.05 | test_kubernetes_clusters.py
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 318.77 | test_kubernetes_clusters.py
   ContextSuite context=TestListIdsParams>:setup | `Error` | 0.00 | test_list_ids_parameter.py
   test_nic_secondaryip_add_remove | `Error` | 102.77 | test_multipleips_per_nic.py
   test_nested_virtualization_vmware | `Error` | 97.20 | test_nested_virtualization.py
   test_01_create_delete_portforwarding_fornonvpc | `Error` | 99.73 | test_portforwardingrules.py
   test_02_vpc_privategw_static_routes | `Failure` | 134.59 | test_privategw_acl.py
   test_02_vpc_privategw_static_routes | `Error` | 136.75 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 121.36 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Error` | 123.50 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Failure` | 240.57 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Error` | 242.72 | test_privategw_acl.py
   test_09_project_suspend | `Error` | 101.34 | test_projects.py
   test_10_project_activation | `Error` | 16.57 | test_projects.py
   ContextSuite context=TestResetVmOnReboot>:setup | `Error` | 0.00 | test_reset_vm_on_reboot.py
   test_01_so_removal_resource_update | `Error` | 102.33 | test_resource_accounting.py
   ContextSuite context=TestRouterDHCPHosts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDHCPOpts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDns>:setup | `Error` | 0.00 | test_router_dns.py
   ContextSuite context=TestRouterDnsService>:setup | `Error` | 0.00 | test_router_dnsservice.py
   test_02_routervm_iptables_policies | `Error` | 97.11 | test_routers_iptables_default_policy.py
   test_01_single_VPC_iptables_policies | `Error` | 71.98 | test_routers_iptables_default_policy.py
   test_01_single_VPC_iptables_policies | `Error` | 91.45 | test_routers_iptables_default_policy.py
   test_01_isolate_network_FW_PF_default_routes_egress_true | `Error` | 98.19 | test_routers_network_ops.py
   test_02_isolate_network_FW_PF_default_routes_egress_false | `Error` | 116.58 | test_routers_network_ops.py
   test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Error` | 185.76 | test_routers_network_ops.py
   test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Error` | 189.15 | test_routers_network_ops.py
   test_03_RVR_Network_check_router_state | `Error` | 187.79 | test_routers_network_ops.py
   ContextSuite context=TestRouterServices>:setup | `Error` | 0.00 | test_routers.py
   ContextSuite context=TestServiceOfferings>:setup | `Error` | 107.75 | test_service_offerings.py
   ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py
   ContextSuite context=TestVAppsVM>:setup | `Error` | 44.53 | test_vm_life_cycle.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 122.12 | test_vpc_redundant.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Error` | 138.61 | test_vpc_redundant.py
   test_02_redundant_VPC_default_routes | `Failure` | 118.10 | test_vpc_redundant.py
   test_02_redundant_VPC_default_routes | `Error` | 136.63 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 114.00 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Error` | 129.45 | test_vpc_redundant.py
   test_04_rvpc_network_garbage_collector_nics | `Failure` | 113.12 | test_vpc_redundant.py
   test_04_rvpc_network_garbage_collector_nics | `Error` | 129.62 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Failure` | 122.03 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Error` | 135.44 | test_vpc_redundant.py
   test_01_VPC_nics_after_destroy | `Failure` | 66.19 | test_vpc_router_nics.py
   test_02_VPC_default_routes | `Failure` | 68.82 | test_vpc_router_nics.py
   test_01_redundant_vpc_site2site_vpn | `Failure` | 198.25 | test_vpc_vpn.py
   test_01_redundant_vpc_site2site_vpn | `Error` | 200.38 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn_multiple_options | `Failure` | 107.89 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn_multiple_options | `Error` | 110.01 | test_vpc_vpn.py
   test_01_vpc_remote_access_vpn | `Failure` | 67.70 | test_vpc_vpn.py
   test_01_vpc_remote_access_vpn | `Error` | 68.77 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn | `Failure` | 113.80 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn | `Error` | 115.92 | test_vpc_vpn.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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-801050535






----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-801018985


   @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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-803247477


   <b>[S] Trillian test result (tid-182)</b>
   Environment: vmware-60u2 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 53679 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4717-t182-vmware-60u2.zip
   Intermittent failure detected: /marvin/tests/smoke/test_accounts.py
   Intermittent failure detected: /marvin/tests/smoke/test_affinity_groups_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_async_job.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vgpu_enabled_vm.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_extra_config_data.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vms_with_varied_deploymentplanners.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_with_userdata.py
   Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
   Intermittent failure detected: /marvin/tests/smoke/test_domain_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_human_readable_logs.py
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_supported_versions.py
   Intermittent failure detected: /marvin/tests/smoke/test_list_ids_parameter.py
   Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py
   Intermittent failure detected: /marvin/tests/smoke/test_metrics_api.py
   Intermittent failure detected: /marvin/tests/smoke/test_multipleips_per_nic.py
   Intermittent failure detected: /marvin/tests/smoke/test_nested_virtualization.py
   Intermittent failure detected: /marvin/tests/smoke/test_network_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_network.py
   Intermittent failure detected: /marvin/tests/smoke/test_nic.py
   Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
   Intermittent failure detected: /marvin/tests/smoke/test_portforwardingrules.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
   Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers.py
   Intermittent failure detected: /marvin/tests/smoke/test_scale_vm.py
   Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Smoke tests completed. 43 look OK, 43 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   ContextSuite context=TestAddVmToSubDomain>:setup | `Error` | 88.62 | test_accounts.py
   test_DeleteDomain | `Error` | 70.36 | test_accounts.py
   test_forceDeleteDomain | `Failure` | 72.44 | test_accounts.py
   test_forceDeleteDomain | `Error` | 88.83 | test_accounts.py
   test_01_user_remove_VM_running | `Error` | 70.28 | test_accounts.py
   test_DeployVmAntiAffinityGroup_in_project | `Error` | 76.76 | test_affinity_groups_projects.py
   test_query_async_job_result | `Error` | 93.93 | test_async_job.py
   ContextSuite context=TestLoadBalance>:setup | `Error` | 0.00 | test_loadbalance.py
   test_3d_gpu_support | `Error` | 105.85 | test_deploy_vgpu_enabled_vm.py
   test_list_vms_metrics | `Error` | 70.79 | test_metrics_api.py
   test_05_deploy_vm_with_extraconfig_vmware | `Error` | 701.58 | test_deploy_vm_extra_config_data.py
   test_network_acl | `Error` | 78.03 | test_network_acl.py
   test_deployvm_firstfit | `Error` | 70.57 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userconcentrated | `Error` | 16.56 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userdispersing | `Error` | 15.58 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userdata | `Error` | 80.56 | test_deploy_vm_with_userdata.py
   test_deployvm_userdata_post | `Error` | 27.71 | test_deploy_vm_with_userdata.py
   ContextSuite context=TestRemoteDiagnostics>:setup | `Error` | 0.00 | test_diagnostics.py
   test_delete_account | `Error` | 94.46 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 11.45 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 12.52 | test_network.py
   test_deploy_vm_l2network | `Error` | 9.40 | test_network.py
   test_deploy_vm_l2network | `Error` | 10.47 | test_network.py
   test_l2network_restart | `Error` | 10.58 | test_network.py
   test_l2network_restart | `Error` | 11.66 | test_network.py
   ContextSuite context=TestL2Networks>:teardown | `Error` | 12.76 | test_network.py
   ContextSuite context=TestPortForwarding>:setup | `Error` | 80.74 | test_network.py
   ContextSuite context=TestPublicIP>:setup | `Error` | 64.59 | test_network.py
   test_reboot_router | `Error` | 71.03 | test_network.py
   test_releaseIP | `Error` | 71.00 | test_network.py
   ContextSuite context=TestRouterRules>:setup | `Error` | 140.10 | test_network.py
   test_01_nic | `Error` | 752.64 | test_nic.py
   test_03_deploy_vm_domain_service_offering | `Error` | 72.34 | test_domain_service_offerings.py
   ContextSuite context=TestIsolatedNetworksPasswdServer>:setup | `Error` | 0.00 | test_password_server.py
   test_01_disableHumanReadableLogs | `Error` | 602.69 | test_human_readable_logs.py
   test_02_enableHumanReadableLogs | `Error` | 602.65 | test_human_readable_logs.py
   test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | `Error` | 83.40 | test_internal_lb.py
   test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | `Error` | 85.54 | test_internal_lb.py
   test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Error` | 156.05 | test_internal_lb.py
   test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Error` | 158.19 | test_internal_lb.py
   test_03_vpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 75.13 | test_internal_lb.py
   test_03_vpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 76.21 | test_internal_lb.py
   test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 151.51 | test_internal_lb.py
   test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 152.58 | test_internal_lb.py
   test_01_deploy_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_02_invalid_upgrade_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_04_deploy_and_scale_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_05_delete_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_06_deploy_invalid_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_09_project_suspend | `Error` | 71.75 | test_projects.py
   test_10_project_activation | `Error` | 17.66 | test_projects.py
   test_01_add_delete_kubernetes_supported_version | `Error` | 0.01 | test_kubernetes_supported_versions.py
   ContextSuite context=TestListIdsParams>:setup | `Error` | 0.00 | test_list_ids_parameter.py
   test_nic_secondaryip_add_remove | `Error` | 72.10 | test_multipleips_per_nic.py
   test_nested_virtualization_vmware | `Error` | 70.80 | test_nested_virtualization.py
   test_01_create_delete_portforwarding_fornonvpc | `Error` | 69.33 | test_portforwardingrules.py
   test_02_vpc_privategw_static_routes | `Failure` | 136.85 | test_privategw_acl.py
   test_02_vpc_privategw_static_routes | `Error` | 139.04 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 148.25 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Error` | 150.42 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Failure` | 263.38 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Error` | 265.58 | test_privategw_acl.py
   ContextSuite context=TestResetVmOnReboot>:setup | `Error` | 0.00 | test_reset_vm_on_reboot.py
   test_01_so_removal_resource_update | `Error` | 71.72 | test_resource_accounting.py
   ContextSuite context=TestRouterDHCPHosts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDHCPOpts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDns>:setup | `Error` | 0.00 | test_router_dns.py
   ContextSuite context=TestRouterDnsService>:setup | `Error` | 0.00 | test_router_dnsservice.py
   test_02_routervm_iptables_policies | `Error` | 76.73 | test_routers_iptables_default_policy.py
   test_01_single_VPC_iptables_policies | `Error` | 90.26 | test_routers_iptables_default_policy.py
   test_01_single_VPC_iptables_policies | `Error` | 109.78 | test_routers_iptables_default_policy.py
   test_01_isolate_network_FW_PF_default_routes_egress_true | `Error` | 64.68 | test_routers_network_ops.py
   test_02_isolate_network_FW_PF_default_routes_egress_false | `Error` | 80.90 | test_routers_network_ops.py
   test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Error` | 144.32 | test_routers_network_ops.py
   test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Error` | 148.34 | test_routers_network_ops.py
   test_03_RVR_Network_check_router_state | `Error` | 150.94 | test_routers_network_ops.py
   ContextSuite context=TestRouterServices>:setup | `Error` | 0.00 | test_routers.py
   test_01_scale_vm | `Error` | 604.68 | test_scale_vm.py
   ContextSuite context=TestServiceOfferings>:setup | `Error` | 79.31 | test_service_offerings.py
   ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py
   test_03_ssvm_internals | `Error` | 602.90 | test_ssvm.py
   test_04_cpvm_internals | `Error` | 602.87 | test_ssvm.py
   test_05_stop_ssvm | `Error` | 687.05 | test_ssvm.py
   test_06_stop_cpvm | `Error` | 708.26 | test_ssvm.py
   test_07_reboot_ssvm | `Error` | 702.15 | test_ssvm.py
   test_08_reboot_cpvm | `Error` | 611.20 | test_ssvm.py
   test_09_destroy_ssvm | `Error` | 682.57 | test_ssvm.py
   test_10_destroy_cpvm | `Error` | 675.68 | test_ssvm.py
   ContextSuite context=TestVAppsVM>:setup | `Error` | 44.25 | test_vm_life_cycle.py
   test_10_attachAndDetach_iso | `Failure` | 607.75 | test_vm_life_cycle.py
   test_change_service_offering_for_vm_with_snapshots | `Failure` | 710.59 | test_vm_snapshots.py
   test_01_create_vm_snapshots | `Failure` | 604.82 | test_vm_snapshots.py
   test_02_revert_vm_snapshots | `Failure` | 601.70 | test_vm_snapshots.py
   test_03_delete_vm_snapshots | `Failure` | 0.02 | test_vm_snapshots.py
   test_01_create_volume | `Failure` | 615.08 | test_volumes.py
   test_02_attach_volume | `Failure` | 612.84 | test_volumes.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 151.94 | test_vpc_redundant.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Error` | 167.46 | test_vpc_redundant.py
   test_02_redundant_VPC_default_routes | `Failure` | 148.81 | test_vpc_redundant.py
   test_02_redundant_VPC_default_routes | `Error` | 166.41 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 144.98 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Error` | 162.57 | test_vpc_redundant.py
   test_04_rvpc_network_garbage_collector_nics | `Failure` | 149.88 | test_vpc_redundant.py
   test_04_rvpc_network_garbage_collector_nics | `Error` | 167.44 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Failure` | 153.06 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Error` | 168.56 | test_vpc_redundant.py
   test_01_VPC_nics_after_destroy | `Failure` | 82.58 | test_vpc_router_nics.py
   test_02_VPC_default_routes | `Failure` | 77.52 | test_vpc_router_nics.py
   test_01_redundant_vpc_site2site_vpn | `Failure` | 270.05 | test_vpc_vpn.py
   test_01_redundant_vpc_site2site_vpn | `Error` | 272.24 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn_multiple_options | `Failure` | 131.32 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn_multiple_options | `Error` | 133.48 | test_vpc_vpn.py
   test_01_vpc_remote_access_vpn | `Failure` | 77.92 | test_vpc_vpn.py
   test_01_vpc_remote_access_vpn | `Error` | 78.99 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn | `Failure` | 133.21 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn | `Error` | 135.36 | test_vpc_vpn.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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r595742398



##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -201,7 +202,8 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
         }
 
         boolean isAdmin = false;

Review comment:
       ```suggestion
           boolean isAdmin = _accountService.isRootAdmin(caller.getId());
   ```
   
   Similarly as the other vars for domain admin / user.
   




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-801738042


   @DaanHoogland @andrijapanicsb sorry guys, disagree with you - the PR introduces an enhancement to allow admins to get recursive records of a domain much like other APIs such as listVolumes, listVirtualMachines. Moved it to 4.15.1 milestone.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] andrijapanicsb commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
andrijapanicsb commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-799233559


   AS discussed with Daan, this is NOT a bug, but a feature, extension of the current API to provide new functionality
   
   Moving to 4.16.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-812451002


   Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 309


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r581744653



##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -216,6 +216,31 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
             s_logger.debug("Account details not available. Using userContext accountId: " + accountId);
         }
 
+        // Check if a domain admin is allowed to access the requested account info.
+        if (_accountService.isDomainAdmin(caller.getId()) && accountId != null){
+            long accountDomainId = _accountDao.getDomainIdForGivenAccountId(accountId);
+            long callerDomainId = caller.getDomainId();
+            boolean matchFound = false;
+
+            if (callerDomainId == accountDomainId) {
+                matchFound = true;
+            } else {
+                // Check if the account is in a child domain of this domain admin.
+                List<DomainVO> childDomains = _domainDao.findAllChildren(_domainDao.findById(caller.getDomainId()).getPath(), caller.getDomainId());
+
+                for (DomainVO domainVO: childDomains) {
+                    if (accountDomainId == domainVO.getId()) {

Review comment:
       The _accountMgr.buildACLSearchParameter method needs a seaarchBuilder and not searchCriteria. I believe you intended it for fetching usage records, but I'm I am simply doing authorization here. 
   
   If a Root admin does the call with a domain id, that is handled on line 276. If a domain admin does the call, authorization will happen first. Users cannot call this API.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r583593402



##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -248,7 +273,17 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
             sc.addAnd("domainId", SearchCriteria.Op.IN, domainIds.toArray());
         }
 
-        if (domainId != null) {
+        if (cmd.isRecursive() && domainId != null){
+            SearchCriteria<DomainVO> sdc = _domainDao.createSearchCriteria();
+            sdc.addOr("path", SearchCriteria.Op.LIKE, _domainDao.findById(domainId).getPath() + "%");
+            List<DomainVO> domains = _domainDao.search(sdc, null);
+            List<Long> domainIds = new ArrayList<Long>();
+            for (DomainVO domain : domains)
+                domainIds.add(domain.getId());
+            sc.addAnd("domainId", SearchCriteria.Op.IN, domainIds.toArray());
+        }
+
+        if (!cmd.isRecursive() && domainId != null) {

Review comment:
       Addressed




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-801055567


   Packaging result: :heavy_check_mark: centos7 :heavy_multiplication_x: centos8 :heavy_check_mark: debian. SL-JID 155


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-801059905


   @blueorangutan test centos7 vmware60u2


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r583593233



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/usage/ListUsageRecordsCmd.java
##########
@@ -85,6 +85,10 @@
     @Parameter(name = ApiConstants.OLD_FORMAT, type = CommandType.BOOLEAN, description = "Flag to enable description rendered in old format which uses internal database IDs instead of UUIDs. False by default.")
     private Boolean oldFormat;
 
+    @Parameter(name = ApiConstants.IS_RECURSIVE, type = CommandType.BOOLEAN,
+            description = "Specify if usage records should be fetched recursively per domain.")

Review comment:
       Done




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-787777110






----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r584491107



##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -210,12 +212,45 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
             //If account_id or account_name is explicitly mentioned, list records for the specified account only even if the caller is of type admin
             if (_accountService.isRootAdmin(caller.getId())) {
                 isAdmin = true;
-            } else if (_accountService.isDomainAdmin(caller.getId())) {
-                isDomainAdmin = true;
             }
             s_logger.debug("Account details not available. Using userContext accountId: " + accountId);
         }
 
+        // Check if a domain admin is allowed to access the requested domain id
+        if (isDomainAdmin) {
+            if (domainId != null) {
+                Account callerAccount = _accountService.getAccount(caller.getId());
+                Domain domain = _domainDao.findById(domainId);
+                _accountService.checkAccess(callerAccount, domain);
+            } else {
+                // Domain admins can only access their own domain's usage records.
+                // Set the domain if not specified.
+                domainId = caller.getDomainId();
+            }
+
+            // Check if a domain admin is allowed to access the requested account info.
+            Account account = _accountService.getAccount(accountId);
+            Domain domain = _domainDao.findById(caller.getDomainId());
+            _accountService.checkAccess(account, domain);

Review comment:
       This is checking if the requested account exists in the same domain as the caller. I don't see anything wrong 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-800346639


   @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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-787803120


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


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-807588724


   <b>Trillian test result (tid-262)</b>
   Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 43620 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4717-t262-vmware-67u3.zip
   Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Smoke tests completed. 83 look OK, 3 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_10_traceroute_in_vr | `Failure` | 61.23 | test_diagnostics.py
   test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 800.14 | test_kubernetes_clusters.py
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 83.14 | test_kubernetes_clusters.py
   ContextSuite context=TestVAppsVM>:setup | `Error` | 43.52 | test_vm_life_cycle.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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-787776784


   @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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-783766482


   <b>Trillian test result (tid-3590)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 32143 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4717-t3590-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_iso.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Smoke tests completed. 84 look OK, 2 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_04_rvpc_privategw_static_routes | `Failure` | 234.79 | test_privategw_acl.py
   test_04_extract_Iso | `Failure` | 128.29 | test_iso.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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-805661356


   Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 232


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-787975895


   @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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-790404167


   Why not create a `sb` @Spaceman1984  and then use it to create a `sc`?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-800903368


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


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-783417348


   @Spaceman1984 a 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#discussion_r586286938



##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -210,12 +212,62 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
             //If account_id or account_name is explicitly mentioned, list records for the specified account only even if the caller is of type admin
             if (_accountService.isRootAdmin(caller.getId())) {
                 isAdmin = true;
-            } else if (_accountService.isDomainAdmin(caller.getId())) {
-                isDomainAdmin = true;
             }
             s_logger.debug("Account details not available. Using userContext accountId: " + accountId);
         }
 
+        // Check if a domain admin is allowed to access the requested domain id
+        if (isDomainAdmin) {
+            if (domainId != null) {
+                Account callerAccount = _accountService.getAccount(caller.getId());
+                Domain domain = _domainDao.findById(domainId);
+                _accountService.checkAccess(callerAccount, domain);
+            } else {
+                // Domain admins can only access their own domain's usage records.
+                // Set the domain if not specified.
+                domainId = caller.getDomainId();
+            }
+
+            // Check if a domain admin is allowed to access the requested account info.
+            Account account = _accountService.getAccount(accountId);

Review comment:
       I don't understand, that is being checked inside that block using checkAccess().




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-787777110


   @Spaceman1984 a Jenkins job has been kicked to build packages. 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-800376245


   Packaging result: :heavy_check_mark: centos7 :heavy_multiplication_x: centos8 :heavy_check_mark: debian. SL-JID 139


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-794531950


   <b>[S] Trillian test result (tid-61)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 32531 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4717-t61-kvm-centos7.zip
   Smoke tests completed. 86 look OK, 0 have error(s)
   Only failed 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] andrijapanicsb commented on pull request #4717: Added recursive fetch of child domains for listUsageRecords API call

Posted by GitBox <gi...@apache.org>.
andrijapanicsb commented on pull request #4717:
URL: https://github.com/apache/cloudstack/pull/4717#issuecomment-816752121


   LGTM 


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org