You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "DaanHoogland (via GitHub)" <gi...@apache.org> on 2025/04/14 14:27:35 UTC
[PR] Ghi9800 update vr on setting change [cloudstack]
DaanHoogland opened a new pull request, #10710:
URL: https://github.com/apache/cloudstack/pull/10710
### Description
This PR...
<!--- 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: #9800
<!--- ******************************************************************************* -->
<!--- NOTE: AUTOMATION 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)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
- [ ] build/CI
- [ ] test (unit or integration test code)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [ ] Major
- [ ] Minor
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [ ] Minor
- [ ] Trivial
### Screenshots (if appropriate):
### How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
#### How did you try to break this feature and the system with this change?
<!-- see how your change affects other areas of the code, etc. -->
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3317079904
> @vishesh92 @harikrishna-patnala @weizhouapache , there is a slight backwards incompatibility with this change; success can now be true, false or not present. In addition to result there is status, which can be “success”, “failure”, “warning” or “unknown”. So the incompatibilty is that result may not be present. I think this is not grave as it would be a false positive in the past. What do you think?
>
> Other than this we are good to merge (@vladimirpetrov tested but forgot to give his opinion here;)
as we are only updating the value of the response parameter properly I think this is fine and moreover more detailed 'status' is now present.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2832303690
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3162807467
> We probably need to raise this against 4.19?
@Pearl1594 , this is kind of an improvement, right, not a bug fix. I think we can choose where ever we want to merge it, no?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3219771517
<b>[SF] Trillian Build Failed (tid-14107)<b/>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3247446036
<b>[SF] Trillian test result (tid-14186)</b>
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 52512 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr10710-t14186-kvm-ol8.zip
Smoke tests completed. 145 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 288.99 | test_vpc_redundant.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3244797529
@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3244732334
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 14817
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3313207357
Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 15111
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3219369741
<b>[SF] Trillian Build Failed (tid-14104)<b/>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#discussion_r2303185380
##########
engine/schema/src/main/resources/META-INF/db/schema-41910to41920.sql:
##########
Review Comment:
remove this change completely
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2854580098
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2865785411
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2862582434
@blueorangutan test keepEnv
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2815237245
@sureshanaparti , you tested this (https://github.com/apache/cloudstack/issues/10522#issuecomment-2800636495)?
Do you think we should merge this and take it from there, or ignore if @Sparc0 does not confirm it fixed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2802146413
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13050
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#discussion_r2309264906
##########
server/src/test/java/com/cloud/vpc/MockConfigurationManagerImpl.java:
##########
@@ -96,6 +96,14 @@
@Component
public class MockConfigurationManagerImpl extends ManagerBase implements ConfigurationManager, ConfigurationService {
+ public static final ConfigKey<Integer> NETWORK_LB_HAPROXY_MAX_CONN = new ConfigKey<>(
+ "Network",
+ Integer.class,
+ "network.loadbalancer.haproxy.max.conn",
+ "4096",
+ "Load Balancer(haproxy) maximum number of concurrent connections(global max)",
+ true,
+ ConfigKey.Scope.Global);
Review Comment:
move to configuration manager
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3315942276
<b>[SF] Trillian test result (tid-14415)</b>
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 50207 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr10710-t14415-kvm-ol8.zip
Smoke tests completed. 147 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2801935426
## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/10710?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 4.28%. Comparing base [(`99ea77d`)](https://app.codecov.io/gh/apache/cloudstack/commit/99ea77dc83901fd88db4e1fcb2f7623bd343b5b1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`82cc21f`)](https://app.codecov.io/gh/apache/cloudstack/commit/82cc21f5bcc63c9142f1e30aee9d2539a42c11e2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 1 commits behind head on 4.19.
> :exclamation: There is a different number of reports uploaded between BASE (99ea77d) and HEAD (82cc21f). Click for more details.
>
> <details><summary>HEAD has 1 upload less than BASE</summary>
>
>| Flag | BASE (99ea77d) | HEAD (82cc21f) |
>|------|------|------|
>|unittests|1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 4.19 #10710 +/- ##
=============================================
- Coverage 15.16% 4.28% -10.88%
=============================================
Files 5415 369 -5046
Lines 474848 29672 -445176
Branches 57914 5208 -52706
=============================================
- Hits 72018 1272 -70746
+ Misses 394781 28255 -366526
+ Partials 8049 145 -7904
```
| [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/10710/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/10710/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `4.28% <ø> (-0.01%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/cloudstack/pull/10710/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/10710?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Ghi9800 update vr on setting change [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2801902592
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala merged PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#discussion_r2303187003
##########
engine/schema/src/main/resources/META-INF/db/schema-42000to42010.sql:
##########
Review Comment:
no change should be here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2854576402
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2830621614
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13183
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2846675843
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3169589803
<b>[SF] Trillian test result (tid-14043)</b>
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 51698 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr10710-t14043-kvm-ol8.zip
Smoke tests completed. 140 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_02_router_internal_adv | `Failure` | 14.75 | test_routers.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2830391749
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2804121810
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#discussion_r2303187003
##########
engine/schema/src/main/resources/META-INF/db/schema-42000to42010.sql:
##########
Review Comment:
move this change to 21->22 update
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3315158419
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15123
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3243092333
<b>[SF] Trillian Build Failed (tid-14171)<b/>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3244791983
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3219838815
<b>[SF] Trillian Build Failed (tid-14108)<b/>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2846698541
Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 13239
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3166278276
<b>[SF] Trillian test result (tid-14042)</b>
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 55917 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr10710-t14042-kvm-ol8.zip
Smoke tests completed. 140 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_02_router_internal_adv | `Failure` | 15.15 | test_routers.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3244411766
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3244461092
Packaging result [SF]: ✔️ debian. SL-JID 14816
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3315112559
> @DaanHoogland is this failure related "test_01_deployVMInSharedNetwork" ?
maybe just a intermittent failure
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#discussion_r2309791366
##########
engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java:
##########
@@ -70,10 +70,19 @@ public interface ConfigurationManager {
"0.5",
"Weight for CPU (as a value between 0 and 1) applied to compute capacity for Pods, Clusters and Hosts for COMBINED capacityType for ordering. Weight for RAM will be (1 - weight of CPU)",
true, ConfigKey.Scope.Global);
+ ConfigKey<Integer> NETWORK_LB_HAPROXY_MAX_CONN = new ConfigKey<>(
Review Comment:
this introduces a circular dependency. I will check where else to put 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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#discussion_r2309795719
##########
server/src/main/java/com/cloud/network/router/CommandSetupHelper.java:
##########
@@ -542,11 +543,11 @@ public void createApplyIpv6FirewallRulesCommands(final List<? extends FirewallRu
}
public void createFirewallRulesCommands(final List<? extends FirewallRule> rules, final VirtualRouter router, final Commands cmds, final long guestNetworkId) {
- final List<FirewallRuleTO> rulesTO = new ArrayList<FirewallRuleTO>();
+ final List<FirewallRuleTO> rulesTO = new ArrayList<>();
String systemRule = null;
Boolean defaultEgressPolicy = false;
if (rules != null) {
- if (rules.size() > 0) {
+ if (!rules.isEmpty()) {
Review Comment:
no, there is a for loop in the != null block after this if statement.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3315924255
@vishesh92 @harikrishna-patnala @weizhouapache , there is a slight backwards incompatibility with this change; success can now be true, false or not present. In addition to result there is status, which can be “success”, “failure”, “warning” or “unknown”. So the incompatibilty is that result may not be present. I think this is not grave as it would be a false positive in the past. What do you think?
Other than this we are good to merge (@vladimirpetrov tested but forgot to give his opinion here;)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2846680401
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2801911801
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3234057023
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14754
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3244418691
@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3219658835
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14722
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3219772755
<b>[SF] Trillian Build Failed (tid-14106)<b/>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3242903648
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 14808
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3310748258
<b>[SF] Trillian test result (tid-14379)</b>
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 54023 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr10710-t14379-kvm-ol8.zip
Smoke tests completed. 145 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_03_deploy_and_scale_kubernetes_cluster | `Failure` | 1.17 | test_kubernetes_clusters.py
test_04_autoscale_kubernetes_cluster | `Failure` | 27.82 | test_kubernetes_clusters.py
test_01_deployVMInSharedNetwork | `Failure` | 410.15 | test_network.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2804118765
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2807596277
<b>[SF] Trillian test result (tid-12989)</b>
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 48168 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr10710-t12989-kvm-ol8.zip
Smoke tests completed. 133 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2854336223
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13274
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2857333477
<b>[SF] Trillian test result (tid-13221)</b>
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 59934 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr10710-t13221-kvm-ol8.zip
Smoke tests completed. 140 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_02_router_internal_adv | `Failure` | 14.93 | test_routers.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3163050169
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14571
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3228127956
Packaging result [SF]: ✖️ el8 ✖️ el9 ✔️ debian ✖️ suse15. SL-JID 14741
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on code in PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#discussion_r2309298866
##########
api/src/main/java/org/apache/cloudstack/api/response/RouterHealthCheckResultResponse.java:
##########
@@ -19,6 +19,7 @@
import java.util.Date;
+import com.cloud.network.VirtualNetworkApplianceService;
Review Comment:
may import `com.cloud.network.VirtualNetworkApplianceService.RouterHealthStatus` instead
##########
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java:
##########
@@ -824,26 +811,23 @@ protected void updateSite2SiteVpnConnectionState(final List<DomainRouterVO> rout
}
final String privateIP = router.getPrivateIpAddress();
final HostVO host = _hostDao.findById(router.getHostId());
- if (host == null || host.getState() != Status.Up) {
- continue;
- } else if (host.getManagementServerId() != ManagementServerNode.getManagementServerId()) {
- /* Only cover hosts managed by this management server */
- continue;
- } else if (privateIP != null) {
+ if ( !(host == null || host.getState() != Status.Up)
Review Comment:
maybe define a boolean variable ?
##########
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java:
##########
@@ -1176,35 +1129,34 @@ protected void runInContext() {
}
} catch (final Exception ex) {
logger.error("Fail to complete the FetchRouterHealthChecksResultTask! ", ex);
- ex.printStackTrace();
}
}
}
private List<String> getFailingChecks(DomainRouterVO router, GetRouterMonitorResultsAnswer answer) {
if (answer == null) {
- logger.warn("Unable to fetch monitor results for router " + router);
- resetRouterHealthChecksAndConnectivity(router.getId(), false, false, "Communication failed");
- return Arrays.asList(CONNECTIVITY_TEST);
+ logger.warn("Unable to fetch monitor results for router {}", router);
+ resetRouterHealthChecksAndConnectivity(router.getId(), RouterHealthStatus.UNKNOWN, RouterHealthStatus.UNKNOWN, "Communication failed");
+ return List.of(CONNECTIVITY_TEST);
} else if (!answer.getResult()) {
logger.warn("Failed to fetch monitor results from router " + router + " with details: " + answer.getDetails());
if (StringUtils.isNotBlank(answer.getDetails()) && answer.getDetails().equalsIgnoreCase(READONLY_FILESYSTEM_ERROR)) {
- resetRouterHealthChecksAndConnectivity(router.getId(), true, false, "Failed to write: " + answer.getDetails());
- return Arrays.asList(FILESYSTEM_WRITABLE_TEST);
+ resetRouterHealthChecksAndConnectivity(router.getId(), RouterHealthStatus.SUCCESS, RouterHealthStatus.FAILED, "Failed to write: " + answer.getDetails());
+ return List.of(FILESYSTEM_WRITABLE_TEST);
} else {
- resetRouterHealthChecksAndConnectivity(router.getId(), false, false, "Failed to fetch results with details: " + answer.getDetails());
- return Arrays.asList(CONNECTIVITY_TEST);
+ resetRouterHealthChecksAndConnectivity(router.getId(), RouterHealthStatus.FAILED, RouterHealthStatus.UNKNOWN, "Failed to fetch results with details: " + answer.getDetails());
+ return List.of(CONNECTIVITY_TEST);
}
} else {
- resetRouterHealthChecksAndConnectivity(router.getId(), true, true, "Successfully fetched data");
+ resetRouterHealthChecksAndConnectivity(router.getId(), RouterHealthStatus.SUCCESS, RouterHealthStatus.SUCCESS, "Successfully fetched data");
updateDbHealthChecksFromRouterResponse(router, answer.getMonitoringResults());
return answer.getFailingChecks();
}
}
private void handleFailingChecks(DomainRouterVO router, List<String> failingChecks) {
- if (failingChecks == null || failingChecks.size() == 0) {
+ if (failingChecks == null || failingChecks.isEmpty()) {
Review Comment:
CollectionUtils.isEmpty ?
##########
engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java:
##########
@@ -70,10 +70,19 @@ public interface ConfigurationManager {
"0.5",
"Weight for CPU (as a value between 0 and 1) applied to compute capacity for Pods, Clusters and Hosts for COMBINED capacityType for ordering. Weight for RAM will be (1 - weight of CPU)",
true, ConfigKey.Scope.Global);
+ ConfigKey<Integer> NETWORK_LB_HAPROXY_MAX_CONN = new ConfigKey<>(
Review Comment:
currently most network-related configurations are defined in NetworkOrchestrator
##########
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java:
##########
@@ -1673,19 +1624,15 @@ private boolean updateRouterHealthChecksConfig(DomainRouterVO router) {
}
private String getSystemThresholdsHealthChecksData(final DomainRouterVO router) {
- return new StringBuilder()
- .append("minDiskNeeded=" + RouterHealthChecksFreeDiskSpaceThreshold.valueIn(router.getDataCenterId()))
- .append(",maxCpuUsage=" + RouterHealthChecksMaxCpuUsageThreshold.valueIn(router.getDataCenterId()))
- .append(",maxMemoryUsage=" + RouterHealthChecksMaxMemoryUsageThreshold.valueIn(router.getDataCenterId()) + ";")
- .toString();
+ return "minDiskNeeded=" + RouterHealthChecksFreeDiskSpaceThreshold.valueIn(router.getDataCenterId()) +
+ ",maxCpuUsage=" + RouterHealthChecksMaxCpuUsageThreshold.valueIn(router.getDataCenterId()) +
+ ",maxMemoryUsage=" + RouterHealthChecksMaxMemoryUsageThreshold.valueIn(router.getDataCenterId()) + ";";
}
private String getRouterVersionHealthChecksData(final DomainRouterVO router) {
if (router.getTemplateVersion() != null && router.getScriptsVersion() != null) {
- StringBuilder routerVersion = new StringBuilder()
- .append("templateVersion=" + router.getTemplateVersion())
- .append(",scriptsVersion=" + router.getScriptsVersion());
- return routerVersion.toString();
+ return "templateVersion=" + router.getTemplateVersion() +
Review Comment:
String.format ?
##########
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java:
##########
@@ -1088,7 +1041,7 @@ protected void runInContext() {
continue;
}
- DomainRouterVO router = router0;
+ DomainRouterVO router;
Review Comment:
can this line and next 5 lines be simplified to 1 line ?
##########
server/src/main/java/com/cloud/network/router/CommandSetupHelper.java:
##########
@@ -542,11 +543,11 @@ public void createApplyIpv6FirewallRulesCommands(final List<? extends FirewallRu
}
public void createFirewallRulesCommands(final List<? extends FirewallRule> rules, final VirtualRouter router, final Commands cmds, final long guestNetworkId) {
- final List<FirewallRuleTO> rulesTO = new ArrayList<FirewallRuleTO>();
+ final List<FirewallRuleTO> rulesTO = new ArrayList<>();
String systemRule = null;
Boolean defaultEgressPolicy = false;
if (rules != null) {
- if (rules.size() > 0) {
+ if (!rules.isEmpty()) {
Review Comment:
use CollectionUtils ?
it looks like `if (rules != null) {` and `if (!rules.isEmpty()) {` can be merged into 1 check
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3315076399
@DaanHoogland is this failure related "test_01_deployVMInSharedNetwork" ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3315163538
@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "Pearl1594 (via GitHub)" <gi...@apache.org>.
Pearl1594 commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2934117324
We probably need to raise this against 4.19?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2830395640
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3244017325
<b>[SF] Trillian Build Failed (tid-14176)<b/>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#discussion_r2313459688
##########
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java:
##########
@@ -1364,9 +1310,9 @@ private void updateRouterHealthCheckResult(final long routerId, String checkName
}
private RouterHealthCheckResultVO parseHealthCheckVOFromJson(final long routerId,
- final String checkName, final String checkType, final Map<String, String> checkData,
- final Map<String, Map<String, RouterHealthCheckResultVO>> checksInDb) {
- boolean success = Boolean.parseBoolean(checkData.get("success"));
+ final String checkName, final String checkType, final Map<String, String> checkData,
+ final Map<String, Map<String, RouterHealthCheckResultVO>> checksInDb) {
+ RouterHealthStatus success = RouterHealthStatus.valueOf(checkData.get("success"));
Review Comment:
Using valueOf() directly without validation could throw IllegalArgumentException if the string value doesn't match any enum constant. Consider adding error handling or using a safer parsing method to handle invalid health status values gracefully.
```suggestion
RouterHealthStatus success = parseRouterHealthStatus(checkData.get("success"));
```
##########
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java:
##########
@@ -2343,10 +2277,7 @@ public boolean finalizeCommandsOnStart(final Commands cmds, final VirtualMachine
// restart network if restartNetwork = false is not specified in profile
// parameters
- boolean reprogramGuestNtwks = true;
- if (profile.getParameter(Param.ReProgramGuestNetworks) != null && (Boolean) profile.getParameter(Param.ReProgramGuestNetworks) == false) {
- reprogramGuestNtwks = false;
- }
+ boolean reprogramGuestNtwks = ! Boolean.FALSE.equals(profile.getParameter(Param.ReProgramGuestNetworks));
Review Comment:
The spacing around the '!' operator is inconsistent with Java conventions. It should be `boolean reprogramGuestNtwks = !Boolean.FALSE.equals(profile.getParameter(Param.ReProgramGuestNetworks));` (no space after the exclamation mark).
```suggestion
boolean reprogramGuestNtwks = !Boolean.FALSE.equals(profile.getParameter(Param.ReProgramGuestNetworks));
```
##########
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java:
##########
@@ -3259,18 +3178,18 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
if (stats.getCurrentBytesReceived() > answerFinal.getBytesReceived()) {
- if (logger.isDebugEnabled()) {
- logger.debug("Received # of bytes that's less than the last one. " + "Assuming something went wrong and persisting it. Router: "
- + answerFinal.getRouterName() + " Reported: " + toHumanReadableSize(answerFinal.getBytesReceived()) + " Stored: " + toHumanReadableSize(stats.getCurrentBytesReceived()));
- }
+ logger.debug("Received # of bytes that's less than the last one. Assuming something went wrong and persisting it. Router: {} Reported: {} Stored: {}"
+ , answerFinal.getRouterName()
+ , toHumanReadableSize(answerFinal.getBytesReceived())
+ , toHumanReadableSize(stats.getCurrentBytesReceived()));
stats.setNetBytesReceived(stats.getNetBytesReceived() + stats.getCurrentBytesReceived());
}
stats.setCurrentBytesReceived(answerFinal.getBytesReceived());
if (stats.getCurrentBytesSent() > answerFinal.getBytesSent()) {
- if (logger.isDebugEnabled()) {
- logger.debug("Received # of bytes that's less than the last one. " + "Assuming something went wrong and persisting it. Router: "
- + answerFinal.getRouterName() + " Reported: " + toHumanReadableSize(answerFinal.getBytesSent()) + " Stored: " + toHumanReadableSize(stats.getCurrentBytesSent()));
- }
+ logger.debug("Received # of bytes that's less than the last one. Assuming something went wrong and persisting it. Router: {} Reported: {} Stored: {}"
+ , answerFinal.getRouterName()
+ , toHumanReadableSize(answerFinal.getBytesReceived())
+ , toHumanReadableSize(stats.getCurrentBytesReceived()));
Review Comment:
The logging parameters are inconsistent - line 3191 uses answerFinal.getBytesReceived() but line 3192 uses stats.getCurrentBytesReceived(), but the log message suggests both should show the same type of data for comparison.
##########
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java:
##########
@@ -545,19 +540,11 @@ public boolean configure(final String name, final Map<String, Object> params) th
final Map<String, String> configs = _configDao.getConfiguration("AgentManager", params);
- _routerRamSize = NumbersUtil.parseInt(configs.get("router.ram.size"), DEFAULT_ROUTER_VM_RAMSIZE);
- _routerCpuMHz = NumbersUtil.parseInt(configs.get("router.cpu.mhz"), DEFAULT_ROUTER_CPU_MHZ);
+ int _routerRamSize = NumbersUtil.parseInt(configs.get("router.ram.size"), DEFAULT_ROUTER_VM_RAMSIZE);
+ int _routerCpuMHz = NumbersUtil.parseInt(configs.get("router.cpu.mhz"), DEFAULT_ROUTER_CPU_MHZ);
Review Comment:
Variables _routerRamSize and _routerCpuMHz are declared as local variables but use instance variable naming convention (underscore prefix). They should be renamed to routerRamSize and routerCpuMHz to follow local variable naming conventions.
```suggestion
int routerRamSize = NumbersUtil.parseInt(configs.get("router.ram.size"), DEFAULT_ROUTER_VM_RAMSIZE);
int routerCpuMHz = NumbersUtil.parseInt(configs.get("router.cpu.mhz"), DEFAULT_ROUTER_CPU_MHZ);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3266721574
this has been (dev-)tested but needs an upgrade test as most important validation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3303801947
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15063
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2864973181
<b>[SF] Trillian test result (tid-13240)</b>
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 57408 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr10710-t13240-kvm-ol8.zip
Smoke tests completed. 140 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_02_router_internal_adv | `Failure` | 14.94 | test_routers.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3228721857
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14742
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2832345063
<b>[SF] Trillian Build Failed (tid-13166)<b/>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2832304861
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-2862584198
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3315113245
@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3315162861
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3315888574
<b>[SF] Trillian test result (tid-14410)</b>
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 52362 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr10710-t14410-kvm-ol8.zip
Smoke tests completed. 147 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3229468648
<b>[SF] Trillian Build Failed (tid-14122)<b/>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#issuecomment-3248691614
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#discussion_r2309193464
##########
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java:
##########
@@ -1470,14 +1420,15 @@ private GetRouterMonitorResultsAnswer fetchAndUpdateRouterHealthChecks(DomainRou
try {
final Answer answer = _agentMgr.easySend(router.getHostId(), command);
+ logger.info("Got health check results from router {}: {}", router.getHostName(), answer != null ? answer.getDetails() : "null answer");
Review Comment:
This info-level logging of health check results on every execution could generate excessive log volume in production. Consider using debug level or adding conditional logging based on result status.
```suggestion
logger.debug("Got health check results from router {}: {}", router.getHostName(), answer != null ? answer.getDetails() : "null answer");
```
##########
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java:
##########
@@ -3259,18 +3182,18 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
if (stats.getCurrentBytesReceived() > answerFinal.getBytesReceived()) {
- if (logger.isDebugEnabled()) {
- logger.debug("Received # of bytes that's less than the last one. " + "Assuming something went wrong and persisting it. Router: "
- + answerFinal.getRouterName() + " Reported: " + toHumanReadableSize(answerFinal.getBytesReceived()) + " Stored: " + toHumanReadableSize(stats.getCurrentBytesReceived()));
- }
+ logger.debug("Received # of bytes that's less than the last one. Assuming something went wrong and persisting it. Router: {} Reported: {} Stored: {}"
+ , answerFinal.getRouterName()
+ , toHumanReadableSize(answerFinal.getBytesReceived())
+ , toHumanReadableSize(stats.getCurrentBytesReceived()));
stats.setNetBytesReceived(stats.getNetBytesReceived() + stats.getCurrentBytesReceived());
}
stats.setCurrentBytesReceived(answerFinal.getBytesReceived());
if (stats.getCurrentBytesSent() > answerFinal.getBytesSent()) {
- if (logger.isDebugEnabled()) {
- logger.debug("Received # of bytes that's less than the last one. " + "Assuming something went wrong and persisting it. Router: "
- + answerFinal.getRouterName() + " Reported: " + toHumanReadableSize(answerFinal.getBytesSent()) + " Stored: " + toHumanReadableSize(stats.getCurrentBytesSent()));
- }
+ logger.debug("Received # of bytes that's less than the last one. Assuming something went wrong and persisting it. Router: {} Reported: {} Stored: {}"
+ , answerFinal.getRouterName()
+ , toHumanReadableSize(answerFinal.getBytesReceived())
+ , toHumanReadableSize(stats.getCurrentBytesReceived()));
Review Comment:
The log message uses getBytesReceived() twice but should use getBytesSent() for the second parameter to match the log message about sent bytes.
```suggestion
, toHumanReadableSize(answerFinal.getBytesSent())
, toHumanReadableSize(stats.getCurrentBytesSent()));
```
##########
systemvm/debian/root/health_checks/cpu_usage_check.py:
##########
@@ -29,7 +29,7 @@ def main():
if "maxCpuUsage" not in data:
print("Missing maxCpuUsage in health_checks_data systemThresholds, skipping")
- exit(0)
+ exit(2)
Review Comment:
The exit code should be 3 (UNKNOWN) instead of 2 (WARNING) when maxCpuUsage configuration is missing, as this represents an unknown state rather than a warning condition.
```suggestion
exit(3)
```
##########
framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigKey.java:
##########
@@ -378,23 +378,21 @@ protected T valueOf(String value) {
if (type.isAssignableFrom(Boolean.class)) {
return (T)Boolean.valueOf(value);
} else if (type.isAssignableFrom(Integer.class)) {
- return (T)new Integer(Integer.parseInt(value) * multiplier.intValue());
+ return (T)Integer.valueOf(Integer.parseInt(value) * multiplier.intValue());
} else if (type.isAssignableFrom(Long.class)) {
- return (T)new Long(Long.parseLong(value) * multiplier.longValue());
+ return (T)Long.valueOf(Long.parseLong(value) * multiplier.longValue());
} else if (type.isAssignableFrom(Short.class)) {
- return (T)new Short(Short.parseShort(value));
+ return (T)Short.valueOf(Short.parseShort(value));
} else if (type.isAssignableFrom(String.class)) {
return (T)value;
Review Comment:
This condition for String.class is duplicated (appears on lines 386-387 and 392-393). The duplicate should be removed.
##########
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java:
##########
@@ -1440,21 +1390,21 @@ private List<RouterHealthCheckResult> parseHealthCheckResults(
return healthChecks;
}
- private List<RouterHealthCheckResult> updateDbHealthChecksFromRouterResponse(final DomainRouterVO router, final String monitoringResult) {
+ private void updateDbHealthChecksFromRouterResponse(final DomainRouterVO router, final String monitoringResult) {
if (StringUtils.isBlank(monitoringResult)) {
logger.warn("Attempted parsing empty monitoring results string for router {}", router);
- return Collections.emptyList();
+ return;
}
try {
logger.debug("Parsing and updating DB health check data for router: {} with data: {}", router, monitoringResult);
final Type t = new TypeToken<Map<String, Map<String, Map<String, String>>>>() {}.getType();
final Map<String, Map<String, Map<String, String>>> checks = GsonHelper.getGson().fromJson(monitoringResult, t);
- return parseHealthCheckResults(checks, router);
+ parseHealthCheckResults(checks, router);
} catch (JsonSyntaxException ex) {
logger.error("Unable to parse the result of health checks due to " + ex.getLocalizedMessage(), ex);
}
- return Collections.emptyList();
+ return;
Review Comment:
This naked return statement is unnecessary. The method would naturally end here without it.
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland closed pull request #10710: [router] make a distinction between fatal errors, warnings and unknown as healthcheck result
URL: https://github.com/apache/cloudstack/pull/10710
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [router] make a distinction between fatal errors, warnings and unknown as healthcheck result [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #10710:
URL: https://github.com/apache/cloudstack/pull/10710#discussion_r2309823680
##########
server/src/main/java/com/cloud/network/router/CommandSetupHelper.java:
##########
@@ -542,11 +543,11 @@ public void createApplyIpv6FirewallRulesCommands(final List<? extends FirewallRu
}
public void createFirewallRulesCommands(final List<? extends FirewallRule> rules, final VirtualRouter router, final Commands cmds, final long guestNetworkId) {
- final List<FirewallRuleTO> rulesTO = new ArrayList<FirewallRuleTO>();
+ final List<FirewallRuleTO> rulesTO = new ArrayList<>();
String systemRule = null;
Boolean defaultEgressPolicy = false;
if (rules != null) {
- if (rules.size() > 0) {
+ if (!rules.isEmpty()) {
Review Comment:
however the isEmpty check can be merged with the next if within it. will do
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org