From fe1039034ffecba73b7d4e49cf0fd43dc7ecc7a4 Mon Sep 17 00:00:00 2001 From: Brad House Date: Thu, 10 Sep 2026 14:35:42 +0000 Subject: [PATCH 1/5] Use a keepalive option HAProxy still accepts 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 --- .../java/com/cloud/network/HAProxyConfigurator.java | 2 +- .../com/cloud/network/HAProxyConfiguratorTest.java | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/com/cloud/network/HAProxyConfigurator.java b/core/src/main/java/com/cloud/network/HAProxyConfigurator.java index b3f7da1c6d82..2f5f5fb9d729 100644 --- a/core/src/main/java/com/cloud/network/HAProxyConfigurator.java +++ b/core/src/main/java/com/cloud/network/HAProxyConfigurator.java @@ -573,7 +573,7 @@ private List getRulesForPool(final LoadBalancerTO lbTO, final LoadBalanc boolean http = (publicPort == NetUtils.HTTP_PORT && !keepAliveEnabled); if (http || httpbasedStickiness || sslOffloading) { frontendConfigs.add("\tmode http"); - String keepAliveLine = keepAliveEnabled ? "\tno option forceclose" : "\toption httpclose"; + String keepAliveLine = keepAliveEnabled ? "\toption http-keep-alive" : "\toption httpclose"; frontendConfigs.add(keepAliveLine); } diff --git a/core/src/test/java/com/cloud/network/HAProxyConfiguratorTest.java b/core/src/test/java/com/cloud/network/HAProxyConfiguratorTest.java index 073f976719b5..211e85d1fe2d 100644 --- a/core/src/test/java/com/cloud/network/HAProxyConfiguratorTest.java +++ b/core/src/test/java/com/cloud/network/HAProxyConfiguratorTest.java @@ -158,6 +158,19 @@ public void generateConfigurationTestWithSslCert() { Assert.assertTrue(result.contains("bind 10.2.0.1:443 ssl crt /etc/cloudstack/ssl/10_2_0_1-443.pem")); } + @Test + public void generateConfigurationTestKeepAliveWithSslOffloading() { + LoadBalancerTO lb = new LoadBalancerTO("1", "10.2.0.1", 443, "ssl", "roundrobin", false, false, false, null); + lb.setLbSslCert(new LbSslCert("cert", "key", "password", "chain", "fingerprint", false)); + LoadBalancerTO[] lba = new LoadBalancerTO[1]; + lba[0] = lb; + HAProxyConfigurator hpg = new HAProxyConfigurator(); + LoadBalancerConfigCommand cmd = new LoadBalancerConfigCommand(lba, "10.0.0.1", "10.1.0.1", "10.1.1.1", null, 1L, "12", true, 0L); + String result = genConfig(hpg, cmd); + Assert.assertFalse("'forceclose' is rejected by HAProxy 2.0 and later", result.contains("forceclose")); + Assert.assertTrue("keepalive should be requested explicitly", result.contains("\toption http-keep-alive")); + } + private String genConfig(HAProxyConfigurator hpg, LoadBalancerConfigCommand cmd) { String[] sa = hpg.generateConfiguration(cmd); StringBuilder sb = new StringBuilder(); From a68d33e1e1a70b2587ae1a76ebf722928f928ebb Mon Sep 17 00:00:00 2001 From: Brad House Date: Thu, 10 Sep 2026 14:35:42 +0000 Subject: [PATCH 2/5] Stop one load balancer's config leaking into the next 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 --- .../cloud/network/HAProxyConfigurator.java | 29 +++++++++---------- .../network/HAProxyConfiguratorTest.java | 28 ++++++++++++++++++ 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/core/src/main/java/com/cloud/network/HAProxyConfigurator.java b/core/src/main/java/com/cloud/network/HAProxyConfigurator.java index 2f5f5fb9d729..230db113bbe8 100644 --- a/core/src/main/java/com/cloud/network/HAProxyConfigurator.java +++ b/core/src/main/java/com/cloud/network/HAProxyConfigurator.java @@ -20,7 +20,6 @@ package com.cloud.network; import java.util.ArrayList; -import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -46,13 +45,15 @@ public class HAProxyConfigurator implements LoadBalancerConfigurator { protected Logger logger = LogManager.getLogger(getClass()); private static final String blankLine = "\t "; - private static String[] globalSection = {"global", "\tlog 127.0.0.1:3914 local0 warning", "\tmaxconn 4096", "\tmaxpipes 1024", "\tchroot /var/lib/haproxy", - "\tuser haproxy", "\tgroup haproxy", "\tstats socket /run/haproxy/admin.sock", "\tdaemon"}; + // Immutable so a config cannot be built by writing into the shared copy. + private static final List globalSection = List.of("global", "\tlog 127.0.0.1:3914 local0 warning", "\tmaxconn 4096", "\tmaxpipes 1024", + "\tchroot /var/lib/haproxy", "\tuser haproxy", "\tgroup haproxy", "\tstats socket /run/haproxy/admin.sock", "\tdaemon"); - private static String[] defaultsSection = {"defaults", "\tlog global", "\tmode tcp", "\toption dontlognull", "\tretries 3", "\toption redispatch", - "\toption forwardfor", "\toption httpclose", "\ttimeout connect 5000", "\ttimeout client 50000", "\ttimeout server 50000"}; + private static final List defaultsSection = List.of("defaults", "\tlog global", "\tmode tcp", "\toption dontlognull", "\tretries 3", + "\toption redispatch", "\toption forwardfor", "\toption httpclose", "\ttimeout connect 5000", "\ttimeout client 50000", + "\ttimeout server 50000"); - private static String[] defaultListen = {"listen vmops", "\tbind 0.0.0.0:9", "\toption transparent"}; + private static final List defaultListen = List.of("listen vmops", "\tbind 0.0.0.0:9", "\toption transparent"); private static final String SSL_CERTS_DIR = "/etc/cloudstack/ssl/"; @@ -80,16 +81,16 @@ public String[] generateConfiguration(final List fwRules) final List result = new ArrayList(); - result.addAll(Arrays.asList(globalSection)); + result.addAll(globalSection); result.add(blankLine); - result.addAll(Arrays.asList(defaultsSection)); + result.addAll(defaultsSection); result.add(blankLine); if (pools.isEmpty()) { // haproxy cannot handle empty listen / frontend or backend, so add // a dummy listener // on port 9 - result.addAll(Arrays.asList(defaultListen)); + result.addAll(defaultListen); } result.add(blankLine); @@ -617,8 +618,7 @@ private String generateStatsRule(final LoadBalancerConfigCommand lbCmd, final St @Override public String[] generateConfiguration(final LoadBalancerConfigCommand lbCmd) { final List result = new ArrayList(); - final List gSection = Arrays.asList(globalSection); - // note that this is overwritten on the String in the static ArrayList + final List gSection = new ArrayList<>(globalSection); gSection.set(2, "\tmaxconn " + lbCmd.maxconn); // TODO DH: write test for this function final String pipesLine = "\tmaxpipes " + Long.toString(Long.parseLong(lbCmd.maxconn) / 4); @@ -631,7 +631,7 @@ public String[] generateConfiguration(final LoadBalancerConfigCommand lbCmd) { result.addAll(gSection); result.add(blankLine); - final List dSection = Arrays.asList(defaultsSection); + final List dSection = new ArrayList<>(defaultsSection); if (lbCmd.keepAliveEnabled) { dSection.set(7, "\tno option httpclose"); } @@ -639,8 +639,7 @@ public String[] generateConfiguration(final LoadBalancerConfigCommand lbCmd) { dSection.set(9, "\ttimeout client " + Long.toString(lbCmd.idleTimeout)); dSection.set(10, "\ttimeout server " + Long.toString(lbCmd.idleTimeout)); } else if (lbCmd.idleTimeout == 0) { - // .remove() is not allowed, only .set() operations are allowed as the list - // is a fixed size. So lets just mark the entry as blank. + // blank rather than removed, so the indexes above stay valid dSection.set(9, ""); dSection.set(10, ""); } else { @@ -695,7 +694,7 @@ public String[] generateConfiguration(final LoadBalancerConfigCommand lbCmd) { // haproxy cannot handle empty listen / frontend or backend, so add // a dummy listener // on port 9 - result.addAll(Arrays.asList(defaultListen)); + result.addAll(defaultListen); } return result.toArray(new String[result.size()]); } diff --git a/core/src/test/java/com/cloud/network/HAProxyConfiguratorTest.java b/core/src/test/java/com/cloud/network/HAProxyConfiguratorTest.java index 211e85d1fe2d..2f65d536e410 100644 --- a/core/src/test/java/com/cloud/network/HAProxyConfiguratorTest.java +++ b/core/src/test/java/com/cloud/network/HAProxyConfiguratorTest.java @@ -171,6 +171,34 @@ public void generateConfigurationTestKeepAliveWithSslOffloading() { Assert.assertTrue("keepalive should be requested explicitly", result.contains("\toption http-keep-alive")); } + @Test + public void generateConfigurationTestKeepAliveDoesNotLeakToNextConfig() { + LoadBalancerTO lb = new LoadBalancerTO("1", "10.2.0.1", 80, "http", "bla", false, false, false, null); + LoadBalancerTO[] lba = new LoadBalancerTO[1]; + lba[0] = lb; + HAProxyConfigurator hpg = new HAProxyConfigurator(); + + genConfig(hpg, new LoadBalancerConfigCommand(lba, "10.0.0.1", "10.1.0.1", "10.1.1.1", null, 1L, "12", true, 0L)); + + String result = genConfig(hpg, new LoadBalancerConfigCommand(lba, "10.0.0.1", "10.1.0.1", "10.1.1.1", null, 1L, "12", false, 0L)); + Assert.assertFalse("keepalive from an earlier config should not survive into this one", + result.contains("\tno option httpclose")); + } + + @Test + public void generateConfigurationTestIdleTimeoutDoesNotLeakToNextConfig() { + LoadBalancerTO lb = new LoadBalancerTO("1", "10.2.0.1", 80, "http", "bla", false, false, false, null); + LoadBalancerTO[] lba = new LoadBalancerTO[1]; + lba[0] = lb; + HAProxyConfigurator hpg = new HAProxyConfigurator(); + + genConfig(hpg, new LoadBalancerConfigCommand(lba, "10.0.0.1", "10.1.0.1", "10.1.1.1", null, 1L, "12", false, 1234L)); + + String result = genConfig(hpg, new LoadBalancerConfigCommand(lba, "10.0.0.1", "10.1.0.1", "10.1.1.1", null, 1L, "12", false, -1L)); + assertTrue("an unset idle timeout should fall back to the default", result.contains("\ttimeout client 50000")); + assertTrue("an unset idle timeout should fall back to the default", result.contains("\ttimeout server 50000")); + } + private String genConfig(HAProxyConfigurator hpg, LoadBalancerConfigCommand cmd) { String[] sa = hpg.generateConfiguration(cmd); StringBuilder sb = new StringBuilder(); From e0521a31d231176510702e5554cb0c5e92fa9bbe Mon Sep 17 00:00:00 2001 From: Brad House Date: Thu, 10 Sep 2026 16:03:11 +0000 Subject: [PATCH 3/5] Replace appsession, removed from HAProxy in 1.6 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 size 10k expire stick store-response res.cook() stick match req.cook() stick store-request req.cook() # 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 --- .../cloud/network/HAProxyConfigurator.java | 16 +++++++------ .../network/HAProxyConfiguratorTest.java | 24 +++++++++++++++++++ 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/com/cloud/network/HAProxyConfigurator.java b/core/src/main/java/com/cloud/network/HAProxyConfigurator.java index 230db113bbe8..fbe2ce3eb371 100644 --- a/core/src/main/java/com/cloud/network/HAProxyConfigurator.java +++ b/core/src/main/java/com/cloud/network/HAProxyConfigurator.java @@ -452,15 +452,17 @@ private String getLbSubRuleForStickiness(final LoadBalancerTO lbTO) { tempSb.append("appcookie_").append(srcip.hashCode()).append("_").append(lbTO.getSrcPort()); cookieName = tempSb.toString(); } - sb.append("\t").append("appsession ").append(cookieName).append(" len ").append(length).append(" timeout ").append(holdtime).append(" "); - if (prefix) { - sb.append("prefix "); - } + // "appsession" was removed in haproxy 1.6 and is a fatal parse error on the + // versions the system VM ships. A stick table on the cookie is the replacement. + sb.append("\t").append("stick-table type string len ").append(length).append(" size 10k expire ").append(holdtime).append("\n"); + sb.append("\t").append("stick store-response res.cook(").append(cookieName).append(")").append("\n"); + sb.append("\t").append("stick match req.cook(").append(cookieName).append(")").append("\n"); if (requestlearn) { - sb.append("request-learn").append(" "); + sb.append("\t").append("stick store-request req.cook(").append(cookieName).append(")").append("\n"); } - if (mode != null) { - sb.append("mode ").append(mode).append(" "); + if (prefix || mode != null) { + logger.warn("Haproxy stickiness policy for lb rule: {}:{}: prefix and mode are not supported since haproxy 1.6 and are ignored", + lbTO.getSrcIp(), lbTO.getSrcPort()); } } else { /* diff --git a/core/src/test/java/com/cloud/network/HAProxyConfiguratorTest.java b/core/src/test/java/com/cloud/network/HAProxyConfiguratorTest.java index 2f65d536e410..ea5d0c6f31de 100644 --- a/core/src/test/java/com/cloud/network/HAProxyConfiguratorTest.java +++ b/core/src/test/java/com/cloud/network/HAProxyConfiguratorTest.java @@ -31,6 +31,9 @@ import com.cloud.agent.api.routing.LoadBalancerConfigCommand; import com.cloud.agent.api.to.LoadBalancerTO; import com.cloud.network.lb.LoadBalancingRule.LbDestination; +import com.cloud.utils.Pair; +import com.cloud.network.rules.LbStickinessMethod.StickinessMethodType; +import com.cloud.network.lb.LoadBalancingRule.LbStickinessPolicy; import com.cloud.network.lb.LoadBalancingRule.LbSslCert; import java.util.List; @@ -199,6 +202,27 @@ public void generateConfigurationTestIdleTimeoutDoesNotLeakToNextConfig() { assertTrue("an unset idle timeout should fall back to the default", result.contains("\ttimeout server 50000")); } + @Test + public void generateConfigurationTestAppCookieStickinessUsesAStickTable() { + List> params = new ArrayList<>(); + params.add(new Pair<>("cookie-name", "JSESSIONID")); + params.add(new Pair<>("length", "52")); + params.add(new Pair<>("holdtime", "3h")); + List policies = new ArrayList<>(); + policies.add(new LbStickinessPolicy(StickinessMethodType.AppCookieBased.getName(), params)); + List dests = new ArrayList<>(); + dests.add(new LbDestination(80, 80, "10.1.10.2", false)); + LoadBalancerTO lb = new LoadBalancerTO("1", "10.2.0.1", 80, "http", "roundrobin", false, false, false, dests, policies); + LoadBalancerTO[] lba = new LoadBalancerTO[1]; + lba[0] = lb; + String result = genConfig(new HAProxyConfigurator(), + new LoadBalancerConfigCommand(lba, "10.0.0.1", "10.1.0.1", "10.1.1.1", null, 1L, "12", false, 50000L)); + Assert.assertFalse("appsession was removed in haproxy 1.6", result.contains("appsession")); + assertTrue(result.contains("stick-table type string len 52 size 10k expire 3h")); + assertTrue(result.contains("stick store-response res.cook(JSESSIONID)")); + assertTrue(result.contains("stick match req.cook(JSESSIONID)")); + } + private String genConfig(HAProxyConfigurator hpg, LoadBalancerConfigCommand cmd) { String[] sa = hpg.generateConfiguration(cmd); StringBuilder sb = new StringBuilder(); From 08327da074d4264886bf917c94babcbff5ed6258 Mon Sep 17 00:00:00 2001 From: Brad House Date: Thu, 10 Sep 2026 16:03:11 +0000 Subject: [PATCH 4/5] Make keepalive and timeouts settable per load balancer rule #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 --- .../cloud/agent/api/to/LoadBalancerTO.java | 27 +++++ .../network/lb/LoadBalancingRulesService.java | 7 ++ .../com/cloud/network/rules/LoadBalancer.java | 5 + .../apache/cloudstack/api/ApiConstants.java | 3 + .../CreateLoadBalancerRuleCmd.java | 29 +++++ .../UpdateLoadBalancerRuleCmd.java | 28 +++++ .../api/response/LoadBalancerResponse.java | 24 ++++ .../cloud/network/HAProxyConfigurator.java | 40 ++++++- .../network/HAProxyConfiguratorTest.java | 112 +++++++++++++++++- .../java/com/cloud/api/ApiResponseHelper.java | 21 ++++ .../lb/LoadBalancingRulesManagerImpl.java | 45 ++++++- .../network/router/CommandSetupHelper.java | 29 +++++ .../lb/LoadBalancingRulesManagerImplTest.java | 12 ++ ui/public/locales/en.json | 7 ++ ui/src/views/network/LoadBalancing.vue | 74 +++++++++++- 15 files changed, 449 insertions(+), 14 deletions(-) diff --git a/api/src/main/java/com/cloud/agent/api/to/LoadBalancerTO.java b/api/src/main/java/com/cloud/agent/api/to/LoadBalancerTO.java index 6c4b9e607c51..f9ab5d2d2e6f 100644 --- a/api/src/main/java/com/cloud/agent/api/to/LoadBalancerTO.java +++ b/api/src/main/java/com/cloud/agent/api/to/LoadBalancerTO.java @@ -54,6 +54,9 @@ public class LoadBalancerTO { final static int MAX_HEALTHCHECK_POLICIES = 1; private String cidrList; + private Boolean keepAlive; + private Long idleTimeout; + private Long keepAliveTimeout; public LoadBalancerTO(String uuid, String srcIp, int srcPort, String protocol, String algorithm, boolean revoked, boolean alreadyAdded, boolean inline, List destinations) { @@ -249,6 +252,30 @@ public String getCidrList() { return cidrList; } + public void setKeepAlive(Boolean keepAlive) { + this.keepAlive = keepAlive; + } + + public Boolean getKeepAlive() { + return keepAlive; + } + + public void setIdleTimeout(Long idleTimeout) { + this.idleTimeout = idleTimeout; + } + + public Long getIdleTimeout() { + return idleTimeout; + } + + public void setKeepAliveTimeout(Long keepAliveTimeout) { + this.keepAliveTimeout = keepAliveTimeout; + } + + public Long getKeepAliveTimeout() { + return keepAliveTimeout; + } + public static class StickinessPolicyTO { private String methodName; private List> params; diff --git a/api/src/main/java/com/cloud/network/lb/LoadBalancingRulesService.java b/api/src/main/java/com/cloud/network/lb/LoadBalancingRulesService.java index b7fe3b26761c..0abe20e274cd 100644 --- a/api/src/main/java/com/cloud/network/lb/LoadBalancingRulesService.java +++ b/api/src/main/java/com/cloud/network/lb/LoadBalancingRulesService.java @@ -70,6 +70,13 @@ LoadBalancer createPublicLoadBalancerRule(String xId, String name, String descri LoadBalancer updateLoadBalancerRule(UpdateLoadBalancerRuleCmd cmd); + /** + * Stores the optional per rule haproxy settings. A null leaves the current value alone. + * + * @return true if any value changed + */ + boolean updateLoadBalancerConnectionSettings(long lbRuleId, Boolean keepAlive, Long idleTimeout, Long keepAliveTimeout); + boolean deleteLoadBalancerRule(long lbRuleId, boolean apply); /** diff --git a/api/src/main/java/com/cloud/network/rules/LoadBalancer.java b/api/src/main/java/com/cloud/network/rules/LoadBalancer.java index 9b4e991b5b5a..5c64e018acbe 100644 --- a/api/src/main/java/com/cloud/network/rules/LoadBalancer.java +++ b/api/src/main/java/com/cloud/network/rules/LoadBalancer.java @@ -21,6 +21,11 @@ */ public interface LoadBalancer extends FirewallRule, LoadBalancerContainer { + // Optional per rule haproxy settings, held as firewall rule details. Absent means inherit. + String KEEPALIVE = "keepalive"; + String IDLE_TIMEOUT = "idletimeout"; + String KEEPALIVE_TIMEOUT = "keepalivetimeout"; + int getDefaultPortStart(); int getDefaultPortEnd(); diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index f74c46161180..f58df6c200a1 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -357,6 +357,7 @@ public class ApiConstants { public static final String IS_ISO = "isiso"; public static final String IS_PORTABLE = "isportable"; public static final String IS_PUBLIC = "ispublic"; + public static final String IDLE_TIMEOUT = "idletimeout"; public static final String IS_PERSISTENT = "ispersistent"; public static final String EGRESS_DEFAULT_POLICY = "egressdefaultpolicy"; public static final String IS_READY = "isready"; @@ -368,6 +369,8 @@ public class ApiConstants { public static final String JAVA_VERSION = "javaversion"; public static final String JOB_ID = "jobid"; public static final String JOB_STATUS = "jobstatus"; + public static final String KEEPALIVE = "keepalive"; + public static final String KEEPALIVE_TIMEOUT = "keepalivetimeout"; public static final String KEEPALIVE_ENABLED = "keepaliveenabled"; public static final String KERNEL_VERSION = "kernelversion"; public static final String KEYPAIR_ID = "keypairid"; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerRuleCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerRuleCmd.java index bd72f248364e..b6cb8cc9ca70 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerRuleCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerRuleCmd.java @@ -119,6 +119,22 @@ public class CreateLoadBalancerRuleCmd extends BaseAsyncCreateCmd /*implements L @Parameter(name = ApiConstants.FOR_DISPLAY, type = CommandType.BOOLEAN, description = "An optional field, whether to the display the rule to the end user or not", since = "4.4", authorized = {RoleType.Admin}) private Boolean display; + @Parameter(name = ApiConstants.KEEPALIVE, type = CommandType.BOOLEAN, since = "4.23.0", + description = "Whether the load balancer keeps client connections open between requests. " + + "Only applies to rules the router serves in HTTP mode. If not set, the network offering's setting is used.") + private Boolean keepAlive; + + @Parameter(name = ApiConstants.IDLE_TIMEOUT, type = CommandType.LONG, since = "4.23.0", + description = "How long an idle connection is held open, in milliseconds. Use 0 for infinite. " + + "If not set, the global setting network.loadbalancer.haproxy.idle.timeout is used.") + private Long idleTimeout; + + @Parameter(name = ApiConstants.KEEPALIVE_TIMEOUT, type = CommandType.LONG, since = "4.23.0", + description = "How long an idle keepalive connection is held open waiting for the next request, " + + "in milliseconds. Only applies to rules the router serves in HTTP mode. " + + "If not set, idletimeout applies.") + private Long keepAliveTimeout; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -135,6 +151,18 @@ public boolean isDisplay() { public String getAlgorithm() { return algorithm; } + public Boolean getKeepAlive() { + return keepAlive; + } + + public Long getIdleTimeout() { + return idleTimeout; + } + public Long getKeepAliveTimeout() { + return keepAliveTimeout; + } + + public String getDescription() { return description; @@ -307,6 +335,7 @@ public void create() { getCidrList()); this.setEntityId(result.getId()); this.setEntityUuid(result.getUuid()); + _lbService.updateLoadBalancerConnectionSettings(result.getId(), getKeepAlive(), getIdleTimeout(), getKeepAliveTimeout()); } catch (NetworkRuleConflictException e) { logger.warn("Exception: ", e); throw new ServerApiException(ApiErrorCode.NETWORK_RULE_CONFLICT_ERROR, e.getMessage()); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/loadbalancer/UpdateLoadBalancerRuleCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/loadbalancer/UpdateLoadBalancerRuleCmd.java index 0ac99f1c760c..a66a2d22a548 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/loadbalancer/UpdateLoadBalancerRuleCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/loadbalancer/UpdateLoadBalancerRuleCmd.java @@ -68,6 +68,22 @@ public class UpdateLoadBalancerRuleCmd extends BaseAsyncCustomIdCmd { @Parameter(name = ApiConstants.CIDR_LIST, type = CommandType.LIST, collectionType = CommandType.STRING, description = "the cidr list to forward traffic from", since = "4.22") private List cidrList; + @Parameter(name = ApiConstants.KEEPALIVE, type = CommandType.BOOLEAN, since = "4.23.0", + description = "Whether the load balancer keeps client connections open between requests. " + + "Only applies to rules the router serves in HTTP mode. If not set, the network offering's setting is used.") + private Boolean keepAlive; + + @Parameter(name = ApiConstants.IDLE_TIMEOUT, type = CommandType.LONG, since = "4.23.0", + description = "How long an idle connection is held open, in milliseconds. Use 0 for infinite. " + + "If not set, the global setting network.loadbalancer.haproxy.idle.timeout is used.") + private Long idleTimeout; + + @Parameter(name = ApiConstants.KEEPALIVE_TIMEOUT, type = CommandType.LONG, since = "4.23.0", + description = "How long an idle keepalive connection is held open waiting for the next request, " + + "in milliseconds. Only applies to rules the router serves in HTTP mode. " + + "If not set, idletimeout applies.") + private Long keepAliveTimeout; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -75,6 +91,18 @@ public class UpdateLoadBalancerRuleCmd extends BaseAsyncCustomIdCmd { public String getAlgorithm() { return algorithm; } + public Boolean getKeepAlive() { + return keepAlive; + } + + public Long getIdleTimeout() { + return idleTimeout; + } + public Long getKeepAliveTimeout() { + return keepAliveTimeout; + } + + public String getDescription() { return description; diff --git a/api/src/main/java/org/apache/cloudstack/api/response/LoadBalancerResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/LoadBalancerResponse.java index c57323d439dd..fd25c4e0042c 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/LoadBalancerResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/LoadBalancerResponse.java @@ -63,6 +63,18 @@ public class LoadBalancerResponse extends BaseResponse implements ControlledEnti @Param(description = "The ID of the guest Network the LB rule belongs to") private String networkId; + @SerializedName(ApiConstants.KEEPALIVE) + @Param(description = "whether the load balancer keeps client connections open between requests, unset means the network offering's setting is used", since = "4.23.0") + private Boolean keepAlive; + + @SerializedName(ApiConstants.IDLE_TIMEOUT) + @Param(description = "how long an idle connection is held open, in milliseconds, unset means the global setting is used", since = "4.23.0") + private Long idleTimeout; + + @SerializedName(ApiConstants.KEEPALIVE_TIMEOUT) + @Param(description = "how long an idle keepalive connection is held open waiting for the next request, in milliseconds", since = "4.23.0") + private Long keepAliveTimeout; + @SerializedName(ApiConstants.CIDR_LIST) @Param(description = "The CIDR list to allow traffic, all other CIDRs will be blocked. Multiple entries must be separated by a single comma character (,).") private String cidrList; @@ -143,6 +155,18 @@ public void setCidrList(String cidrs) { this.cidrList = cidrs; } + public void setKeepAlive(Boolean keepAlive) { + this.keepAlive = keepAlive; + } + + public void setIdleTimeout(Long idleTimeout) { + this.idleTimeout = idleTimeout; + } + + public void setKeepAliveTimeout(Long keepAliveTimeout) { + this.keepAliveTimeout = keepAliveTimeout; + } + public void setAlgorithm(String algorithm) { this.algorithm = algorithm; } diff --git a/core/src/main/java/com/cloud/network/HAProxyConfigurator.java b/core/src/main/java/com/cloud/network/HAProxyConfigurator.java index fbe2ce3eb371..3be197953b26 100644 --- a/core/src/main/java/com/cloud/network/HAProxyConfigurator.java +++ b/core/src/main/java/com/cloud/network/HAProxyConfigurator.java @@ -480,6 +480,18 @@ private String getLbSubRuleForStickiness(final LoadBalancerTO lbTO) { return sb.toString(); } + /** + * Haproxy rejects a negative timeout, and one bad value costs the whole file. Drop it and keep + * whatever the defaults section says, the same way the global idle timeout does. + */ + private Long timeoutOrNull(final LoadBalancerTO lbTO, final String name, final Long value) { + if (value != null && value < 0) { + logger.warn("Ignoring negative {} [{}] on lb rule {}:{}", name, value, lbTO.getSrcIp(), lbTO.getSrcPort()); + return null; + } + return value; + } + private List getRulesForPool(final LoadBalancerTO lbTO, final LoadBalancerConfigCommand lbCmd) { StringBuilder sb = new StringBuilder(); final String poolName = sb.append(lbTO.getSrcIp().replace(".", "_")).append('-').append(lbTO.getSrcPort()).toString(); @@ -572,12 +584,30 @@ private List getRulesForPool(final LoadBalancerTO lbTO, final LoadBalanc if (stickinessSubRule != null && !destsAvailable) { logger.warn("Haproxy stickiness policy for lb rule: " + lbTO.getSrcIp() + ":" + lbTO.getSrcPort() + ": Not Applied, cause: backends are unavailable"); } - boolean keepAliveEnabled = lbCmd.keepAliveEnabled; - boolean http = (publicPort == NetUtils.HTTP_PORT && !keepAliveEnabled); - if (http || httpbasedStickiness || sslOffloading) { + final Boolean ruleKeepAlive = lbTO.getKeepAlive(); + final Long ruleIdleTimeout = timeoutOrNull(lbTO, "idletimeout", lbTO.getIdleTimeout()); + final Long ruleKeepAliveTimeout = timeoutOrNull(lbTO, "keepalivetimeout", lbTO.getKeepAliveTimeout()); + final boolean keepAliveEnabled = ruleKeepAlive != null ? ruleKeepAlive : lbCmd.keepAliveEnabled; + // A rule that asks for keepalive itself stays in http mode on port 80, so forwardfor keeps + // working. Without it, keepalive falls back to tcp mode as it always has. + final boolean port80HttpMode = publicPort == NetUtils.HTTP_PORT && (ruleKeepAlive != null || !keepAliveEnabled); + final boolean httpMode = port80HttpMode || httpbasedStickiness || sslOffloading; + if (httpMode) { frontendConfigs.add("\tmode http"); - String keepAliveLine = keepAliveEnabled ? "\toption http-keep-alive" : "\toption httpclose"; - frontendConfigs.add(keepAliveLine); + frontendConfigs.add(keepAliveEnabled ? "\toption http-keep-alive" : "\toption httpclose"); + if (keepAliveEnabled && ruleKeepAliveTimeout != null) { + frontendConfigs.add("\ttimeout http-keep-alive " + ruleKeepAliveTimeout); + } else if (ruleKeepAliveTimeout != null) { + logger.warn("Keepalive timeout ignored for lb rule {}:{}, keepalive is off for this rule", + lbTO.getSrcIp(), lbTO.getSrcPort()); + } + } else if (ruleKeepAlive != null || ruleKeepAliveTimeout != null) { + logger.warn("Keepalive ignored for lb rule {}:{}, it is served in tcp mode. Keepalive applies on port {}, " + + "with ssl offload, or with http based stickiness.", lbTO.getSrcIp(), lbTO.getSrcPort(), NetUtils.HTTP_PORT); + } + if (ruleIdleTimeout != null) { + frontendConfigs.add("\ttimeout client " + ruleIdleTimeout); + frontendConfigs.add("\ttimeout server " + ruleIdleTimeout); } // add line like this: "listen 65_37_141_30-80\n\tbind 65.37.141.30:80" diff --git a/core/src/test/java/com/cloud/network/HAProxyConfiguratorTest.java b/core/src/test/java/com/cloud/network/HAProxyConfiguratorTest.java index ea5d0c6f31de..3bbbe20b357b 100644 --- a/core/src/test/java/com/cloud/network/HAProxyConfiguratorTest.java +++ b/core/src/test/java/com/cloud/network/HAProxyConfiguratorTest.java @@ -31,9 +31,9 @@ import com.cloud.agent.api.routing.LoadBalancerConfigCommand; import com.cloud.agent.api.to.LoadBalancerTO; import com.cloud.network.lb.LoadBalancingRule.LbDestination; -import com.cloud.utils.Pair; -import com.cloud.network.rules.LbStickinessMethod.StickinessMethodType; import com.cloud.network.lb.LoadBalancingRule.LbStickinessPolicy; +import com.cloud.network.rules.LbStickinessMethod.StickinessMethodType; +import com.cloud.utils.Pair; import com.cloud.network.lb.LoadBalancingRule.LbSslCert; import java.util.List; @@ -202,6 +202,114 @@ public void generateConfigurationTestIdleTimeoutDoesNotLeakToNextConfig() { assertTrue("an unset idle timeout should fall back to the default", result.contains("\ttimeout server 50000")); } + private LoadBalancerConfigCommand cmdFor(LoadBalancerTO lb, boolean offeringKeepAlive) { + LoadBalancerTO[] lba = new LoadBalancerTO[1]; + lba[0] = lb; + return new LoadBalancerConfigCommand(lba, "10.0.0.1", "10.1.0.1", "10.1.1.1", null, 1L, "12", offeringKeepAlive, 50000L); + } + + private LoadBalancerTO httpRule() { + return new LoadBalancerTO("1", "10.2.0.1", 80, "http", "roundrobin", false, false, false, null); + } + + /** Just the "listen" block for this rule, so assertions cannot match the defaults or stats sections. */ + private String poolSection(String config, String poolName) { + int start = config.indexOf("listen " + poolName + "\n"); + Assert.assertTrue("no listen block for " + poolName, start >= 0); + int next = config.indexOf("\nlisten ", start + 1); + return next < 0 ? config.substring(start) : config.substring(start, next); + } + + @Test + public void generateConfigurationTestPerRuleKeepAliveKeepsHttpMode() { + LoadBalancerTO lb = httpRule(); + lb.setKeepAlive(true); + String pool = poolSection(genConfig(new HAProxyConfigurator(), cmdFor(lb, false)), "10_2_0_1-80"); + assertTrue("the rule should stay in http mode so forwardfor still works", pool.contains("\tmode http")); + assertTrue(pool.contains("\toption http-keep-alive")); + Assert.assertFalse(pool.contains("\toption httpclose")); + } + + @Test + public void generateConfigurationTestPerRuleKeepAliveOverridesTheOffering() { + LoadBalancerTO lb = httpRule(); + lb.setKeepAlive(false); + String pool = poolSection(genConfig(new HAProxyConfigurator(), cmdFor(lb, true)), "10_2_0_1-80"); + assertTrue("the rule should win over the offering", pool.contains("\toption httpclose")); + Assert.assertFalse(pool.contains("\toption http-keep-alive")); + } + + @Test + public void generateConfigurationTestUnsetKeepAliveLeavesOldBehaviourAlone() { + String pool = poolSection(genConfig(new HAProxyConfigurator(), cmdFor(httpRule(), true)), "10_2_0_1-80"); + Assert.assertFalse("keepalive from the offering still drops to tcp mode", pool.contains("\tmode http")); + + pool = poolSection(genConfig(new HAProxyConfigurator(), cmdFor(httpRule(), false)), "10_2_0_1-80"); + assertTrue(pool.contains("\tmode http")); + assertTrue(pool.contains("\toption httpclose")); + } + + @Test + public void generateConfigurationTestKeepAliveTimeout() { + LoadBalancerTO lb = httpRule(); + lb.setKeepAlive(true); + lb.setKeepAliveTimeout(15000L); + String pool = poolSection(genConfig(new HAProxyConfigurator(), cmdFor(lb, false)), "10_2_0_1-80"); + assertTrue(pool.contains("\ttimeout http-keep-alive 15000")); + } + + @Test + public void generateConfigurationTestKeepAliveTimeoutNeedsKeepAlive() { + LoadBalancerTO lb = httpRule(); + lb.setKeepAliveTimeout(15000L); + String pool = poolSection(genConfig(new HAProxyConfigurator(), cmdFor(lb, false)), "10_2_0_1-80"); + Assert.assertFalse("without keepalive there is no idle connection to time out", pool.contains("timeout http-keep-alive")); + } + + @Test + public void generateConfigurationTestIdleTimeoutOverridesTheGlobalPerRule() { + LoadBalancerTO lb = new LoadBalancerTO("1", "10.2.0.1", 3306, "tcp", "roundrobin", false, false, false, null); + lb.setIdleTimeout(600000L); + String config = genConfig(new HAProxyConfigurator(), cmdFor(lb, false)); + String pool = poolSection(config, "10_2_0_1-3306"); + assertTrue("a tcp rule gets the timeouts too", pool.contains("\ttimeout client 600000")); + assertTrue(pool.contains("\ttimeout server 600000")); + assertTrue("the global still sets the defaults", config.contains("\ttimeout client 50000")); + } + + @Test + public void generateConfigurationTestNegativeTimeoutsAreDropped() { + LoadBalancerTO lb = httpRule(); + lb.setKeepAlive(true); + lb.setIdleTimeout(-1L); + lb.setKeepAliveTimeout(-5L); + String pool = poolSection(genConfig(new HAProxyConfigurator(), cmdFor(lb, false)), "10_2_0_1-80"); + Assert.assertFalse("a negative timeout is a fatal haproxy parse error", pool.contains("-1")); + Assert.assertFalse("a negative timeout is a fatal haproxy parse error", pool.contains("-5")); + Assert.assertFalse(pool.contains("timeout client")); + Assert.assertFalse(pool.contains("timeout http-keep-alive")); + } + + @Test + public void generateConfigurationTestPerRuleSettingsDoNotCrossContaminate() { + LoadBalancerTO tuned = httpRule(); + tuned.setKeepAlive(true); + tuned.setIdleTimeout(600000L); + tuned.setKeepAliveTimeout(15000L); + LoadBalancerTO plain = new LoadBalancerTO("2", "10.2.0.1", 8080, "tcp", "roundrobin", false, false, false, null); + LoadBalancerTO[] lba = new LoadBalancerTO[] {tuned, plain}; + LoadBalancerConfigCommand cmd = new LoadBalancerConfigCommand(lba, "10.0.0.1", "10.1.0.1", "10.1.1.1", null, 1L, "12", false, 50000L); + String config = genConfig(new HAProxyConfigurator(), cmd); + + String tunedPool = poolSection(config, "10_2_0_1-80"); + assertTrue(tunedPool.contains("\toption http-keep-alive")); + assertTrue(tunedPool.contains("\ttimeout client 600000")); + + String plainPool = poolSection(config, "10_2_0_1-8080"); + Assert.assertFalse("the other rule's settings must not leak here", plainPool.contains("http-keep-alive")); + Assert.assertFalse("the other rule's settings must not leak here", plainPool.contains("timeout client")); + } + @Test public void generateConfigurationTestAppCookieStickinessUsesAStickTable() { List> params = new ArrayList<>(); diff --git a/server/src/main/java/com/cloud/api/ApiResponseHelper.java b/server/src/main/java/com/cloud/api/ApiResponseHelper.java index f56cda6e557a..2eed2674bdd6 100644 --- a/server/src/main/java/com/cloud/api/ApiResponseHelper.java +++ b/server/src/main/java/com/cloud/api/ApiResponseHelper.java @@ -240,9 +240,12 @@ import org.apache.cloudstack.usage.UsageService; import org.apache.cloudstack.usage.UsageTypes; import org.apache.cloudstack.vm.UnmanagedInstanceTO; +import org.apache.cloudstack.resourcedetail.FirewallRuleDetailVO; +import org.apache.cloudstack.resourcedetail.dao.FirewallRuleDetailsDao; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang3.ObjectUtils; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.math.NumberUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -467,6 +470,8 @@ public class ApiResponseHelper implements ResponseGenerator, ResourceIdSupport { protected Logger logger = LogManager.getLogger(ApiResponseHelper.class); private static final DecimalFormat s_percentFormat = new DecimalFormat("##.##"); + @Inject + private FirewallRuleDetailsDao firewallRuleDetailsDao; @Inject private EntityManager _entityMgr; @Inject @@ -1305,6 +1310,21 @@ private void showVmInfoForSharedNetworks(boolean forVirtualNetworks, IpAddress i } } + private void setLbConnectionSettings(LoadBalancerResponse lbResponse, long lbRuleId) { + FirewallRuleDetailVO keepAlive = firewallRuleDetailsDao.findDetail(lbRuleId, LoadBalancer.KEEPALIVE); + if (keepAlive != null) { + lbResponse.setKeepAlive(Boolean.valueOf(keepAlive.getValue())); + } + FirewallRuleDetailVO idleTimeout = firewallRuleDetailsDao.findDetail(lbRuleId, LoadBalancer.IDLE_TIMEOUT); + if (idleTimeout != null) { + lbResponse.setIdleTimeout(NumberUtils.toLong(idleTimeout.getValue())); + } + FirewallRuleDetailVO keepAliveTimeout = firewallRuleDetailsDao.findDetail(lbRuleId, LoadBalancer.KEEPALIVE_TIMEOUT); + if (keepAliveTimeout != null) { + lbResponse.setKeepAliveTimeout(NumberUtils.toLong(keepAliveTimeout.getValue())); + } + } + @Override public LoadBalancerResponse createLoadBalancerResponse(LoadBalancer loadBalancer) { LoadBalancerResponse lbResponse = new LoadBalancerResponse(); @@ -1313,6 +1333,7 @@ public LoadBalancerResponse createLoadBalancerResponse(LoadBalancer loadBalancer lbResponse.setDescription(loadBalancer.getDescription()); List cidrs = ApiDBUtils.findFirewallSourceCidrs(loadBalancer.getId()); lbResponse.setCidrList(StringUtils.join(cidrs, ",")); + setLbConnectionSettings(lbResponse, loadBalancer.getId()); IPAddressVO publicIp = ApiDBUtils.findIpAddressById(loadBalancer.getSourceIpAddressId()); lbResponse.setPublicIpId(publicIp.getUuid()); diff --git a/server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java b/server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java index 38f405fe1e4d..ae75b75f81c6 100644 --- a/server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java +++ b/server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java @@ -35,6 +35,8 @@ import org.apache.cloudstack.acl.ApiKeyPairVO; import org.apache.cloudstack.acl.SecurityChecker; import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.resourcedetail.FirewallRuleDetailVO; +import org.apache.cloudstack.resourcedetail.dao.FirewallRuleDetailsDao; import org.apache.cloudstack.api.ApiErrorCode; import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.command.user.loadbalancer.CreateLBHealthCheckPolicyCmd; @@ -183,6 +185,8 @@ public class LoadBalancingRulesManagerImpl extends ManagerBase implements LoadBalancingRulesManager, LoadBalancingRulesService { + @Inject + FirewallRuleDetailsDao _firewallRuleDetailsDao; @Inject NetworkOrchestrationService _networkMgr; @Inject @@ -2285,6 +2289,42 @@ public List getExistingDestinations(long lbId) { return dstList; } + /** + * Haproxy rejects a negative timeout, and a rejected file leaves every rule on the router + * running its previous config. Refuse the value here rather than let it reach the VR. + */ + protected void validateConnectionTimeout(String name, Long value) { + if (value != null && value < 0) { + throw new InvalidParameterValueException(String.format("%s must be 0 or greater, got [%s]. 0 means no timeout.", name, value)); + } + } + + @Override + public boolean updateLoadBalancerConnectionSettings(long lbRuleId, Boolean keepAlive, Long idleTimeout, Long keepAliveTimeout) { + validateConnectionTimeout(ApiConstants.IDLE_TIMEOUT, idleTimeout); + validateConnectionTimeout(ApiConstants.KEEPALIVE_TIMEOUT, keepAliveTimeout); + + boolean changed = storeDetail(lbRuleId, LoadBalancer.KEEPALIVE, keepAlive == null ? null : keepAlive.toString()); + changed |= storeDetail(lbRuleId, LoadBalancer.IDLE_TIMEOUT, idleTimeout == null ? null : idleTimeout.toString()); + changed |= storeDetail(lbRuleId, LoadBalancer.KEEPALIVE_TIMEOUT, keepAliveTimeout == null ? null : keepAliveTimeout.toString()); + return changed; + } + + private boolean storeDetail(long lbRuleId, String key, String value) { + if (value == null) { + return false; + } + FirewallRuleDetailVO existing = _firewallRuleDetailsDao.findDetail(lbRuleId, key); + if (existing != null && value.equals(existing.getValue())) { + return false; + } + if (existing != null) { + _firewallRuleDetailsDao.removeDetail(lbRuleId, key); + } + _firewallRuleDetailsDao.addDetail(lbRuleId, key, value, true); + return true; + } + @Override @ActionEvent(eventType = EventTypes.EVENT_LOAD_BALANCER_UPDATE, eventDescription = "updating load balancer", async = true) public LoadBalancer updateLoadBalancerRule(UpdateLoadBalancerRuleCmd cmd) { @@ -2342,6 +2382,9 @@ public LoadBalancer updateLoadBalancerRule(UpdateLoadBalancerRuleCmd cmd) { lb.setCidrList(cidrListStr); } + // lb.getId() rather than the id off the command, which is a Long and unboxes badly + boolean settingsChanged = updateLoadBalancerConnectionSettings(lb.getId(), cmd.getKeepAlive(), cmd.getIdleTimeout(), cmd.getKeepAliveTimeout()); + // Validate rule in LB provider LoadBalancingRule rule = getLoadBalancerRuleToApply(lb); if (!validateLbRule(rule)) { @@ -2356,7 +2399,7 @@ public LoadBalancer updateLoadBalancerRule(UpdateLoadBalancerRuleCmd cmd) { boolean protocolChanged = !Objects.equals(lbProtocol, tmplbVo.getLbProtocol()); boolean cidrListChanged = !Objects.equals(tmplbVo.getCidrList(), lb.getCidrList()); - if (algorithmChanged || protocolChanged || cidrListChanged) { + if (algorithmChanged || protocolChanged || cidrListChanged || settingsChanged) { try { lb.setState(FirewallRule.State.Add); _lbDao.persist(lb); diff --git a/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java b/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java index 58b2892e09c3..05dd3d4bc0b0 100644 --- a/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java +++ b/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java @@ -34,6 +34,8 @@ import org.apache.cloudstack.network.BgpPeer; import org.apache.cloudstack.network.BgpPeerTO; import org.apache.cloudstack.network.dao.BgpPeerDetailsDao; +import org.apache.cloudstack.resourcedetail.FirewallRuleDetailVO; +import org.apache.cloudstack.resourcedetail.dao.FirewallRuleDetailsDao; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; @@ -119,6 +121,7 @@ import com.cloud.network.lb.LoadBalancingRule.LbDestination; import com.cloud.network.lb.LoadBalancingRule.LbStickinessPolicy; import com.cloud.network.rules.FirewallRule; +import com.cloud.network.rules.LoadBalancer; import com.cloud.network.rules.FirewallRule.Purpose; import com.cloud.network.rules.FirewallRuleVO; import com.cloud.network.rules.PortForwardingRule; @@ -169,6 +172,8 @@ public class CommandSetupHelper { @Inject private DomainDao domainDao; @Inject + private FirewallRuleDetailsDao _firewallRuleDetailsDao; + @Inject private NicDao _nicDao; @Inject private NetworkDao _networkDao; @@ -352,6 +357,29 @@ public void configDnsMasq(final VirtualRouter router, final Network network, fin cmds.addCommand("dnsMasqConfig", dnsMasqConfigCmd); } + /** Optional per rule haproxy settings. Absent means the rule inherits, so nothing is set. */ + protected void setConnectionSettings(final LoadBalancerTO lb, final long lbRuleId) { + final FirewallRuleDetailVO keepAlive = _firewallRuleDetailsDao.findDetail(lbRuleId, LoadBalancer.KEEPALIVE); + if (keepAlive != null) { + lb.setKeepAlive(Boolean.valueOf(keepAlive.getValue())); + } + lb.setIdleTimeout(longDetail(lbRuleId, LoadBalancer.IDLE_TIMEOUT)); + lb.setKeepAliveTimeout(longDetail(lbRuleId, LoadBalancer.KEEPALIVE_TIMEOUT)); + } + + private Long longDetail(final long lbRuleId, final String key) { + final FirewallRuleDetailVO detail = _firewallRuleDetailsDao.findDetail(lbRuleId, key); + if (detail == null) { + return null; + } + try { + return Long.valueOf(detail.getValue()); + } catch (final NumberFormatException e) { + logger.warn("Ignoring lb rule {} detail {}, [{}] is not a number", lbRuleId, key, detail.getValue()); + return null; + } + } + public void createApplyLoadBalancingRulesCommands(final List rules, final VirtualRouter router, final Commands cmds, final long guestNetworkId) { final LoadBalancerTO[] lbs = new LoadBalancerTO[rules.size()]; int i = 0; @@ -375,6 +403,7 @@ public void createApplyLoadBalancingRulesCommands(final List } lb.setLbProtocol(lb_protocol); lb.setLbSslCert(rule.getLbSslCert()); + setConnectionSettings(lb, rule.getId()); lbs[i++] = lb; } String routerPublicIp = null; diff --git a/server/src/test/java/com/cloud/network/lb/LoadBalancingRulesManagerImplTest.java b/server/src/test/java/com/cloud/network/lb/LoadBalancingRulesManagerImplTest.java index 282d892600b5..491cfab44196 100644 --- a/server/src/test/java/com/cloud/network/lb/LoadBalancingRulesManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/lb/LoadBalancingRulesManagerImplTest.java @@ -378,4 +378,16 @@ public void createPublicLoadBalancerRuleWithDnsPortAndNoIpDoesNotNpe() throws Ex lbr.createPublicLoadBalancerRule("xid", "name", "desc", 53, 53, 53, 53, null, "tcp", "roundrobin", networkId, lbOwnerId, false, "tcp", null, null); } + + @Test + public void testValidateConnectionTimeoutAcceptsZeroAndAbove() { + lbr.validateConnectionTimeout(ApiConstants.IDLE_TIMEOUT, null); + lbr.validateConnectionTimeout(ApiConstants.IDLE_TIMEOUT, 0L); + lbr.validateConnectionTimeout(ApiConstants.IDLE_TIMEOUT, 600000L); + } + + @Test(expected = InvalidParameterValueException.class) + public void testValidateConnectionTimeoutRejectsNegative() { + lbr.validateConnectionTimeout(ApiConstants.IDLE_TIMEOUT, -1L); + } } diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index 99bf2cf7aef9..37dfa39e8a17 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -1370,6 +1370,7 @@ "label.in.progress": "in progress", "label.in.progress.for": "in progress for", "label.info": "Info", +"label.inherit.default": "Use default", "label.info.upper": "INFO", "label.infrastructure": "Infrastructure", "label.ingest.instance": "Ingest Instance", @@ -1459,6 +1460,8 @@ "label.isadvanced": "Show advanced settings", "label.iscsi": "iSCSI", "label.iscustomized": "Custom disk size", +"label.idletimeout": "Idle timeout (ms)", +"label.idletimeout.tooltip": "How long an idle connection is held open, in milliseconds. Use 0 for no timeout. Leave blank to use the global setting.", "label.iscustomizeddiskiops": "Custom IOPS", "label.iscustomizediops": "Custom IOPS", "label.isdedicated": "Dedicated", @@ -1508,6 +1511,10 @@ "label.key": "Key", "label.keybits": "Key Bits", "label.keyboard": "Keyboard language", +"label.keepalive": "Keepalive", +"label.keepalive.tooltip": "Whether the load balancer keeps client connections open between requests. Only applies to rules served in HTTP mode. Leave blank to use the Network offering setting.", +"label.keepalivetimeout": "Keepalive timeout (ms)", +"label.keepalivetimeout.tooltip": "How long an idle keepalive connection is held open waiting for the next request, in milliseconds. Only applies when keepalive is on.", "label.keyboardtype": "Keyboard type", "label.keypair": "SSH key pair", "label.keypairs": "SSH key pair(s)", diff --git a/ui/src/views/network/LoadBalancing.vue b/ui/src/views/network/LoadBalancing.vue index 0b9ed7684a89..ae37caee714c 100644 --- a/ui/src/views/network/LoadBalancing.vue +++ b/ui/src/views/network/LoadBalancing.vue @@ -472,6 +472,61 @@ :placeholder="$t('label.sourcecidrlist')" /> +
+

+ {{ $t('label.keepalive') }} + +

+ + {{ $t('label.inherit.default') }} + {{ $t('label.yes') }} + {{ $t('label.no') }} + +
+
+

+ {{ $t('label.idletimeout') }} + +

+ +
+
+

+ {{ $t('label.keepalivetimeout') }} + +

+ +
{{ $t('label.cancel') }} {{ $t('label.ok') }} @@ -859,7 +914,10 @@ export default { name: '', algorithm: '', protocol: '', - cidrlist: '' + cidrlist: '', + keepalive: undefined, + idletimeout: undefined, + keepalivetimeout: undefined }, newRule: { algorithm: 'roundrobin', @@ -1663,6 +1721,9 @@ export default { this.editRuleDetails.algorithm = this.lbProvider !== 'Netris' ? this.selectedRule.algorithm : undefined this.editRuleDetails.protocol = this.selectedRule.protocol // Normalize cidrlist: replace spaces with commas and clean up + this.editRuleDetails.keepalive = this.selectedRule.keepalive + this.editRuleDetails.idletimeout = this.selectedRule.idletimeout + this.editRuleDetails.keepalivetimeout = this.selectedRule.keepalivetimeout this.editRuleDetails.cidrlist = (this.selectedRule.cidrlist || '') .split(/[\s,]+/) // Split on spaces or commas .map(c => c.trim()) @@ -1680,11 +1741,12 @@ export default { cidrList: (this.editRuleDetails.cidrlist || '').split(',').map(c => c.trim()).filter(c => c) }) } - postAPI('updateLoadBalancerRule', { - ...this.editRuleDetails, - id: this.selectedRule.id, - ...payload - }).then(response => { + for (const key of ['keepalive', 'idletimeout', 'keepalivetimeout']) { + if (payload[key] === '' || payload[key] === null || payload[key] === undefined) { + delete payload[key] + } + } + postAPI('updateLoadBalancerRule', payload).then(response => { this.$pollJob({ jobId: response.updateloadbalancerruleresponse.jobid, successMessage: this.$t('message.success.edit.rule'), From c8cf0fdec01b64499a37629851c8ee70c129d9e4 Mon Sep 17 00:00:00 2001 From: Brad House Date: Thu, 10 Sep 2026 18:18:20 +0000 Subject: [PATCH 5/5] Check the per rule settings in the VR health check 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 --- .../VirtualNetworkApplianceManagerImpl.java | 24 +++++++- .../root/health_checks/haproxy_check.py | 58 ++++++++++++++++++- 2 files changed, 79 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java b/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java index a166e894be1b..7f985b91e975 100644 --- a/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java +++ b/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java @@ -78,6 +78,8 @@ import org.apache.cloudstack.utils.CloudStackVersion; import org.apache.cloudstack.utils.identity.ManagementServerNode; import org.apache.cloudstack.utils.usage.UsageUtils; +import org.apache.cloudstack.resourcedetail.FirewallRuleDetailVO; +import org.apache.cloudstack.resourcedetail.dao.FirewallRuleDetailsDao; import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.lang3.ObjectUtils; import org.apache.commons.lang3.StringUtils; @@ -199,6 +201,7 @@ import com.cloud.network.router.VirtualRouter.RedundantState; import com.cloud.network.router.VirtualRouter.Role; import com.cloud.network.rules.FirewallRule; +import com.cloud.network.rules.LoadBalancer; import com.cloud.network.rules.FirewallRule.Purpose; import com.cloud.network.rules.FirewallRuleVO; import com.cloud.network.rules.LoadBalancerContainer.Scheme; @@ -352,6 +355,8 @@ public class VirtualNetworkApplianceManagerImpl extends ManagerBase implements V @Inject protected CommandSetupHelper _commandSetupHelper; @Inject ManagementServer mgr; @Inject + private FirewallRuleDetailsDao _firewallRuleDetailsDao; + @Inject RoutedIpv4Manager routedIpv4Manager; @Inject BGPService bgpService; @@ -1711,7 +1716,9 @@ private void updateWithLbRules(final DomainRouterJoinVO routerJoinVO, final Stri .append(",protocol=").append(appLoadBalancerVO.getLbProtocol()); } loadBalancingData.append(",stickiness=").append(getStickinessPolicies(firewallRuleVO.getId())); - loadBalancingData.append(",keepAliveEnabled=").append(offering.isKeepAliveEnabled()).append(",vmIps="); + loadBalancingData.append(",keepAliveEnabled=").append(offering.isKeepAliveEnabled()); + appendLbRuleConnectionSettings(loadBalancingData, firewallRuleVO.getId()); + loadBalancingData.append(",vmIps="); for (LoadBalancerVMMapVO vmMapVO : vmMapVOs) { loadBalancingData.append(vmMapVO.getInstanceIp()).append(" "); } @@ -1720,6 +1727,21 @@ private void updateWithLbRules(final DomainRouterJoinVO routerJoinVO, final Stri } } + /** + * The per rule haproxy settings, so the health check can tell what the rule's own listen + * section should hold. Empty when the rule inherits. + */ + protected void appendLbRuleConnectionSettings(final StringBuilder loadBalancingData, long lbRuleId) { + loadBalancingData.append(",ruleKeepAlive=").append(lbRuleDetail(lbRuleId, LoadBalancer.KEEPALIVE)) + .append(",ruleIdleTimeout=").append(lbRuleDetail(lbRuleId, LoadBalancer.IDLE_TIMEOUT)) + .append(",ruleKeepAliveTimeout=").append(lbRuleDetail(lbRuleId, LoadBalancer.KEEPALIVE_TIMEOUT)); + } + + private String lbRuleDetail(long lbRuleId, String key) { + FirewallRuleDetailVO detail = _firewallRuleDetailsDao.findDetail(lbRuleId, key); + return detail == null ? "" : detail.getValue(); + } + protected void updateWithLbRuleSslCertificates(final StringBuilder loadBalancingData, LoadBalancerVO loadBalancerVO, String sourceIp) { if (NetUtils.SSL_PROTO.equals(loadBalancerVO.getLbProtocol())) { final LbSslCert sslCert = _lbMgr.getLbSslCert(loadBalancerVO.getId()); diff --git a/systemvm/debian/root/health_checks/haproxy_check.py b/systemvm/debian/root/health_checks/haproxy_check.py index cc9d90f7c18e..f06f7a685977 100644 --- a/systemvm/debian/root/health_checks/haproxy_check.py +++ b/systemvm/debian/root/health_checks/haproxy_check.py @@ -69,6 +69,52 @@ def checkIdletimeout(haproxyData, haCfgSections): return False return True +def hasOption(cfgSection, option): + return option in cfgSection.get("option", []) + + +def timeoutValue(cfgSection, kind): + for tline in cfgSection.get("timeout", []): + parts = tline.strip().split(None, 1) + if len(parts) == 2 and parts[0].strip() == kind: + return parts[1].strip() + return None + + +def checkRuleConnectionSettings(lbSec, cfgSection, secName, httpModeExpected): + """ + Per rule keepalive and timeouts, which live in the rule's own listen section rather than in + defaults. An empty value means the rule inherits, so there is nothing to check. + """ + correct = True + + keepAlive = lbSec.get("ruleKeepAlive", "") + if keepAlive != "" and httpModeExpected: + expected = "http-keep-alive" if keepAlive == "true" else "httpclose" + if not hasOption(cfgSection, expected): + print("Expected 'option " + expected + "' in " + secName) + correct = False + + idleTimeout = lbSec.get("ruleIdleTimeout", "") + if idleTimeout != "": + for kind in ("client", "server"): + found = timeoutValue(cfgSection, kind) + if found != idleTimeout: + print("Expected 'timeout " + kind + " " + idleTimeout + "' in " + secName + + " but found " + str(found)) + correct = False + + keepAliveTimeout = lbSec.get("ruleKeepAliveTimeout", "") + if keepAliveTimeout != "" and keepAlive == "true" and httpModeExpected: + found = timeoutValue(cfgSection, "http-keep-alive") + if found != keepAliveTimeout: + print("Expected 'timeout http-keep-alive " + keepAliveTimeout + "' in " + secName + + " but found " + str(found)) + correct = False + + return correct + + def checkLoadBalance(haproxyData, haCfgSections): correct = True for lbSec in haproxyData: @@ -94,12 +140,20 @@ def checkLoadBalance(haproxyData, haCfgSections): print("Incorrect bind string found. Expected " + bindStr + " but found " + cfgSection["bind"][0] + ".") correct = False - if (lbSec["sourcePortStart"] == "80" and lbSec["sourcePortEnd"] == "80" and lbSec["keepAliveEnabled"] == "false") \ - or (lbSec["stickiness"].find("AppCookie") != -1 or lbSec["stickiness"].find("LbCookie") != -1): + # A rule that sets keepalive itself stays in http mode on port 80. Without it, the + # network offering decides, and keepalive there drops the rule to tcp mode. + onHttpPort = lbSec["sourcePortStart"] == "80" and lbSec["sourcePortEnd"] == "80" + ruleKeepAlive = lbSec.get("ruleKeepAlive", "") + httpModeExpected = (onHttpPort and (ruleKeepAlive != "" or lbSec["keepAliveEnabled"] == "false")) \ + or lbSec["stickiness"].find("AppCookie") != -1 or lbSec["stickiness"].find("LbCookie") != -1 + if httpModeExpected: if not ("mode" in cfgSection and cfgSection["mode"][0] == "http"): print("Expected HTTP mode but not found") correct = False + if not checkRuleConnectionSettings(lbSec, cfgSection, secName, httpModeExpected): + correct = False + expectedServerIps = lbSec["vmIps"].split(" ") for expectedServerIp in expectedServerIps: pattern = expectedServerIp + ":" + \