Skip to content

LB: per-rule keepalive and timeouts, plus three HAProxy config fixes - #14131

Open
bhouse-nexthop wants to merge 13 commits into
apache:mainfrom
bhouse-nexthop:haproxy-keepalive
Open

bhouse-nexthop wants to merge 13 commits into
apache:mainfrom
bhouse-nexthop:haproxy-keepalive

Conversation

@bhouse-nexthop

@bhouse-nexthop bhouse-nexthop commented Sep 10, 2026 •

Copy link
Copy Markdown
Collaborator

Description

#12586 made the load balancer idle timeout globally configurable, as
network.loadbalancer.haproxy.idle.timeout. This expands that to per load balancer rule, and
does the same for keepalive, which until now could only be set on the network offering.

Where these settings live today:

setting scope set where reaches haproxy as
keepalive network offering keepaliveenabled, create-time only, no UI option httpclose in defaults
idle timeout global network.loadbalancer.haproxy.idle.timeout (#12574, #12586) timeout client, timeout server in defaults

Both land in the defaults section, so they apply to every LB rule on every network using that
offering. One rule cannot differ from another on the same router.

New parameters on createLoadBalancerRule and updateLoadBalancerRule

Each is optional, and each writes into that rule's own listen section, which overrides defaults:

parameter type haproxy directive applies to unset means
keepalive boolean option http-keep-alive, else option httpclose rules served in http mode the offering's keepaliveenabled
idletimeout long, ms timeout client and timeout server every rule, tcp and http the global from #12586
keepalivetimeout long, ms timeout http-keep-alive http mode, keepalive on idletimeout applies

idletimeout keeps the global's meaning, including 0 for infinite.

What that produces, taken from the generator - an http rule with all three set, a tcp rule with
only an idle timeout, and a rule that set nothing:

listen 10_2_0_2-80
	bind 10.2.0.2:80
	mode http
	option http-keep-alive
	timeout http-keep-alive 15000
	timeout client     600000
	timeout server     600000
	balance roundrobin
	server 10_2_0_2-80_0 10.1.10.3:80 check

listen 10_2_0_3-3306
	bind 10.2.0.3:3306
	timeout client     600000
	timeout server     600000
	balance roundrobin
	server 10_2_0_3-3306_0 10.1.10.4:3306 check

listen 10_2_0_4-8080
	bind 10.2.0.4:8080
	balance roundrobin
	server 10_2_0_4-8080_0 10.1.10.5:8080 check

The three values are returned on listLoadBalancerRules, and are settable on the load balancer
rule's edit form in the UI, where blank reads as "Use default".

A negative timeout is rejected by the API. Haproxy treats one as a fatal parse error, which would
leave every rule on that router serving its previous config, so the generator drops it as well.

Stored as firewall_rule_details. No schema change.

Applies to public LB rules. Application load balancers are created through a different command and
are not covered.

What this makes possible

A rule on port 80 can now have keep-alive and X-Forwarded-For. It could not before:

keepalive mode connection reuse XFF
offering flag off (the default) mode http no, Connection: close per response yes
offering flag on mode tcp yes no, forwardfor needs http mode
per rule, this PR mode http yes yes

A rule that sets nothing keeps today's behaviour exactly, including the fall back to mode tcp
when the offering flag is on.

Also fixes three bugs in the same file

1. option forceclose is not a valid keyword

With keepalive enabled, a rule that also uses HTTP-based stickiness or SSL offload emits
no option forceclose. HAProxy has rejected that keyword since 2.0, and the system VM is Debian 12
(HAProxy 2.6):

[ALERT] config : parsing [haproxy.cfg:22]: option 'forceclose' is not supported any more
        since HAProxy 2.0, please just remove it, or use 'option httpclose' if absolutely
        needed.
[ALERT] config : Fatal errors found in configuration.

The whole file is rejected, so haproxy keeps serving the previous config and the rule silently
never takes effect. Replaced with option http-keep-alive.

2. One rule's settings leak into every rule generated afterwards

globalSection and defaultsSection are static String[]. The generator wraps them in
Arrays.asList(...) and calls set(), which writes through to the arrays, so the value
survives the call:

set once by leaks as
keepalive on any rule no option httpclose in the defaults for every later rule, any network, any account
idleTimeout > 0 that timeout becomes the default for the next rule
idleTimeout == 0 timeouts blanked permanently

The generator now copies into a fresh list, and the sections themselves became List.of(...), so a
future set() throws instead of silently corrupting every config the agent builds afterwards.
Output is unchanged for a single rule.

3. appsession was removed from HAProxy in 1.6

An LB rule with AppCookie stickiness emitted appsession, which the shipped haproxy refuses:

[ALERT] config : 'appsession' is not supported anymore since HAProxy 1.6.
[ALERT] config : Fatal errors found in configuration.

Same blast radius as bug 1 - the file is rejected, so every rule on that router keeps serving its
previous config. The VR advertises AppCookie as a supported stickiness method, so this is
reachable from the API.

Replaced with the documented stick table equivalent:

appsession replacement
len <n> stick-table type string len <n> size 10k
timeout <h> expire <h>
the cookie stick store-response res.cook(<name>) + stick match req.cook(<name>)
request-learn stick store-request req.cook(<name>)
prefix, mode no equivalent, logged and ignored

prefix and mode were never applied - the directive carrying them was rejected - so no working
behaviour changes.

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

How Has This Been Tested?

HAProxyConfiguratorTest Tests run: 17, Failures: 0 and LoadBalancingRulesManagerImplTest
Tests run: 16, Failures: 0.

Each bug fix has a case that fails without it:

  • keepalive with SSL offload emits no forceclose, and does emit option http-keep-alive
  • a keepalive config followed by a non-keepalive one leaves no no option httpclose behind
  • an idle timeout followed by an unset one falls back to the built-in default
  • AppCookie stickiness emits a stick table and never appsession

The rest cover the per-rule settings:

  • a rule that sets keepalive stays in http mode
  • a rule's keepalive wins over the offering, in both directions
  • a rule that sets nothing is byte for byte what it was before
  • timeout http-keep-alive is emitted, and only when keepalive is on
  • idletimeout overrides per rule on a tcp rule, leaving the defaults section alone
  • a negative timeout never reaches the config, and is rejected at the API
  • two rules in one config do not pick up each other's settings

Generated configs are checked against a real haproxy -c, which is where the alert text above
comes from. A file holding an AppCookie rule, an http rule with all three settings, a tcp rule
with a timeout, and a rule with a negative timeout is fatal before these changes and valid
after
- the three alerts above become zero.

@codecov

codecov Bot commented Sep 10, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 51.11111% with 88 lines in your changes missing coverage. Please review.
✅ Project coverage is 20.79%. Comparing base (345db51) to head (39efd7c).
⚠️ Report is 411 commits behind head on main.

Files with missing lines Patch % Lines
...a/com/cloud/network/router/CommandSetupHelper.java 0.00% 18 Missing ⚠️
...in/java/com/cloud/network/HAProxyConfigurator.java 70.73% 9 Missing and 3 partials ⚠️
...ork/router/VirtualNetworkApplianceManagerImpl.java 0.00% 12 Missing ⚠️
...d/user/loadbalancer/CreateLoadBalancerRuleCmd.java 0.00% 10 Missing ⚠️
...in/java/com/cloud/agent/api/to/LoadBalancerTO.java 50.00% 9 Missing ⚠️
.../cloudstack/api/response/LoadBalancerResponse.java 0.00% 9 Missing ⚠️
...src/main/java/com/cloud/api/ApiResponseHelper.java 10.00% 9 Missing ⚠️
...d/user/loadbalancer/UpdateLoadBalancerRuleCmd.java 33.33% 8 Missing ⚠️
...loud/network/lb/LoadBalancingRulesManagerImpl.java 98.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #14131      +/-   ##
============================================
+ Coverage     18.07%   20.79%   +2.72%     
- Complexity    16357    21973    +5616     
============================================
  Files          5937     6496     +559     
  Lines        537029   591824   +54795     
  Branches      65833    72617    +6784     
============================================
+ Hits          97043   123097   +26054     
- Misses       428901   455369   +26468     
- Partials      11085    13358    +2273     
Flag Coverage Δ
uitests 3.88% <ø> (-0.16%) ⬇️
unittests 22.11% <51.11%> (+2.96%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bhouse-nexthop
bhouse-nexthop marked this pull request as draft September 10, 2026 15:03
@bhouse-nexthop
bhouse-nexthop changed the base branch from 4.22 to main September 10, 2026 15:10
@bhouse-nexthop bhouse-nexthop changed the title Fix two bugs in the virtual router's HAProxy config LB: per-rule keepalive and timeouts, plus two HAProxy config fixes Sep 10, 2026
@bhouse-nexthop
bhouse-nexthop marked this pull request as ready for review September 10, 2026 16:04
@bhouse-nexthop bhouse-nexthop changed the title LB: per-rule keepalive and timeouts, plus two HAProxy config fixes LB: per-rule keepalive and timeouts, plus three HAProxy config fixes Sep 10, 2026
@weizhouapache weizhouapache added this to the 24.0 milestone Sep 10, 2026
@weizhouapache

Copy link
Copy Markdown
Member

@bhouse-nexthop
could you make some changes for VR health check ?
similar to #12596 which fixes #12591

@boring-cyborg boring-cyborg Bot added component:virtual-router Python Warning... Python code Ahead! labels Sep 10, 2026
@bhouse-nexthop

Copy link
Copy Markdown
Collaborator Author

@weizhouapache good catch, thanks - added in the latest commit.

haproxy_check.py knew about maxconn and the global idle timeout, so the new per rule settings
could drift with nothing noticing. The health check data now carries all three, and the check
validates them in that rule's own listen section:

data checked
ruleKeepAlive option http-keep-alive, or option httpclose
ruleIdleTimeout timeout client, timeout server
ruleKeepAliveTimeout timeout http-keep-alive

The http mode test needed widening as well, otherwise a rule that sets keepalive itself stopped
being checked at all:

before   port 80 and the offering has keepalive off
after    port 80 and (the rule sets keepalive, or the offering has it off)

Compatibility: an older management server sends none of these keys, and a rule that inherits sends
them empty. Both read as nothing to check, so an updated system VM template against an older
management server still passes.

I ran the script against generated configs rather than only reading it - a matching config passes,
and each of a wrong keepalive option, a drifted timeout client and a missing
timeout http-keep-alive fails.

@Damans227 Damans227 left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @bhouse-nexthop, the changes look right. CLGTM.

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19626

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan test

With keepalive enabled, an LB rule that also uses HTTP stickiness or SSL
offload emitted "no option forceclose". HAProxy has rejected that keyword
since 2.0, and the system VM has shipped 2.x for several releases:

  [ALERT] config : parsing [haproxy.cfg:22]: option 'forceclose' is not
  supported any more since HAProxy 2.0, please just remove it, or use
  'option httpclose' if absolutely needed.

It is a fatal parse error, so haproxy keeps running the previous config
and the rule silently never takes effect.

Replaced with "option http-keep-alive", which says the same thing and is
valid on every version the system VM has shipped.

Signed-off-by: Brad House <[email protected]>
The global and defaults sections are static arrays. Wrapping them in
Arrays.asList() and calling set() writes through to the array, so a value
from one LB rule stays there for every config generated afterwards:

  - keepalive on any rule leaves "no option httpclose" in the defaults
    for every later rule, in any network, in any account
  - an idle timeout from one rule becomes the default for the next
  - idleTimeout 0 blanks the timeouts permanently

Copy the arrays instead. Same output, no shared state.

Signed-off-by: Brad House <[email protected]>
An LB rule with AppCookie stickiness emitted "appsession", which haproxy
has refused since 1.6:

  [ALERT] config : 'appsession' is not supported anymore since HAProxy 1.6.
  [ALERT] config : Fatal errors found in configuration.

The whole file is rejected, so every rule on that router keeps serving
its previous config, not just the one with the policy. The VR advertises
AppCookie as supported, so this is reachable from the API.

A stick table on the cookie is the documented replacement:

  stick-table type string len <length> size 10k expire <holdtime>
  stick store-response res.cook(<name>)
  stick match req.cook(<name>)
  stick store-request req.cook(<name>)      # only with request-learn

The prefix and mode options have no equivalent and are now logged and
ignored. They were never applied - the directive carrying them was
rejected - so no working behaviour changes.

Signed-off-by: Brad House <[email protected]>
apache#12586 made the idle timeout global. Keepalive is set on the network
offering, at create time only, with no UI. Both land in the haproxy
defaults section, so every rule on a router shares them.

Three optional parameters on createLoadBalancerRule and
updateLoadBalancerRule, each written to that rule's own listen section:

  | parameter        | haproxy directive                    | unset      |
  | keepalive        | option http-keep-alive / httpclose   | offering   |
  | idletimeout      | timeout client, timeout server       | global     |
  | keepalivetimeout | timeout http-keep-alive              | idletimeout|

Held as firewall rule details, so no schema change.

A rule on port 80 can now have keepalive and X-Forwarded-For together,
which no combination of the existing settings can produce:

  offering flag off   mode http   closes per response   XFF works
  offering flag on    mode tcp    reuses connections    XFF lost
  per rule            mode http   reuses connections    XFF works

A rule that sets nothing behaves exactly as before, including the fall
back to tcp mode when the offering flag is on.

Negative timeouts are rejected at the API and dropped in the generator.
Haproxy treats one as a fatal parse error, which would strand every rule
on the router.

Applies to public LB rules. Application load balancers are created
through a different command and are not covered.

Signed-off-by: Brad House <[email protected]>
haproxy_check.py compares the router's haproxy.cfg against what the
management server thinks it configured. It knew about maxconn and the
global idle timeout, so a rule's own keepalive and timeouts could drift
without anything noticing.

The health check data now carries the three per rule values, empty when
the rule inherits, and the check validates them in that rule's listen
section:

  ruleKeepAlive         option http-keep-alive, or option httpclose
  ruleIdleTimeout       timeout client, timeout server
  ruleKeepAliveTimeout  timeout http-keep-alive

The http mode test needed widening too. It read the offering flag alone,
so a rule that sets keepalive itself stopped being checked at all:

  before   port 80 and the offering has keepalive off
  after    port 80 and (the rule sets keepalive, or the offering has it off)

An older management server sends none of these keys, and a rule that sets
nothing sends them empty. Both read as nothing to check.

Signed-off-by: Brad House <[email protected]>
lb was looked up by that same id and is non null by this point, so the
Long unboxes safely. The comment claiming otherwise was wrong, and the
id now matches the one applyLoadBalancerConfig gets a few lines later.

Signed-off-by: Brad House <[email protected]>
Once a rule had its own keepalive or timeout there was no way back to
inheriting. Leaving a parameter out keeps the current value, and there
is no value that means unset: negative timeouts are refused, and
keepalive is a boolean.

updateLoadBalancerRule takes cleanupconnectionsettings, after the
cleanupdetails flag on updateVirtualMachine and updateTemplate. It drops
all three of the rule's own settings, then sets again any passed in the
same call, so one call can reset one and keep the rest:

  rule holds        call                                  rule holds after
  idletimeout=5000  cleanupconnectionsettings=true        nothing
  keepalive=true
  idletimeout=5000  cleanupconnectionsettings=true        idletimeout=5000
  keepalive=true      idletimeout=5000
  idletimeout=5000  keepalivetimeout=2000                 idletimeout=5000
                                                          keepalivetimeout=2000

A change is now worked out by comparing the settings before and after,
so a cleanup on a rule that holds none does not reapply the config.

The update parameter descriptions said an unset value inherits, which is
only true on create. They now say it keeps the current value.

Signed-off-by: Brad House <[email protected]>
Clearing keepalive, idle timeout or keepalive timeout in the edit rule
dialog dropped the parameter, which the API reads as keep the current
value, so the old setting stayed. When a field the rule had set is now
blank the dialog sends cleanupconnectionsettings, along with the fields
still filled in.

Signed-off-by: Brad House <[email protected]>
When applying an updated rule to the router fails, updateLoadBalancerRule
puts back the name, description, algorithm and cidrs, but not the
keepalive and timeouts. Those stayed in the database while the router
still ran the old values, and the next successful apply of anything on
that router would quietly push them.

The settings held before the update are now restored along with the
rest.

They were also written before the provider validated the rule, so a
rule it rejected kept the new settings. The other fields only reach the
database at _lbDao.update, after validation; the settings are now
written at the same point.

Signed-off-by: Brad House <[email protected]>
The keepalive, idle timeout and keepalive timeout parameters, and the
cleanupconnectionsettings flag, were marked since 4.23.0. That release
shipped without them, and main is now 24.0.0-SNAPSHOT following the
vote to drop the "4." prefix starting with release 24.

Signed-off-by: Brad House <[email protected]>
updateLoadBalancerRule now passes the command's id to the connection
settings, which take a long. UpdateLoadBalancerTest built its command
without an id, so the null unboxed and testValidateRuleBeforeUpdateLB
threw. The API marks id as required, so only the test could get there.

Set the id on the command and stub the DAO calls with it, and mock the
firewall rule details DAO the settings are read from. testRuleNotValidated
only passed before because the null id missed the lookup and raised the
same exception type. It now finds the rule and fails on validation, which
is what it means to test.

Signed-off-by: Brad House <[email protected]>
@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19684

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

@DaanHoogland DaanHoogland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clgtm

@blueorangutan

Copy link
Copy Markdown

[SF] Trillian Build Failed (tid-17222)

@blueorangutan

Copy link
Copy Markdown

[SF] Trillian test result (tid-17228)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 61697 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr14131-t17228-kvm-ol8.zip
Smoke tests completed. 156 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_04_dhcp_dns_userdata Error 7.36 test_network_extension_namespace.py

updateLoadBalancerRule called lb.getLbProtocol().equals(...) after
applying the config. lb_protocol is null for a rule created through the
API without a protocol, so the update threw an NPE and reported failure
although the change had already been applied to the router. Stock hits
this on an algorithm or CIDR change; with the per-rule connection
settings every keepalive or timeout update on such a rule hits it.

Signed-off-by: Brad House <[email protected]>
updateLoadBalancerRule took its result from _lbDao.update(), which
returns false when no column of the rule changed. The keepalive and
timeout settings are kept in firewall_rule_details, so an update that
only changed them applied the new config to the router and then failed
with "Failed to update load balancer rule". Count a settings change as
an update. An update that changes nothing still fails, as before.

Signed-off-by: Brad House <[email protected]>
@bhouse-nexthop

Copy link
Copy Markdown
Collaborator Author

Test report: per-rule keepalive and timeouts, and the HAProxy fixes, on running virtual routers

Environment

  • A CloudStack 24.0 (main) build with this PR applied, alongside some of my other open PRs. This PR targets main, so no backport caveat applies.
  • Three KVM hosts on Ubuntu 24.04, nested virtualisation; Ceph RBD primary storage.
  • System VM template 4.22.0 (Debian 12, HAProxy 2.6.12). The routers were created on this build. Their configure.py and /root/health_checks/haproxy_check.py match the build's.
  • Two isolated networks:
    • N1 on DefaultIsolatedNetworkOfferingWithSourceNatService, with backends B1 and B2 (Ubuntu 24.04, nginx). nginx logs X-Forwarded-For, $connection and $connection_requests, and sets JSESSIONID=sess-<name>.
    • N2 on a copy of that offering with keepaliveenabled=true, with backend B3.
  • I moved N2's router onto N1's KVM host, so that one agent JVM generated both routers' configs.
  • The client was a host on the public network.

1. An http rule with all three settings

createLoadBalancerRule ... publicport=80 keepalive=true idletimeout=600000 keepalivetimeout=15000, with B1 and B2:

listen <P1>-80
	bind <P1>:80
	mode http
	option http-keep-alive
	timeout http-keep-alive 15000
	timeout client     600000
	timeout server     600000
	balance roundrobin
	server <P1>-80_0 <B2>:80 check
	server <P1>-80_1 <B1>:80 check
  • haproxy -c reports the file valid.
  • listLoadBalancerRules returns keepalive=true, idletimeout=600000, keepalivetimeout=15000.
  • curl -v with four URLs on one command line: Connected, then Re-using existing connection three times. The answers alternate B2, B1, B2, B1.
  • nginx on each backend shows the client address in XFF, and two requests on one backend connection: xff="<client>" conn=2 reqs=1 and then conn=2 reqs=2.

So keep-alive and X-Forwarded-For work together on one rule. They could not before.

2. A tcp rule with only an idle timeout (publicport=3306 idletimeout=120000)

listen <P1>-3306
	bind <P1>:3306
	timeout client     120000
	timeout server     120000
	balance roundrobin
	server <P1>-3306_0 <B1>:3306 check

3. Rules that set nothing render as before:

  • Port 80: mode http, option httpclose, balance, server lines. The client gets connection: close on every response.
  • Port 8080: bind, balance, server only.
  • The defaults section keeps option httpclose and timeout client/server 50000.

4. no option forceclose is gone (keepalive with LbCookie stickiness)

  • On rule 1 (per-rule keepalive), and on an N2 rule (keepalive from the offering, which is the old trigger), I added an LbCookie policy.
  • Both render mode http, option http-keep-alive, the cookie ... insert line and the per-server cookies.
  • grep -c forceclose is 0, and haproxy -c passes.
  • haproxy is listening, and the haproxy cookie pins the client: four requests with it all go to the same backend.
  • For comparison, the same file with no option forceclose in place of option http-keep-alive fails haproxy -c:
    [ALERT] config : parsing [/tmp/fc.cfg:36]: option 'forceclose' is not supported any more since HAProxy 2.0 ...
    [ALERT] config : Fatal errors found in configuration.
    

5. AppCookie renders as a stick table.

Since #13315 (merged 10-21; this PR's branch does not include it yet), createLBStickinessPolicy methodname=AppCookie is refused for Virtual Router networks:

AppCookie based stickiness (HAProxy 'appsession') is not supported on a Virtual Router, as this directive was removed in HAProxy 1.6

So the generator path is reached only by an AppCookie policy stored before an upgrade. To test that case, I inserted one (cookie-name=JSESSIONID&length=52&holdtime=3h) into load_balancer_stickiness_policies for a port 80 rule. Then I regenerated the config by editing another rule on the network.

	stick-table type string len 52 size 10k expire 3h
	stick store-response res.cook(JSESSIONID)
	stick match req.cook(JSESSIONID)
  • There is no appsession, and haproxy -c passes.
  • Six requests with Cookie: JSESSIONID=sess-B1 all go to B1, and six with sess-B2 all go to B2. Without a cookie they alternate.
  • show table holds both keys.
  • The same file with appsession JSESSIONID len 52 timeout 3h fails haproxy -c with 'appsession' is not supported anymore since HAProxy 1.6.

The PR description says AppCookie "is reachable from the API". That is no longer true on main because of #13315. The fix still matters for policies stored before that change.

6. No leak between configs from one agent

  • I regenerated N2's config (keepalive offering: its defaults has no option httpclose), and then N1's config, on the same agent.
  • N1's defaults still has option httpclose, and its port 80 rule without settings still has option httpclose.
  • The 600000 and 120000 rule timeouts never reach either defaults, which stays at 50000.

7. Health check

  • getRouterHealthCheckResults performfreshchecks=true: haproxy_check.py reports All checks pass.
  • I changed timeout client 45000 to 45001 by hand in the rule's listen section. The next fresh check failed with:
    Expected 'timeout client 45000' in listen <P1>-80 but found 45001
    
  • A real re-apply (an algorithm change on another rule) rewrote the file, and the check passed again.

8. Back to defaults

  • updateLoadBalancerRule cleanupconnectionsettings=true returns success, and the three fields come back empty (retested on the fixed build, see below). The rule renders mode http / option httpclose, with the LbCookie lines kept.
  • With cleanupconnectionsettings=true keepalive=true, the update returns success and only keepalive remains: option http-keep-alive, no timeouts. Connection reuse works again.

9. Validation
idletimeout=-1 is refused on create (idletimeout must be 0 or greater, got [-1]. 0 means no timeout.) and on update. keepalivetimeout=-1 is refused on update with the same wording.

10. Rollback when the router update fails
I reran this step on the build with both update fixes, so that only an unreachable router can make the update fail.

  • updateLoadBalancerRule idletimeout=30000 keepalivetimeout=20000 returned success.
  • I stopped the agent of the router's host, so the update could not reach the router.
  • updateLoadBalancerRule idletimeout=45000 keepalivetimeout=25000 then failed with Failed to update load balancer rule.
  • listLoadBalancerRules and firewall_rule_details still held the old 30000 / 20000, and the management server logged LB Rollback rule.
  • With the agent back, the same update returned success, and the router shows timeout client/server 45000 and timeout http-keep-alive 25000.

The test plan's first idea for this step was chattr +i /etc/haproxy/haproxy.cfg on the router. That does not fail the update: the router logs Could not copy /etc/haproxy/haproxy.cfg.new to /etc/haproxy/haproxy.cfg, reloads the old file and reports success. So the management server has nothing to roll back. That is router-side behaviour, unchanged by this PR.

Result: pass.

  • Every generated config passes haproxy -c and is live.
  • Keep-alive and XFF work together.
  • Rules without settings render as before.
  • There is no leak between configs.
  • The health check notices drift.
  • Reset to defaults and rollback behave as described.

Two update bugs the lab found, both fixed in the PR

The first run found two ways updateLoadBalancerRule returned job result 530 although the change had been applied:

  • A rule created through the API without protocol has a NULL lb_protocol, and the SSL check after the apply threw an NPE on it. The PR now compares null-safely.
  • An update that changes only connection settings failed with Failed to update load balancer rule. These settings live in firewall_rule_details, so no column of the rule changed and the DAO update reported no row updated. The PR now treats a settings change as a successful update.

I retested on a build with both fixes, on two rules: one created without protocol, and one with protocol=tcp as the UI sends it. Every update below returned success. listLoadBalancerRules showed the new values, the router's listen section had them, and haproxy -c passed after each.

Rule Update Router listen section after it
no protocol keepalive=true idletimeout=600000 keepalivetimeout=15000 option http-keep-alive, timeout http-keep-alive 15000, timeout client/server 600000
no protocol idletimeout=120000 timeout client 120000, keep-alive timeout kept at 15000
no protocol keepalivetimeout=20000 timeout http-keep-alive 20000
no protocol algorithm=leastconn balance leastconn, timeouts kept
no protocol cleanupconnectionsettings=true no timeouts, no http-keep-alive
no protocol cleanupconnectionsettings=true keepalive=true option http-keep-alive only
no protocol idletimeout=45000 timeout client 45000, keep-alive kept
protocol=tcp idletimeout=90000 timeout client/server 90000
protocol=tcp keepalive=true timeouts kept
protocol=tcp keepalivetimeout=12000 timeouts kept
protocol=tcp cleanupconnectionsettings=true no timeouts
protocol=tcp algorithm=source idletimeout=70000 balance source, timeout client/server 70000
  • The management server log has no getLbProtocol / NullPointerException line over the retest.
  • Requests through the rule still alternate between both backends.
  • An update that names only the rule id and changes nothing still fails with Failed to update load balancer rule, as on stock. That is the only Failed to update line in the log.

Not covered

  • The UI edit form (blank = "Use default", clearing a field sends cleanupconnectionsettings). I had no browser on the lab network.
  • SSL offload rules, and the tcp-proxy protocol.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants