Fix PING health-monitor with recent haproxy releases

haproxy 2.2.x requires an "insecure-fork-wanted" global option when
using external-check.

This commit also fixes a function that builds the list of the features
available in haproxy based on its version.

Story 2009953
Task 44900

Change-Id: I35c5976c6bdb8828e54bcde00052622a7b6bc96a

octavia/common/constants.py

@@ -796,6 +796,7 @@ AMP_NETNS_SVC_PREFIX = 'amphora-netns'
 # Amphora Feature Compatibility
 HTTP_REUSE = 'has_http_reuse'
 POOL_ALPN = 'has_pool_alpn'
+INSECURE_FORK = 'requires_insecure_fork'
 
 # TODO(johnsom) convert these to octavia_lib constants
 # once octavia is transitioned to use octavia_lib

octavia/common/jinja/haproxy/combined_listeners/jinja_cfg.py

@@ -17,6 +17,7 @@ import re
 
 import jinja2
 from octavia_lib.common import constants as lib_consts
+from oslo_utils import versionutils
 
 from octavia.common.config import cfg
 from octavia.common import constants
@@ -98,13 +99,17 @@ class JinjaTemplater(object):
         # pair might be running an older amphora version.
 
         feature_compatibility = {}
+        version = ".".join(haproxy_versions)
         # Is it newer than haproxy 1.5?
-        if not (int(haproxy_versions[0]) < 2 and int(haproxy_versions[1]) < 6):
+        if versionutils.is_compatible("1.6.0", version, same_major=False):
             feature_compatibility[constants.HTTP_REUSE] = True
-        if not (int(haproxy_versions[0]) < 2 and int(haproxy_versions[1]) < 9):
+        if versionutils.is_compatible("1.9.0", version, same_major=False):
             feature_compatibility[constants.POOL_ALPN] = True
         if int(haproxy_versions[0]) >= 2:
             feature_compatibility[lib_consts.PROTOCOL_PROMETHEUS] = True
+        # haproxy 2.2 requires insecure-fork-wanted for PING healthchecks
+        if versionutils.is_compatible("2.2.0", version, same_major=False):
+            feature_compatibility[constants.INSECURE_FORK] = True
 
         return self.render_loadbalancer_obj(
             host_amphora, listeners, tls_certs=tls_certs,
@@ -173,6 +178,8 @@ class JinjaTemplater(object):
             if listener.protocol == lib_consts.PROTOCOL_PROMETHEUS:
                 prometheus_listener = True
                 break
+        require_insecure_fork = feature_compatibility.get(
+            constants.INSECURE_FORK)
         enable_prometheus = prometheus_listener and feature_compatibility.get(
             lib_consts.PROTOCOL_PROMETHEUS, False)
         return self._get_template().render(
@@ -185,7 +192,8 @@ class JinjaTemplater(object):
                  CONF.amphora_agent.administrative_log_facility,
              'user_log_facility': CONF.amphora_agent.user_log_facility,
              'connection_logging': self.connection_logging,
-             'enable_prometheus': enable_prometheus},
+             'enable_prometheus': enable_prometheus,
+             'require_insecure_fork': require_insecure_fork},
             constants=constants, lib_consts=lib_consts)
 
     def _transform_loadbalancer(self, host_amphora, loadbalancer, listeners,

octavia/common/jinja/haproxy/combined_listeners/templates/base.j2

@@ -32,6 +32,9 @@ global
                   found_ns.found == false %}
                 {% set found_ns.found = true %}
     external-check
+                {% if require_insecure_fork %}
+    insecure-fork-wanted
+                {% endif %}
             {% endif %}
         {% endfor %}
     {% endfor %}

octavia/tests/unit/common/jinja/haproxy/combined_listeners/test_jinja_cfg.py

@@ -15,6 +15,7 @@
 
 import copy
 import os
+from unittest import mock
 
 from octavia_lib.common import constants as lib_consts
 from oslo_config import cfg
@@ -771,6 +772,37 @@ class TestHaproxyCfg(base.TestCase):
         self.assertEqual(sample_configs_combined.sample_base_expected_config(
             backend=be, global_opts=go), rendered_obj)
 
+    def test_render_template_ping_monitor_http_insecure_fork(self):
+        be = ("backend sample_pool_id_1:sample_listener_id_1\n"
+              "    mode http\n"
+              "    balance roundrobin\n"
+              "    cookie SRV insert indirect nocache\n"
+              "    load-server-state-from-file global\n"
+              "    timeout check 31s\n"
+              "    option external-check\n"
+              "    external-check command /var/lib/octavia/ping-wrapper.sh\n"
+              "    fullconn {maxconn}\n"
+              "    option allbackups\n"
+              "    timeout connect 5000\n"
+              "    timeout server 50000\n"
+              "    server sample_member_id_1 10.0.0.99:82 "
+              "weight 13 check inter 30s fall 3 rise 2 "
+              "cookie sample_member_id_1\n"
+              "    server sample_member_id_2 10.0.0.98:82 "
+              "weight 13 check inter 30s fall 3 rise 2 "
+              "cookie sample_member_id_2\n\n").format(
+            maxconn=constants.HAPROXY_DEFAULT_MAXCONN)
+        go = (f"    maxconn {constants.HAPROXY_DEFAULT_MAXCONN}\n"
+              "    external-check\n    insecure-fork-wanted\n\n")
+        rendered_obj = self.jinja_cfg.render_loadbalancer_obj(
+            sample_configs_combined.sample_amphora_tuple(),
+            [sample_configs_combined.sample_listener_tuple(
+                proto='HTTP', monitor_proto='PING')],
+            feature_compatibility={
+                "requires_insecure_fork": True})
+        self.assertEqual(sample_configs_combined.sample_base_expected_config(
+            backend=be, global_opts=go), rendered_obj)
+
     def test_render_template_no_monitor_https(self):
         fe = ("frontend sample_listener_id_1\n"
               "    maxconn {maxconn}\n"
@@ -1758,3 +1790,106 @@ class TestHaproxyCfg(base.TestCase):
             sample_configs_combined.sample_base_expected_config(
                 frontend=fe, backend=be),
             rendered_obj)
+
+    @mock.patch("octavia.common.jinja.haproxy.combined_listeners.jinja_cfg."
+                "JinjaTemplater.render_loadbalancer_obj")
+    def test_build_config(self, mock_render_loadbalancer_obj):
+        mock_amp = mock.Mock()
+        mock_listeners = mock.Mock()
+        mock_tls_certs = mock.Mock()
+        mock_socket_path = mock.Mock()
+
+        j_cfg = jinja_cfg.JinjaTemplater()
+        j_cfg.build_config(mock_amp, mock_listeners, mock_tls_certs,
+                           haproxy_versions=("0", "7", "0"),
+                           socket_path=mock_socket_path)
+
+        expected_fc = {}
+        mock_render_loadbalancer_obj.assert_called_once_with(
+            mock_amp, mock_listeners, tls_certs=mock_tls_certs,
+            socket_path=mock_socket_path,
+            feature_compatibility=expected_fc)
+
+        mock_render_loadbalancer_obj.reset_mock()
+
+        j_cfg.build_config(mock_amp, mock_listeners, mock_tls_certs,
+                           haproxy_versions=("1", "6", "0"),
+                           socket_path=mock_socket_path)
+
+        expected_fc = {
+            constants.HTTP_REUSE: True
+        }
+        mock_render_loadbalancer_obj.assert_called_once_with(
+            mock_amp, mock_listeners, tls_certs=mock_tls_certs,
+            socket_path=mock_socket_path,
+            feature_compatibility=expected_fc)
+
+        mock_render_loadbalancer_obj.reset_mock()
+
+        j_cfg.build_config(mock_amp, mock_listeners, mock_tls_certs,
+                           haproxy_versions=("1", "9", "0"),
+                           socket_path=mock_socket_path)
+
+        expected_fc = {
+            constants.HTTP_REUSE: True,
+            constants.POOL_ALPN: True
+        }
+        mock_render_loadbalancer_obj.assert_called_once_with(
+            mock_amp, mock_listeners, tls_certs=mock_tls_certs,
+            socket_path=mock_socket_path,
+            feature_compatibility=expected_fc)
+
+        mock_render_loadbalancer_obj.reset_mock()
+
+        j_cfg.build_config(mock_amp, mock_listeners, mock_tls_certs,
+                           haproxy_versions=("2", "1", "1"),
+                           socket_path=mock_socket_path)
+
+        expected_fc = {
+            constants.HTTP_REUSE: True,
+            constants.POOL_ALPN: True,
+            lib_consts.PROTOCOL_PROMETHEUS: True
+        }
+        mock_render_loadbalancer_obj.assert_called_once_with(
+            mock_amp, mock_listeners, tls_certs=mock_tls_certs,
+            socket_path=mock_socket_path,
+            feature_compatibility=expected_fc)
+
+        mock_render_loadbalancer_obj.reset_mock()
+
+        j_cfg.build_config(mock_amp, mock_listeners, mock_tls_certs,
+                           haproxy_versions=("2", "2", "1"),
+                           socket_path=mock_socket_path)
+
+        expected_fc = {
+            constants.HTTP_REUSE: True,
+            constants.POOL_ALPN: True,
+            lib_consts.PROTOCOL_PROMETHEUS: True,
+            constants.INSECURE_FORK: True
+        }
+        mock_render_loadbalancer_obj.assert_called_once_with(
+            mock_amp, mock_listeners, tls_certs=mock_tls_certs,
+            socket_path=mock_socket_path,
+            feature_compatibility=expected_fc)
+
+        mock_render_loadbalancer_obj.reset_mock()
+
+        j_cfg.build_config(mock_amp, mock_listeners, mock_tls_certs,
+                           haproxy_versions=("2", "4", "0"),
+                           socket_path=mock_socket_path)
+
+        mock_render_loadbalancer_obj.assert_called_once_with(
+            mock_amp, mock_listeners, tls_certs=mock_tls_certs,
+            socket_path=mock_socket_path,
+            feature_compatibility=expected_fc)
+
+        mock_render_loadbalancer_obj.reset_mock()
+
+        j_cfg.build_config(mock_amp, mock_listeners, mock_tls_certs,
+                           haproxy_versions=("3", "1", "0"),
+                           socket_path=mock_socket_path)
+
+        mock_render_loadbalancer_obj.assert_called_once_with(
+            mock_amp, mock_listeners, tls_certs=mock_tls_certs,
+            socket_path=mock_socket_path,
+            feature_compatibility=expected_fc)

releasenotes/notes/fix-ping-hm-with-haproxy-2.2-9b83777172fb8835.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Fix PING health-monitors with recent haproxy releases (>=2.2), haproxy now
+    requires an additional "insecure-fork-wanted" option to authorize the
+    Octavia PING healthcheck.