HAProxy Configuration: network.loadbalancer.haproxy.idle.timeout#12586
HAProxy Configuration: network.loadbalancer.haproxy.idle.timeout#12586bradh352 wants to merge 4 commits intoapache:4.22from
Conversation
boring-cyborg
bot
added
component:api
component:networking
component:orchestration
component:virtual-router
Python
bradh352
force-pushed
the
haproxy_timeout
branch
5 times, most recently
from
bc72961 to
f5216af
Compare
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 4.22 #12586 +/- ##
============================================
+ Coverage 17.62% 17.94% +0.31%
- Complexity 15668 16166 +498
============================================
Files 5917 5939 +22
Lines 531255 533029 +1774
Branches 64951 65220 +269
============================================
+ Hits 93639 95657 +2018
+ Misses 427077 426642 -435
- Partials 10539 10730 +191
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR adds a new global configuration parameter network.loadbalancer.haproxy.idle.timeout to control HAProxy's timeout client and timeout server directives in the defaults section. The default value is 50000 milliseconds (matching the current hardcoded value), and setting it to 0 removes the timeout directives entirely for infinite timeout. This addresses issue #12574 where the aggressive 50-second timeout was problematic for long-running database connections through load balancers.
Changes:
- Added
NETWORK_LB_HAPROXY_IDLE_TIMEOUTconfiguration key with default value of 50000ms - Updated
LoadBalancerConfigCommandto includeidleTimeoutparameter - Modified HAProxyConfigurator to generate timeout directives based on the configuration value
- Added health check validation for the idle timeout configuration
- Updated all LoadBalancerConfigCommand instantiations across VR, internal LB, and elastic LB implementations
- Added comprehensive unit tests for timeout configuration generation
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java | Defines the new configuration key for HAProxy idle timeout |
| engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java | Registers the new configuration key in the configurable keys array |
| core/src/main/java/com/cloud/agent/api/routing/LoadBalancerConfigCommand.java | Adds idleTimeout field and updates constructor signature |
| core/src/main/java/com/cloud/network/HAProxyConfigurator.java | Implements logic to set or remove timeout directives based on idleTimeout value |
| server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java | Passes idle timeout value to load balancing data for virtual routers |
| server/src/main/java/com/cloud/network/router/CommandSetupHelper.java | Updates LoadBalancerConfigCommand instantiation with idle timeout value |
| plugins/network-elements/internal-loadbalancer/src/main/java/org/apache/cloudstack/network/lb/InternalLoadBalancerVMManagerImpl.java | Updates LoadBalancerConfigCommand instantiation for internal load balancers |
| plugins/network-elements/elastic-loadbalancer/src/main/java/com/cloud/network/lb/ElasticLoadBalancerManagerImpl.java | Updates LoadBalancerConfigCommand instantiation for elastic load balancers |
| systemvm/debian/root/health_checks/haproxy_check.py | Adds health check validation for idle timeout configuration (contains bugs) |
| core/src/test/java/com/cloud/network/HAProxyConfiguratorTest.java | Adds tests for timeout configuration with 0 and non-zero values |
| core/src/test/java/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResourceTest.java | Updates test fixtures with idle timeout parameter |
| core/src/test/java/com/cloud/agent/resource/virtualnetwork/ConfigHelperTest.java | Updates test fixtures with idle timeout parameter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16705 |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-15382)
|
|
hrm, pretty sure those failures can't be related to this PR |
nah |
|
Should I rebase this for v4.22 so it gets included in the next patch? It would be really useful to us if we could start using the official releases. I think the other PRs I opened that we're running patches for have merged and will be in the next release. |
|
@bhouse-nexthop , you can
or even to 4.20 if you wish, We will merge forwards ad call you in if that gives any issues. |
copilot suggestion Co-authored-by: Copilot <[email protected]>
copilot suggestion Co-authored-by: Copilot <[email protected]>
bradh352
force-pushed
the
haproxy_timeout
branch
from
d30e056 to
1e567c0
Compare
|
Ok, its moved to 4.22 now... |
4 participants
Description
This PR adds a new configuration parameter of
network.loadbalancer.haproxy.idle.timeoutwhich controls the HAProxytimeout serverandtimeout clientconfiguration parameters under thedefaultssection.Fixes #12574
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?
It has been observed that this will not update dynamically unless some other load balancer configuration is changed (or the VR is restarted). It is not clear if there is any infrastructure to 'trigger' a push to all existing load balancers to regenerate their configuration since this is global.