Merge pull request #3338 from nvollmar/haproxy-http-check

T6246: improve haproxy http check configuration

4 files changed, 73 insertions, 13 deletions

diff --git a/data/templates/load-balancing/haproxy.cfg.j2 b/data/templates/load-balancing/haproxy.cfg.j2
index 83008e50a..dd93afba5 100644
--- a/data/templates/load-balancing/haproxy.cfg.j2
+++ b/data/templates/load-balancing/haproxy.cfg.j2
@@ -113,11 +113,22 @@ backend {{ back }}
 {%         if back_config.http_check is vyos_defined %}
     option httpchk
 {%         endif %}
-{%         if back_config.http_check.uri is vyos_defined and back_config.http_check.method is vyos_defined %}
-    http-check send meth {{ back_config.http_check.method | upper }} uri {{ back_config.http_check.uri }}
+{%         set send = '' %}
+{%         if back_config.http_check.method is vyos_defined %}
+{%             set send = send + ' meth ' + back_config.http_check.method | upper %}
+{%         endif %}
+{%         if back_config.http_check.uri is vyos_defined %}
+{%             set send = send + ' uri ' + back_config.http_check.uri %}
+{%         endif %}
+{%         if send != '' %}
+    http-check send{{ send }}
 {%         endif %}
 {%         if back_config.http_check.expect is vyos_defined %}
-    http-check expect {{ back_config.http_check.expect }}
+{%             if back_config.http_check.expect.status is vyos_defined %}
+    http-check expect status {{ back_config.http_check.expect.status }}
+{%             elif back_config.http_check.expect.string is vyos_defined %}
+    http-check expect string {{ back_config.http_check.expect.string }}
+{%             endif %}
 {%         endif %}
 {%         if back_config.balance is vyos_defined %}
 {%             set balance_translate = {'least-connection': 'leastconn', 'round-robin': 'roundrobin', 'source-address': 'source'} %}
diff --git a/interface-definitions/load-balancing_reverse-proxy.xml.in b/interface-definitions/load-balancing_reverse-proxy.xml.in
index 645fe30c7..eb01580da 100644
--- a/interface-definitions/load-balancing_reverse-proxy.xml.in
+++ b/interface-definitions/load-balancing_reverse-proxy.xml.in
@@ -110,29 +110,55 @@
                   <leafNode name="method">
                     <properties>
                       <help>HTTP method used for health check</help>
+                      <completionHelp>
+                        <list>options head get post put</list>
+                      </completionHelp>
                       <valueHelp>
-                        <format>options|get|post|put</format>
+                        <format>options|head|get|post|put</format>
                         <description>HTTP method used for health checking</description>
                       </valueHelp>
                       <constraint>
-                        <regex>options|get|post|put</regex>
+                        <regex>(options|head|get|post|put)</regex>
                       </constraint>
                     </properties>
                   </leafNode>
                   <leafNode name="uri">
                     <properties>
                       <help>URI used for HTTP health check (Example: '/' or '/health')</help>
+                      <constraint>
+                        <regex>^\/([^?#\s]*)(\?[^#\s]*)?$</regex>
+                      </constraint>
                     </properties>
                   </leafNode>
-                  <leafNode name="expect">
+                  <node name="expect">
                     <properties>
                       <help>Expected response for the health check to pass</help>
-                      <valueHelp>
-                        <format>txt</format>
-                        <description>expected response (Example: 'status 200-299' or 'string success')</description>
-                      </valueHelp>
                     </properties>
-                  </leafNode>
+                    <children>
+                      <leafNode name="status">
+                        <properties>
+                          <help>Expected response status code for the health check to pass</help>
+                          <valueHelp>
+                            <format>u32:200-399</format>
+                            <description>Expected response code</description>
+                          </valueHelp>
+                          <constraint>
+                            <validator name="numeric" argument="--range 200-399"/>
+                          </constraint>
+                          <constraintErrorMessage>Status code must be in range 200-399</constraintErrorMessage>
+                        </properties>
+                      </leafNode>
+                      <leafNode name="string">
+                        <properties>
+                          <help>Expected to be in response body for the health check to pass</help>
+                          <valueHelp>
+                            <format>txt</format>
+                            <description>A string expected to be in the response</description>
+                          </valueHelp>
+                        </properties>
+                      </leafNode>
+                    </children>
+                  </node>
                 </children>
               </node>
               #include <include/haproxy/rule-backend.xml.i>
diff --git a/smoketest/scripts/cli/test_load-balancing_reverse-proxy.py b/smoketest/scripts/cli/test_load-balancing_reverse-proxy.py
index 8ccf2cf97..737c07401 100755
--- a/smoketest/scripts/cli/test_load-balancing_reverse-proxy.py
+++ b/smoketest/scripts/cli/test_load-balancing_reverse-proxy.py
@@ -304,15 +304,34 @@ class TestLoadBalancingReverseProxy(VyOSUnitTestSHIM.TestCase):
 
       # Set http-check
       self.cli_set(base_path + ['backend', 'bk-01', 'http-check', 'method', 'get'])
-      self.cli_set(base_path + ['backend', 'bk-01', 'http-check', 'uri', '/health'])
-      self.cli_set(base_path + ['backend', 'bk-01', 'http-check', 'expect', 'status 200'])
       self.cli_commit()
 
       # Test http-check
       config = read_file(HAPROXY_CONF)
       self.assertIn('option httpchk', config)
+      self.assertIn('http-check send meth GET', config)
+
+      # Set http-check with uri and status
+      self.cli_set(base_path + ['backend', 'bk-01', 'http-check', 'uri', '/health'])
+      self.cli_set(base_path + ['backend', 'bk-01', 'http-check', 'expect', 'status', '200'])
+      self.cli_commit()
+
+      # Test http-check with uri and status
+      config = read_file(HAPROXY_CONF)
+      self.assertIn('option httpchk', config)
       self.assertIn('http-check send meth GET uri /health', config)
       self.assertIn('http-check expect status 200', config)
 
+      # Set http-check with string
+      self.cli_delete(base_path + ['backend', 'bk-01', 'http-check', 'expect', 'status', '200'])
+      self.cli_set(base_path + ['backend', 'bk-01', 'http-check', 'expect', 'string', 'success'])
+      self.cli_commit()
+
+      # Test http-check with string
+      config = read_file(HAPROXY_CONF)
+      self.assertIn('option httpchk', config)
+      self.assertIn('http-check send meth GET uri /health', config)
+      self.assertIn('http-check expect string success', config)
+
 if __name__ == '__main__':
     unittest.main(verbosity=2)
diff --git a/src/conf_mode/load-balancing_reverse-proxy.py b/src/conf_mode/load-balancing_reverse-proxy.py
index 9f895c4e2..1569d8d71 100755
--- a/src/conf_mode/load-balancing_reverse-proxy.py
+++ b/src/conf_mode/load-balancing_reverse-proxy.py
@@ -75,6 +75,10 @@ def verify(lb):
             raise ConfigError(f'"TCP" port "{tmp_port}" is used by another service')
 
     for back, back_config in lb['backend'].items():
+        if 'http-check' in back_config:
+            http_check = back_config['http-check']
+            if 'expect' in http_check and 'status' in http_check['expect'] and 'string' in http_check['expect']:
+                raise ConfigError(f'"expect status" and "expect string" can not be configured together!')
         if 'server' not in back_config:
             raise ConfigError(f'"{back} server" must be configured!')
         for bk_server, bk_server_conf in back_config['server'].items():