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 b3f7da1c6d82..3be197953b26 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); @@ -451,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 { /* @@ -477,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(); @@ -569,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 ? "\tno option forceclose" : "\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" @@ -617,8 +650,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 +663,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 +671,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 +726,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 073f976719b5..3bbbe20b357b 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.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; @@ -158,6 +161,176 @@ 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")); + } + + @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 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<>(); + 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(); 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/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/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/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 + ":" + \ 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'),