From ed94cfb94144fba07d07c472fbec5e9e6e8f0e43 Mon Sep 17 00:00:00 2001 From: "Gael Chamoulaud (Strider)" Date: Tue, 20 Apr 2021 07:20:58 +0200 Subject: [PATCH] Migrate back haproxy validation to tripleo-validations This patch moves back the haproxy validation from validations-common to tripleo-validations. It has been renamed in tripleo-haproxy and the role is called now tripleo_haproxy. This patch also fix the host targeting in order to only execute this validation on nodes where haproxy is really installed and configured. Note that the haproxy validations hosted in validations-common will be removed in a follow up patch. Closes-Bug: #1926024 Co-Authored-By: Michele Baldessari Signed-off-by: Gael Chamoulaud (Strider) Change-Id: I9869afd60342fdb0aca69a313c1c6e35e0947fb8 (cherry picked from commit 9dd662a1b9d5617e1a929c86f6ffdb09e873252b) (cherry picked from commit 253a16de422b5f22bb1b9e68ffa14e855a22d243) (cherry picked from commit bb5a538a56bc860c25ec32866ec25923099cbecc) (cherry picked from commit 2744976e606e87e8f4c3c9da0ab7d6018c3f1f2d) --- .../modules/modules-tripleo_haproxy_conf.rst | 14 +++ doc/source/roles/role-tripleo_haproxy.rst | 46 ++++++++ library/tripleo_haproxy_conf.py | 108 ++++++++++++++++++ playbooks/tripleo-haproxy.yaml | 26 +++++ roles/tripleo_haproxy/defaults/main.yml | 21 ++++ .../molecule/default/converge.yml | 71 ++++++++++++ .../molecule/default/molecule.yml | 3 + roles/tripleo_haproxy/tasks/main.yml | 56 +++++++++ .../library/test_tripleo_haproxy_conf.py | 56 +++++++++ zuul.d/molecule.yaml | 12 ++ 10 files changed, 413 insertions(+) create mode 100644 doc/source/modules/modules-tripleo_haproxy_conf.rst create mode 100644 doc/source/roles/role-tripleo_haproxy.rst create mode 100644 library/tripleo_haproxy_conf.py create mode 100644 playbooks/tripleo-haproxy.yaml create mode 100644 roles/tripleo_haproxy/defaults/main.yml create mode 100644 roles/tripleo_haproxy/molecule/default/converge.yml create mode 100644 roles/tripleo_haproxy/molecule/default/molecule.yml create mode 100644 roles/tripleo_haproxy/tasks/main.yml create mode 100644 tripleo_validations/tests/library/test_tripleo_haproxy_conf.py diff --git a/doc/source/modules/modules-tripleo_haproxy_conf.rst b/doc/source/modules/modules-tripleo_haproxy_conf.rst new file mode 100644 index 000000000..23b1de6ed --- /dev/null +++ b/doc/source/modules/modules-tripleo_haproxy_conf.rst @@ -0,0 +1,14 @@ +============================= +Module - tripleo_haproxy_conf +============================= + + +This module provides for the following ansible plugin: + + * tripleo_haproxy_conf + + +.. ansibleautoplugin:: + :module: library/tripleo_haproxy_conf.py + :documentation: true + :examples: true diff --git a/doc/source/roles/role-tripleo_haproxy.rst b/doc/source/roles/role-tripleo_haproxy.rst new file mode 100644 index 000000000..a95248550 --- /dev/null +++ b/doc/source/roles/role-tripleo_haproxy.rst @@ -0,0 +1,46 @@ +=============== +tripleo_haproxy +=============== + +-------------- +About The Role +-------------- + +An Ansible role to check if the ``HAProxy`` configuration has recommended +values. + +Requirements +============ + +This role requires and Up and Running Overcloud. + +Dependencies +============ + +None. + +Example Playbook +================ + +.. code-block:: yaml + + - hosts: undercloud + roles: + - { role: tripleo_haproxy } + +License +======= + +Apache + +Author Information +================== + +**Red Hat Tripleo DFG:PIDONE** + +---------------- +Full Description +---------------- + +.. ansibleautoplugin:: + :role: roles/tripleo_haproxy diff --git a/library/tripleo_haproxy_conf.py b/library/tripleo_haproxy_conf.py new file mode 100644 index 000000000..926e7ff23 --- /dev/null +++ b/library/tripleo_haproxy_conf.py @@ -0,0 +1,108 @@ +# -*- coding: utf-8 -*- +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import re + +from ansible.module_utils.basic import AnsibleModule +from yaml import safe_load as yaml_safe_load + +DOCUMENTATION = ''' +--- +module: tripleo_haproxy_conf +short_description: Gather the HAProxy config +description: + - Gather the HAProxy config + - Owned by DFG:PIDONE +options: + path: + required: true + description: + - file path to the config file + type: str +author: "Tomas Sedovic" +''' + +EXAMPLES = ''' +- hosts: webservers + tasks: + - name: Gather the HAProxy config + tripleo_haproxy_conf: path=/etc/haproxy/haproxy.cfg +''' + + +def generic_ini_style_conf_parser(file_path, section_regex, option_regex): + """ + ConfigParser chokes on both mariadb and haproxy files. Luckily, they have + a syntax approaching ini config file so they are relatively easy to parse. + This generic ini style config parser is not perfect, as it can ignore some + valid options, but it is good enough for our use case. + + :return: parsed haproxy configuration + :rtype: dict + """ + config = {} + current_section = None + with open(file_path) as config_file: + for line in config_file: + match_section = re.match(section_regex, line) + if match_section: + current_section = match_section.group(1) + config[current_section] = {} + match_option = re.match(option_regex, line) + if match_option and current_section: + option = re.sub(r'\s+', ' ', match_option.group(1)) + config[current_section][option] = match_option.group(2) + return config + + +def parse_haproxy_conf(file_path): + """ + Provides section and option regex to the parser. + Essentially a wrapper for generic_ini_style_conf_parser. + Provides no extra functionality but simplifies the call, somewhat. + + :return: parsed haproxy configuration + :rtype: dict + + ..note:: + + Both regular expressions bellow are used for parsing haproxy.cfg, + which has a rather vague syntax. The regexes are supposed to match all + possibilities described here, and some more: + https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/7/html/load_balancer_administration/ch-haproxy-setup-vsa + """ + section_regex = r'^(\w+)' + option_regex = r'^(?:\s+)(\w+(?:\s+\w+)*?)\s+([\w/]*)$' + return generic_ini_style_conf_parser(file_path, section_regex, + option_regex) + + +def main(): + module = AnsibleModule( + argument_spec=yaml_safe_load(DOCUMENTATION)['options'] + ) + + haproxy_conf_path = module.params.get('path') + + try: + config = parse_haproxy_conf(haproxy_conf_path) + except IOError: + module.fail_json(msg="Could not open the haproxy conf file at: '%s'" % + haproxy_conf_path) + + module.exit_json(changed=False, ansible_facts={u'haproxy_conf': config}) + + +if __name__ == '__main__': + main() diff --git a/playbooks/tripleo-haproxy.yaml b/playbooks/tripleo-haproxy.yaml new file mode 100644 index 000000000..c1c7c91f4 --- /dev/null +++ b/playbooks/tripleo-haproxy.yaml @@ -0,0 +1,26 @@ +--- +- hosts: haproxy + vars: + metadata: + name: TripleO HAProxy configuration + description: Verify the HAProxy configuration has recommended values. + groups: + - post-deployment + categories: + - os + - system + - ha + - loadbalancing + - proxy + products: + - tripleo + - haproxy + config_file: '/var/lib/config-data/puppet-generated/haproxy/etc/haproxy/haproxy.cfg' + global_maxconn_min: 20480 + defaults_maxconn_min: 4096 + defaults_timeout_queue: '2m' + defaults_timeout_client: '2m' + defaults_timeout_server: '2m' + defaults_timeout_check: '10s' + roles: + - tripleo_haproxy diff --git a/roles/tripleo_haproxy/defaults/main.yml b/roles/tripleo_haproxy/defaults/main.yml new file mode 100644 index 000000000..cc8d36527 --- /dev/null +++ b/roles/tripleo_haproxy/defaults/main.yml @@ -0,0 +1,21 @@ +--- +# Path to the haproxy.cfg file +haproxy_config_file: '/var/lib/config-data/puppet-generated/haproxy/etc/haproxy/haproxy.cfg' + +# Global mininum per-process number of concurrent connections +global_maxconn_min: 20480 + +# Defaults mininum per-process number of concurrent connections +defaults_maxconn_min: 4096 + +# Time to wait in the queue for a connection slot to be free +defaults_timeout_queue: '2m' + +# Inactivity time on the client side +defaults_timeout_client: '2m' + +# Inactivity time on the server side +defaults_timeout_server: '2m' + +# Additional check timeout +defaults_timeout_check: '10s' diff --git a/roles/tripleo_haproxy/molecule/default/converge.yml b/roles/tripleo_haproxy/molecule/default/converge.yml new file mode 100644 index 000000000..cd34c19f3 --- /dev/null +++ b/roles/tripleo_haproxy/molecule/default/converge.yml @@ -0,0 +1,71 @@ +--- +# Copyright 2021 Red Hat, Inc. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + + +- name: Converge + hosts: all + gather_facts: false + + vars: + haproxy_config_file: /haproxy.cfg + + tasks: + - name: create haproxy config file + copy: + dest: /haproxy.cfg + content: | + # This file managed by Puppet + global + daemon + group haproxy + log /dev/log local0 + maxconn 100 + pidfile /var/run/haproxy.pid + ssl-default-bind-ciphers !SSLv2:kEECDH:kRSA:kEDH:kPSK:+3DES:!aNULL:!eNULL:!MD5:!EXP:!RC4:!SEED:!IDEA:!DES + ssl-default-bind-options no-sslv3 no-tlsv10 + stats socket /var/lib/haproxy/stats mode 600 level user + stats timeout 1s + user haproxy + + defaults + log global + maxconn 100 + mode tcp + retries 1 + timeout http-request 1s + timeout queue 1s + timeout connect 1s + timeout client 1s + timeout server 2m + timeout check 10s + - block: + - include_role: + name: tripleo_haproxy + rescue: + - name: Clear host errors + meta: clear_host_errors + + - debug: + msg: The validation works! End the playbook run + + - name: End play + meta: end_play + + - name: Fail the test + fail: + msg: | + The haproxy role should have detected issues within haproxy + configuration file! diff --git a/roles/tripleo_haproxy/molecule/default/molecule.yml b/roles/tripleo_haproxy/molecule/default/molecule.yml new file mode 100644 index 000000000..ba05cf07d --- /dev/null +++ b/roles/tripleo_haproxy/molecule/default/molecule.yml @@ -0,0 +1,3 @@ +--- +# inherits tripleo-validations/.config/molecule/config.yml +# To override default values, please take a look at the config.yml. diff --git a/roles/tripleo_haproxy/tasks/main.yml b/roles/tripleo_haproxy/tasks/main.yml new file mode 100644 index 000000000..2fd1de48c --- /dev/null +++ b/roles/tripleo_haproxy/tasks/main.yml @@ -0,0 +1,56 @@ +--- +- name: Gather the HAProxy configuration + become: true + tripleo_haproxy_conf: + path: "{{ haproxy_config_file }}" + +- name: Check the HAProxy configuration + fail: + msg: >- + {% if haproxy_conf.global.maxconn|int < global_maxconn_min %} + - [FAILED] 'global maxconn' value check + * Current value: {{ haproxy_conf.global.maxconn }}, Recommended value: > {{ global_maxconn_min }} + {% else %} + - [PASSED] 'global maxconn' value check + {% endif %} + {% if haproxy_conf.defaults.maxconn|int < defaults_maxconn_min %} + - [FAILED] 'defaults maxconn' value check + * Current value: {{ haproxy_conf.defaults.maxconn }}, Recommended Value: > {{ defaults_maxconn_min }} + {% else %} + - [PASSED] 'defaults maxconn' value check + {% endif %} + {% if haproxy_conf.defaults['timeout queue'] != defaults_timeout_queue %} + - [FAILED] 'timeout queue' option in 'defaults' check + * Current value: {{ haproxy_conf.defaults['timeout queue'] }} + * Recommended value: {{ defaults_timeout_queue }} + {% else %} + - [PASSED] 'timeout queue' option in 'defaults' check + {% endif %} + {% if haproxy_conf.defaults['timeout client'] != defaults_timeout_client %} + - [FAILED] 'timeout client' option in 'defaults' check + * Current value: {{ haproxy_conf.defaults['timeout client'] }} + * Recommended value: {{ defaults_timeout_client }} + {% else %} + - [PASSED] 'timeout client' option in 'defaults' check + {% endif %} + {% if haproxy_conf.defaults['timeout server'] != defaults_timeout_server %} + - [FAILED] 'timeout server' option in 'defaults' check + * Current value: {{ haproxy_conf.defaults['timeout server'] }} + * Recommended value: {{ defaults_timeout_server }} + {% else %} + - [PASSED] 'timeout server' option in 'defaults' check + {% endif %} + {% if haproxy_conf.defaults['timeout check'] != defaults_timeout_check %} + - [FAILED] 'timeout check' option in 'defaults' check + * Current value: {{ haproxy_conf.defaults['timeout check'] }} + * Recommended value: {{ defaults_timeout_check }} + {% else %} + - [PASSED] 'timeout check' option in 'defaults' check + {% endif %} + failed_when: > + (haproxy_conf.global.maxconn|int < global_maxconn_min) or + (haproxy_conf.defaults.maxconn|int < defaults_maxconn_min) or + (haproxy_conf.defaults['timeout queue'] != defaults_timeout_queue) or + (haproxy_conf.defaults['timeout client'] != defaults_timeout_client) or + (haproxy_conf.defaults['timeout server'] != defaults_timeout_server) or + (haproxy_conf.defaults['timeout check'] != defaults_timeout_check) diff --git a/tripleo_validations/tests/library/test_tripleo_haproxy_conf.py b/tripleo_validations/tests/library/test_tripleo_haproxy_conf.py new file mode 100644 index 000000000..1341a3dcd --- /dev/null +++ b/tripleo_validations/tests/library/test_tripleo_haproxy_conf.py @@ -0,0 +1,56 @@ +# -*- coding: utf-8 -*- + + +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +try: + from unittest import mock +except ImportError: + import mock + +from tripleo_validations.tests import base + +from library import tripleo_haproxy_conf + + +class TestHaproxyConf(base.TestCase): + def setUp(self): + super(TestHaproxyConf, self).setUp() + self.h_conf = tripleo_haproxy_conf + + @mock.patch('library.tripleo_haproxy_conf.generic_ini_style_conf_parser') + def test_parse_haproxy_conf(self, mock_generic_ini_style_conf_parser): + """ Despite the appearences this test is not using regex at all. + These are merely raw strings, that it asserts are passed to the `generic_ini_style_conf_parser`. + From the pov of the test it is irrelevant what form they have. + It's the `generic_ini_style_conf_parser` function that is supposed to receive these strings as arguments. + Test is merely checking that the code immediately preceding it's call does what it should do. + The regexes are finally used for parsing haproxy.cfg, which has a rather vague syntax. + In short: The regexes are supposed to match all possibilities described here, and some more: + https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/7/html/load_balancer_administration/ch-haproxy-setup-vsa + """ + + file_path = './foo/bar' + + args = { + 'file_path': file_path, + 'section_regex': r'^(\w+)', + 'option_regex': r'^(?:\s+)(\w+(?:\s+\w+)*?)\s+([\w/]*)$' + } + + self.h_conf.parse_haproxy_conf(file_path) + mock_generic_ini_style_conf_parser.assert_called_once_with( + args['file_path'], + args['section_regex'], + args['option_regex'] + ) diff --git a/zuul.d/molecule.yaml b/zuul.d/molecule.yaml index c8dc304b2..428411a87 100644 --- a/zuul.d/molecule.yaml +++ b/zuul.d/molecule.yaml @@ -20,6 +20,7 @@ - tripleo-validations-centos-8-molecule-stonith_exists - tripleo-validations-centos-8-molecule-system_encoding - tripleo-validations-centos-8-molecule-tls_everywhere + - tripleo-validations-centos-8-molecule-tripleo_haproxy - tripleo-validations-centos-8-molecule-undercloud_debug - tripleo-validations-centos-8-molecule-undercloud_heat_purge_deleted - tripleo-validations-centos-8-molecule-undercloud_tokenflush @@ -44,6 +45,7 @@ - tripleo-validations-centos-8-molecule-stonith_exists - tripleo-validations-centos-8-molecule-system_encoding - tripleo-validations-centos-8-molecule-tls_everywhere + - tripleo-validations-centos-8-molecule-tripleo_haproxy - tripleo-validations-centos-8-molecule-undercloud_debug - tripleo-validations-centos-8-molecule-undercloud_heat_purge_deleted - tripleo-validations-centos-8-molecule-undercloud_tokenflush @@ -492,3 +494,13 @@ parent: tripleo-validations-centos-8-base vars: tripleo_validations_role_name: compute_tsx +- job: + files: + - ^roles/tripleo_haproxy/.* + - ^tests/prepare-test-host.yml + - ^ci/playbooks/pre.yml + - ^ci/playbooks/run.yml + name: tripleo-validations-centos-8-molecule-tripleo_haproxy + parent: tripleo-validations-centos-8-base + vars: + tripleo_validations_role_name: tripleo_haproxy