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 <bhouse@nexthop.ai>

Author: bhouse-nexthop

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());

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 + ":" + \