From 1ac781812872d4219cf245cf18ca3e93ec1bb035 Mon Sep 17 00:00:00 2001 From: Gregory Thiemonge Date: Fri, 1 Apr 2022 06:56:54 +0200 Subject: [PATCH] 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 | 1 + .../haproxy/combined_listeners/jinja_cfg.py | 14 +- .../combined_listeners/templates/base.j2 | 3 + .../combined_listeners/test_jinja_cfg.py | 135 ++++++++++++++++++ ...-hm-with-haproxy-2.2-9b83777172fb8835.yaml | 6 + 5 files changed, 156 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/fix-ping-hm-with-haproxy-2.2-9b83777172fb8835.yaml diff --git a/octavia/common/constants.py b/octavia/common/constants.py index 9c3d4721ec..6e737e89f7 100644 --- a/octavia/common/constants.py +++ b/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 diff --git a/octavia/common/jinja/haproxy/combined_listeners/jinja_cfg.py b/octavia/common/jinja/haproxy/combined_listeners/jinja_cfg.py index 73013bb756..6677147a22 100644 --- a/octavia/common/jinja/haproxy/combined_listeners/jinja_cfg.py +++ b/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, diff --git a/octavia/common/jinja/haproxy/combined_listeners/templates/base.j2 b/octavia/common/jinja/haproxy/combined_listeners/templates/base.j2 index b2a6dc1ad1..7399c5859a 100644 --- a/octavia/common/jinja/haproxy/combined_listeners/templates/base.j2 +++ b/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 %} diff --git a/octavia/tests/unit/common/jinja/haproxy/combined_listeners/test_jinja_cfg.py b/octavia/tests/unit/common/jinja/haproxy/combined_listeners/test_jinja_cfg.py index 9f4ead9cea..87de66bdf4 100644 --- a/octavia/tests/unit/common/jinja/haproxy/combined_listeners/test_jinja_cfg.py +++ b/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) diff --git a/releasenotes/notes/fix-ping-hm-with-haproxy-2.2-9b83777172fb8835.yaml b/releasenotes/notes/fix-ping-hm-with-haproxy-2.2-9b83777172fb8835.yaml new file mode 100644 index 0000000000..a16e0604e4 --- /dev/null +++ b/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.