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 2020/07/01 12:21:35 UTC
[GitHub] [cloudstack] Pearl1594 opened a new pull request #4193: Fix usage record count fix
Pearl1594 opened a new pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193
## Description
listUsageRecords returns the count equal to the number of objects returned in the page, rather than the total count
Addresses: https://github.com/apache/cloudstack/issues/4191
## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [X] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
----------------------------------------------------------------
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] olivierlemasle commented on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
olivierlemasle commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-713642861
@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] olivierlemasle commented on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
olivierlemasle commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-678076531
Hi @Pearl1594, did you get any chance to look into the pagination issue? Thank you!
----------------------------------------------------------------
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 #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-694654731
@Pearl1594 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] olivierlemasle commented on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
olivierlemasle commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-703694530
@Pearl1594 Any chance merging this PR?
----------------------------------------------------------------
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 #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-654121948
Can you @davidjumani review this? Thanx
----------------------------------------------------------------
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] olivierlemasle removed a comment on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
olivierlemasle removed a comment on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-713642861
@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] olivierlemasle commented on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
olivierlemasle commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-688447107
@Pearl1594 @rhtyd Is that what you propose?
- First, fix the `count` value with https://github.com/apache/cloudstack/pull/4193/commits/5b5a2e54dd2016481385d3b8558f23280d5047d1 AND return all usage records (incl. usages for IP addresses on networks flagged with "Hide IP address usage", which means reverting temporarily #3235)
- Then, in a separate PR, handle the hiding with a proper solution. I think the best solution (performance-wise) would require a DB schema update:
- Adding a flag (e.g. `hidden`) in table `cloud_usage.cloud_usage` ; `listUsageRecords` would then simply filter out these usages.
- It would also require adding a new column in `cloud_usage.usage_ip_address` to know if the corresponding usage should be hidden. This new flag would be set in`UsageManagerImpl`.
@Pearl1594 Is that what you thought? Do you want me to do the PRs (both or only the second) or do you want to do it?
----------------------------------------------------------------
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 #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-653612159
@Pearl1594 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] rhtyd commented on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-693908938
@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 #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-713309378
@Pearl1594 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] Pearl1594 commented on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-653536294
@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 #4193: Fix usage record count fix
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-652386272
@Pearl1594 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 #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-693813540
@Pearl1594 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] olivierlemasle commented on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
olivierlemasle commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-692926410
> Sounds like a good plan - to revert my PR to 5b5a2e5.
@Pearl1594 Don't you want to also remove https://github.com/apache/cloudstack/blob/caefb0c9b5f3b7234291f13d272c15a928489b3f/server/src/main/java/com/cloud/api/ApiResponseHelper.java#L3437-L3441 (as you did in 69eeac38)?
----------------------------------------------------------------
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] olivierlemasle edited a comment on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
olivierlemasle edited a comment on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-688447107
@Pearl1594 @rhtyd Is that what you proposed?
- First, fix the `count` value with https://github.com/apache/cloudstack/pull/4193/commits/5b5a2e54dd2016481385d3b8558f23280d5047d1 AND return all usage records (incl. usages for IP addresses on networks flagged with "Hide IP address usage", which means reverting temporarily #3235)
- Then, in a separate PR, handle the hiding with a proper solution. I think the best solution (performance-wise) would require a DB schema update:
- Adding a flag (e.g. `hidden`) in table `cloud_usage.cloud_usage` ; `listUsageRecords` would then simply filter out these usages.
- It would also require adding a new column in `cloud_usage.usage_ip_address` to know if the corresponding usage should be hidden. This new flag would be set in`UsageManagerImpl`.
@Pearl1594 Is that what you thought? Do you want me to do the PRs (both or only the second) or do you want to do it?
----------------------------------------------------------------
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 #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-653611698
Packaging result: ✔centos7 ✔debian. JID-1517
----------------------------------------------------------------
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 #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-652939720
Packaging result: ✔centos7 ✔debian. JID-1511
----------------------------------------------------------------
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 #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-654607460
Thanks for checking the mandatory params @Pearl1594, you're right with the argument I hadn't considered start/end date as mandatory parameters.
In that case, perhaps we can spend some time in future to optimise it, for example, get all the IPs given a list of IPs (unique usage IDs) and then join against the details table to check how many IPs need to be hidden, and run a quick loop to find number of counts we've to reduce if any; the current code may significantly impact the list API response which would require many DB calls.
----------------------------------------------------------------
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] olivierlemasle edited a comment on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
olivierlemasle edited a comment on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-692159524
@Pearl1594 I suggest you revert in this PR the change on `UsageServiceImpl.java`.
This PR will then do two things:
- Fix the `count` issue;
- Return all usages (even usages for IP addresses on networks flagged with "Hide IP address usage").
This would fix #4191.
On my side, I've just opened a new PR, #4327, which fixes the issue for hidden IP address usages. However, this PR includes a DB schema change, so I guess it could not be merged in all impacted branches (since 4.13).
@Pearl1594 @rhtyd Is that ok?
----------------------------------------------------------------
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] Pearl1594 commented on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-712803494
It's good to go @olivierlemasle
----------------------------------------------------------------
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 #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-693939775
Packaging result: ✔centos7 ✖centos8 ✔debian. JID-2023
----------------------------------------------------------------
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 #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-693882720
Packaging result: ✔centos7 ✖centos8 ✔debian. JID-2021
----------------------------------------------------------------
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 #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-712804212
@Pearl1594 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] olivierlemasle commented on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
olivierlemasle commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-692159524
@Pearl1594 I suggest you revert in this PR the change on `UsageServiceImpl.java`.
This PR will then do two things:
- Fix the `count` issue;
- Return all usages (even usages for IP addresses on networks flagged with "Hide IP address usage").
This would fix #4191.
On my side, I've just opened a new PR, #4327, which fixes the issue for hidden IP address usages. However, this PR includes a DB schema change, so I guess it could not be merged in all impacted branches (since 4.13).
@Pearl1594 @rhtyd Is that ok?
----------------------------------------------------------------
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 #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-693910768
@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] Pearl1594 commented on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-678080007
Hi @olivierlemasle , I've been tied up with other stuff and haven't been able to give time to this. However, to get the right count when hideIpAddressUsage is true, can be a bit involved inorder to make it efficient.
As a first level fix, we can get the pagination issue sorted, as it currently works, and for handling hiding IP usage records, we can have another PR?
----------------------------------------------------------------
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 merged pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
DaanHoogland merged pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193
----------------------------------------------------------------
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] Pearl1594 commented on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-653611920
@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] Pearl1594 commented on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-712803656
@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] Pearl1594 commented on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-694654396
@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] Pearl1594 commented on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-653605175
@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] olivierlemasle commented on a change in pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
olivierlemasle commented on a change in pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#discussion_r449478789
##########
File path: server/src/main/java/com/cloud/usage/UsageServiceImpl.java
##########
@@ -328,9 +333,20 @@ public boolean generateUsageRecords(GenerateUsageRecordsCmd cmd) {
break;
case UsageTypes.IP_ADDRESS:
IPAddressVO ip = _ipDao.findByUuidIncludingRemoved(usageId);
- if (ip != null) {
- usageDbId = ip.getId();
+ if (ip == null) {
+ break;
+ }
+ Long networkId = ip.getAssociatedWithNetworkId();
+ if (networkId == null) {
+ networkId = ip.getSourceNetworkId();
+ }
+ NetworkDetailVO networkDetail = _networkDetailsDao.findDetail(networkId, Network.hideIpAddressUsage);
+ if (networkDetail != null && networkDetail.getValue() != null && networkDetail.getValue().equals("true")) {
Review comment:
As this code is in a test `if (usageId != null)`, it will remove IP address usage only if the API command `listUsageRecords` was called with a specific `usageId` parameter.
For API calls like `command=listUsageRecords&startdate=xxx&enddate=xxx`, it will include all usage records, including for IP addresses on networks with "Hide usage".
----------------------------------------------------------------
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 #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-653710878
<b>Trillian test result (tid-1964)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35174 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4193-t1964-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
Intermittent failure detected: /marvin/tests/smoke/test_outofbandmanagement.py
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Smoke tests completed. 76 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_02_vpc_privategw_static_routes | `Failure` | 226.32 | test_privategw_acl.py
test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 217.54 | test_privategw_acl.py
test_04_rvpc_privategw_static_routes | `Failure` | 331.09 | test_privategw_acl.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] olivierlemasle commented on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
olivierlemasle commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-658865965
Hi @Pearl1594 @rhtyd What are the required steps before merging this?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] olivierlemasle commented on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
olivierlemasle commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-713870794
Thanks @DaanHoogland
----------------------------------------------------------------
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] Pearl1594 commented on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-654372196
@rhtyd since startdate and enddate are mandatory fields in the API , the usageRecords fetched from the database will be within the (date) range provided and further processing is then done on that list obtained
----------------------------------------------------------------
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 #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-692539349
Ping me when you've reverted changes and kick tests @Pearl1594 thnx
I'll look at the new PR soon @olivierlemasle
----------------------------------------------------------------
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 #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-704214358
this is a 4.13 based change @rhtyd do we need to push this towards 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] Pearl1594 commented on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-713309122
@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] blueorangutan commented on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-713641183
<b>Trillian test result (tid-3029)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 33021 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4193-t3029-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Smoke tests completed. 75 look OK, 2 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_02_vpc_privategw_static_routes | `Failure` | 170.50 | test_privategw_acl.py
test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 170.77 | test_privategw_acl.py
test_04_rvpc_privategw_static_routes | `Failure` | 224.14 | test_privategw_acl.py
test_01_vpc_site2site_vpn_multiple_options | `Failure` | 286.82 | 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 #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-652930880
@Pearl1594 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] olivierlemasle commented on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
olivierlemasle commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-712777900
@Pearl1594 Any new 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] Pearl1594 commented on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-660045113
@olivierlemasle It will probably require another round of testing before it can be merged.
----------------------------------------------------------------
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] olivierlemasle commented on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
olivierlemasle commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-704232698
IMHO, this should be backported in 4.13, 4.14 and recent releases, as it fixes a regression in 4.13.
The "long-term fix" #4327 should be included in the next release.
----------------------------------------------------------------
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 #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-653605584
@Pearl1594 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 #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-694661510
Packaging result: ✔centos7 ✖centos8 ✔debian. JID-2036
----------------------------------------------------------------
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] Pearl1594 commented on pull request #4193: Fix usage record count fix
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-652385913
@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] Pearl1594 commented on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-693811664
@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 #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-713307843
<b>Trillian test result (tid-3016)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 59364 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4193-t3016-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_public_ip_range.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_secondary_storage.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_templates.py
Intermittent failure detected: /marvin/tests/smoke/test_usage.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_hostha_kvm.py
Smoke tests completed. 55 look OK, 22 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_02_vpc_privategw_static_routes | `Failure` | 220.63 | test_privategw_acl.py
test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 165.52 | test_privategw_acl.py
test_04_rvpc_privategw_static_routes | `Failure` | 228.18 | test_privategw_acl.py
ContextSuite context=TestResetVmOnReboot>:setup | `Error` | 0.00 | test_reset_vm_on_reboot.py
ContextSuite context=TestRAMCPUResourceAccounting>:setup | `Error` | 0.00 | test_resource_accounting.py
ContextSuite context=TestRouterDHCPHosts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
ContextSuite context=TestRouterDHCPOpts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
ContextSuite context=TestRouterDns>:setup | `Error` | 0.00 | test_router_dns.py
test_01_sys_vm_start | `Failure` | 0.14 | test_secondary_storage.py
ContextSuite context=TestRouterDnsService>:setup | `Error` | 0.00 | test_router_dnsservice.py
ContextSuite context=TestRouterIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
ContextSuite context=TestVPCIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
ContextSuite context=TestIsolatedNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py
ContextSuite context=TestRedundantIsolateNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py
ContextSuite context=TestRouterServices>:setup | `Error` | 0.00 | test_routers.py
ContextSuite context=TestCpuCapServiceOfferings>:setup | `Error` | 0.00 | test_service_offerings.py
ContextSuite context=TestServiceOfferings>:setup | `Error` | 0.29 | test_service_offerings.py
ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py
test_01_list_sec_storage_vm | `Failure` | 0.06 | test_ssvm.py
test_02_list_cpvm_vm | `Failure` | 0.05 | test_ssvm.py
test_03_ssvm_internals | `Failure` | 0.06 | test_ssvm.py
test_04_cpvm_internals | `Failure` | 0.05 | test_ssvm.py
test_05_stop_ssvm | `Failure` | 0.06 | test_ssvm.py
test_06_stop_cpvm | `Failure` | 0.05 | test_ssvm.py
test_07_reboot_ssvm | `Failure` | 0.06 | test_ssvm.py
test_08_reboot_cpvm | `Failure` | 0.05 | test_ssvm.py
test_09_destroy_ssvm | `Failure` | 0.06 | test_ssvm.py
test_10_destroy_cpvm | `Failure` | 0.05 | test_ssvm.py
test_02_create_template_with_checksum_sha1 | `Error` | 65.60 | test_templates.py
test_03_create_template_with_checksum_sha256 | `Error` | 65.70 | test_templates.py
test_04_create_template_with_checksum_md5 | `Error` | 65.61 | test_templates.py
test_05_create_template_with_no_checksum | `Error` | 65.59 | test_templates.py
test_02_deploy_vm_from_direct_download_template | `Error` | 1.39 | test_templates.py
test_03_deploy_vm_wrong_checksum | `Error` | 1.40 | test_templates.py
ContextSuite context=TestTemplates>:setup | `Error` | 19.05 | test_templates.py
ContextSuite context=TestISOUsage>:setup | `Error` | 0.00 | test_usage.py
ContextSuite context=TestLBRuleUsage>:setup | `Error` | 0.00 | test_usage.py
ContextSuite context=TestNatRuleUsage>:setup | `Error` | 0.00 | test_usage.py
ContextSuite context=TestPublicIPUsage>:setup | `Error` | 0.00 | test_usage.py
ContextSuite context=TestSnapshotUsage>:setup | `Error` | 0.00 | test_usage.py
ContextSuite context=TestVmUsage>:setup | `Error` | 0.00 | test_usage.py
ContextSuite context=TestVolumeUsage>:setup | `Error` | 0.00 | test_usage.py
ContextSuite context=TestVpnUsage>:setup | `Error` | 0.00 | test_usage.py
ContextSuite context=Test01DeployVM>:setup | `Error` | 0.00 | test_vm_life_cycle.py
ContextSuite context=Test02VMLifeCycle>:setup | `Error` | 0.00 | test_vm_life_cycle.py
test_14_secure_to_secure_vm_migration | `Error` | 11.53 | test_vm_life_cycle.py
test_15_secured_to_nonsecured_vm_migration | `Error` | 94.28 | test_vm_life_cycle.py
test_16_nonsecured_to_secured_vm_migration | `Error` | 1.32 | test_vm_life_cycle.py
ContextSuite context=TestVmSnapshot>:setup | `Error` | 2.18 | test_vm_snapshots.py
ContextSuite context=TestCreateVolume>:setup | `Error` | 0.00 | test_volumes.py
ContextSuite context=TestVolumes>:setup | `Error` | 0.00 | test_volumes.py
ContextSuite context=TestVPCRedundancy>:setup | `Error` | 0.00 | test_vpc_redundant.py
ContextSuite context=TestVPCNics>:setup | `Error` | 0.00 | test_vpc_router_nics.py
ContextSuite context=TestRVPCSite2SiteVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
ContextSuite context=TestVPCSite2SiteVPNMultipleOptions>:setup | `Error` | 0.00 | test_vpc_vpn.py
ContextSuite context=TestVpcRemoteAccessVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
ContextSuite context=TestVpcSite2SiteVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
test_disable_oobm_ha_state_ineligible | `Error` | 1515.63 | 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] Pearl1594 commented on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-692455059
> @Pearl1594 @rhtyd Is that what you proposed?
>
> * First, fix the `count` value with [5b5a2e5](https://github.com/apache/cloudstack/commit/5b5a2e54dd2016481385d3b8558f23280d5047d1) AND return all usage records (incl. usages for IP addresses on networks flagged with "Hide IP address usage", which means reverting temporarily #3235)
>
> * Then, in a separate PR, handle the hiding with a proper solution. I think the best solution (performance-wise) would require a DB schema update:
>
> * Adding a flag (e.g. `hidden`) in table `cloud_usage.cloud_usage` ; `listUsageRecords` would then simply filter out these usages.
> * It would also require adding a new column in `cloud_usage.usage_ip_address` to know if the corresponding usage should be hidden. This new flag would be set in`UsageManagerImpl`.
>
>
> @Pearl1594 Is that what you thought? Do you want me to do the PRs (both or only the second) or do you want to do it?
@olivierlemasle Apologies for the delay in response, but this was precisely what I meant.
Sounds like a good plan - to revert my PR to https://github.com/apache/cloudstack/commit/5b5a2e54dd2016481385d3b8558f23280d5047d1.
Thanks a lot for the PR!
----------------------------------------------------------------
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] olivierlemasle edited a comment on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
olivierlemasle edited a comment on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-713870794
Thanks @DaanHoogland @Pearl1594
----------------------------------------------------------------
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] Pearl1594 commented on pull request #4193: Fix usage record count
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4193:
URL: https://github.com/apache/cloudstack/pull/4193#issuecomment-652930395
@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