From 9ed0e2d85c93c70146079d32c9854af88d80a3a3 Mon Sep 17 00:00:00 2001 From: James Page Date: Mon, 8 Nov 2021 09:12:27 +0000 Subject: [PATCH] Switch to new configuration file format For newer RabbitMQ versions, switch to using the new ini style configuration file format (rabbitmq.conf vs rabbitmq.config). This allows the charm to configure a wider set of options and is needed to support limitation of TLS versions use for on the wire encryption. Upgrades at RabbitMQ 3.7.0 should switch from old to new format and file name. Change-Id: I6deda5ecf5990d527e22373540074d2a4b7bad38 Func-Test-PR: https://github.com/openstack-charmers/zaza-openstack-tests/pull/668 --- actions/actions.py | 10 +++---- hooks/rabbit_utils.py | 24 ++++++++++++++-- hooks/rabbitmq_context.py | 2 +- hooks/rabbitmq_server_relations.py | 29 +++++++++++++++---- templates/rabbitmq.conf | 43 +++++++++++++++++++++++++++++ unit_tests/test_actions.py | 4 +-- unit_tests/test_rabbit_utils.py | 7 +++-- unit_tests/test_rabbitmq_context.py | 1 - 8 files changed, 100 insertions(+), 20 deletions(-) create mode 100644 templates/rabbitmq.conf diff --git a/actions/actions.py b/actions/actions.py index 081144ff..a0716074 100755 --- a/actions/actions.py +++ b/actions/actions.py @@ -74,13 +74,13 @@ def pause(args): """Pause the RabbitMQ services. @raises Exception should the service fail to stop. """ - pause_unit_helper(ConfigRenderer(CONFIG_FILES)) + pause_unit_helper(ConfigRenderer(CONFIG_FILES())) def resume(args): """Resume the RabbitMQ services. @raises Exception should the service fail to start.""" - resume_unit_helper(ConfigRenderer(CONFIG_FILES)) + resume_unit_helper(ConfigRenderer(CONFIG_FILES())) def cluster_status(args): @@ -130,7 +130,7 @@ def complete_cluster_series_upgrade(args): if is_leader(): # Unset cluster_series_upgrading leader_set(cluster_series_upgrading="") - assess_status(ConfigRenderer(CONFIG_FILES)) + assess_status(ConfigRenderer(CONFIG_FILES())) def forget_cluster_node(args): @@ -240,7 +240,7 @@ def restart(args): os_utils.restart_services_action(deferred_only=True) else: os_utils.restart_services_action(services=svcs) - assess_status(ConfigRenderer(CONFIG_FILES)) + assess_status(ConfigRenderer(CONFIG_FILES())) def _run_deferred_hooks(): @@ -275,7 +275,7 @@ def run_deferred_hooks(args): """ _run_deferred_hooks() os_utils.restart_services_action(deferred_only=True) - assess_status(ConfigRenderer(CONFIG_FILES)) + assess_status(ConfigRenderer(CONFIG_FILES())) def show_deferred_events(args): diff --git a/hooks/rabbit_utils.py b/hooks/rabbit_utils.py index e808046a..08be2ce3 100644 --- a/hooks/rabbit_utils.py +++ b/hooks/rabbit_utils.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import copy import json import os import re @@ -111,7 +112,8 @@ VERSION_PACKAGE = 'rabbitmq-server' RABBITMQ_CTL = '/usr/sbin/rabbitmqctl' COOKIE_PATH = '/var/lib/rabbitmq/.erlang.cookie' ENV_CONF = '/etc/rabbitmq/rabbitmq-env.conf' -RABBITMQ_CONF = '/etc/rabbitmq/rabbitmq.config' +RABBITMQ_CONFIG = '/etc/rabbitmq/rabbitmq.config' +RABBITMQ_CONF = '/etc/rabbitmq/rabbitmq.conf' ENABLED_PLUGINS = '/etc/rabbitmq/enabled_plugins' RABBIT_USER = 'rabbitmq' LIB_PATH = '/var/lib/rabbitmq/' @@ -130,7 +132,7 @@ _local_named_passwd = '/var/lib/charm/{}/{}.local_passwd' # logically, consider building a hook_context for template rendering so # the charm doesn't concern itself with template specifics etc. -CONFIG_FILES = OrderedDict([ +_CONFIG_FILES = OrderedDict([ (RABBITMQ_CONF, { 'hook_contexts': [ RabbitMQSSLContext(), @@ -138,6 +140,13 @@ CONFIG_FILES = OrderedDict([ ], 'services': ['rabbitmq-server'] }), + (RABBITMQ_CONFIG, { + 'hook_contexts': [ + RabbitMQSSLContext(), + RabbitMQClusterContext(), + ], + 'services': ['rabbitmq-server'] + }), (ENV_CONF, { 'hook_contexts': [ RabbitMQEnvContext(), @@ -151,6 +160,15 @@ CONFIG_FILES = OrderedDict([ ]) +def CONFIG_FILES(): + _cfiles = copy.deepcopy(_CONFIG_FILES) + if cmp_pkgrevno('rabbitmq-server', '3.7') >= 0: + del _cfiles[RABBITMQ_CONFIG] + else: + del _cfiles[RABBITMQ_CONF] + return _cfiles + + class ConfigRenderer(object): """ This class is a generic configuration renderer for @@ -816,7 +834,7 @@ def restart_map(): that should be restarted when file changes. ''' _map = [] - for f, ctxt in CONFIG_FILES.items(): + for f, ctxt in _CONFIG_FILES.items(): svcs = [] for svc in ctxt['services']: svcs.append(svc) diff --git a/hooks/rabbitmq_context.py b/hooks/rabbitmq_context.py index 95da30f6..316e0c33 100644 --- a/hooks/rabbitmq_context.py +++ b/hooks/rabbitmq_context.py @@ -119,7 +119,7 @@ class RabbitMQSSLContext(object): "ssl_key_file": SSL_KEY_FILE, "ssl_client": ssl_client, "ssl_ca_file": "", - "ssl_only": ssl_only + "ssl_only": ssl_only, } if ssl_ca: diff --git a/hooks/rabbitmq_server_relations.py b/hooks/rabbitmq_server_relations.py index 53095999..d477dcfc 100755 --- a/hooks/rabbitmq_server_relations.py +++ b/hooks/rabbitmq_server_relations.py @@ -535,7 +535,7 @@ def ha_joined(): level=ERROR) sys.exit(1) - ctxt = {rabbit.ENV_CONF: rabbit.CONFIG_FILES[rabbit.ENV_CONF]} + ctxt = {rabbit.ENV_CONF: rabbit.CONFIG_FILES()[rabbit.ENV_CONF]} rabbit.ConfigRenderer(ctxt).write(rabbit.ENV_CONF) relation_settings = {} @@ -733,8 +733,16 @@ def config_changed(check_deferred_restarts=True): # is not open close_port(55672) + # NOTE(jamespage): If a newer RMQ version is + # installed and the old style configuration file + # is still on disk, remove before re-rendering + # any new configuration. + if (os.path.exists(rabbit.RABBITMQ_CONFIG) and + cmp_pkgrevno('rabbitmq-server', '3.7') >= 0): + os.remove(rabbit.RABBITMQ_CONFIG) + rabbit.ConfigRenderer( - rabbit.CONFIG_FILES).write_all() + rabbit.CONFIG_FILES()).write_all() if is_relation_made("ha"): ha_is_active_active = config("ha-vip-only") @@ -809,7 +817,7 @@ def series_upgrade_prepare(): set_unit_upgrading() if not is_unit_paused_set(): log("Pausing unit for series upgrade.") - rabbit.pause_unit_helper(rabbit.ConfigRenderer(rabbit.CONFIG_FILES)) + rabbit.pause_unit_helper(rabbit.ConfigRenderer(rabbit.CONFIG_FILES())) if is_leader(): if not leader_get('cluster_series_upgrading'): # Inform the entire cluster a series upgrade is occurring. @@ -822,9 +830,18 @@ def series_upgrade_prepare(): @hooks.hook('post-series-upgrade') def series_upgrade_complete(): log("Running complete series upgrade hook", "INFO") + # NOTE(jamespage): If a newer RMQ version is + # installed and the old style configuration file + # is still on disk, remove before re-rendering + # any new configuration. + if (os.path.exists(rabbit.RABBITMQ_CONFIG) and + cmp_pkgrevno('rabbitmq-server', '3.7') >= 0): + os.remove(rabbit.RABBITMQ_CONFIG) + rabbit.ConfigRenderer( + rabbit.CONFIG_FILES()).write_all() clear_unit_paused() clear_unit_upgrading() - rabbit.resume_unit_helper(rabbit.ConfigRenderer(rabbit.CONFIG_FILES)) + rabbit.resume_unit_helper(rabbit.ConfigRenderer(rabbit.CONFIG_FILES())) @hooks.hook('certificates-relation-joined') @@ -844,7 +861,7 @@ def certs_changed(relation_id=None, unit=None): @rabbit.restart_on_change(rabbit.restart_map()) def render_and_restart(): rabbit.ConfigRenderer( - rabbit.CONFIG_FILES).write_all() + rabbit.CONFIG_FILES()).write_all() render_and_restart() update_clients() @@ -872,4 +889,4 @@ if __name__ == '__main__': level=DEBUG) update_clients() - rabbit.assess_status(rabbit.ConfigRenderer(rabbit.CONFIG_FILES)) + rabbit.assess_status(rabbit.ConfigRenderer(rabbit.CONFIG_FILES())) diff --git a/templates/rabbitmq.conf b/templates/rabbitmq.conf new file mode 100644 index 00000000..3043f036 --- /dev/null +++ b/templates/rabbitmq.conf @@ -0,0 +1,43 @@ +collect_statistics_interval = 30000 + +{%- if ssl_only %} +listeners.tcp = none +{%- endif %} + +{%- if connection_backlog %} +tcp_listen_options.backlog = {{connection_backlog}} +{%- endif %} + +{%- if ssl_port %} +listeners.ssl.1 = {{ ssl_port }} +{%- endif %} +{%- if ssl_mode == "on" or ssl_mode == "only" %} +ssl_options.verify = verify_peer +{%- if ssl_client %} +ssl_options.fail_if_no_peer_cert = true +{% else %} +ssl_options.fail_if_no_peer_cert = false +{%- endif %} +{%- if ssl_ca_file %} +ssl_options.cacertfile = {{ ssl_ca_file }} +{%- endif %} +{%- if ssl_cert_file %} +ssl_options.certfile = {{ ssl_cert_file }} +{%- endif %} +{%- if ssl_key_file %} +ssl_options.keyfile = {{ ssl_key_file }} +{%- endif %} +{%- endif %} + +{%- if mnesia_table_loading_retry_timeout %} +mnesia_table_loading_retry_timeout = {{ mnesia_table_loading_retry_timeout }} +{%- endif %} +{%- if mnesia_table_loading_retry_limit %} +mnesia_table_loading_retry_limit = {{ mnesia_table_loading_retry_limit }} +{%- endif %} +{%- if cluster_partition_handling %} +cluster_partition_handling = {{ cluster_partition_handling }} +{%- endif %} +{%- if queue_master_locator %} +queue_master_locator = {{ queue_master_locator }} +{%- endif %} diff --git a/unit_tests/test_actions.py b/unit_tests/test_actions.py index ea260130..6586ee63 100644 --- a/unit_tests/test_actions.py +++ b/unit_tests/test_actions.py @@ -33,7 +33,7 @@ class PauseTestCase(CharmTestCase): def setUp(self): super(PauseTestCase, self).setUp( - actions, ["pause_unit_helper", "ConfigRenderer"]) + actions, ["pause_unit_helper", "ConfigRenderer", "CONFIG_FILES"]) self.ConfigRenderer.return_value = 'test-config' def test_pauses_services(self): @@ -45,7 +45,7 @@ class ResumeTestCase(CharmTestCase): def setUp(self): super(ResumeTestCase, self).setUp( - actions, ["resume_unit_helper", "ConfigRenderer"]) + actions, ["resume_unit_helper", "ConfigRenderer", "CONFIG_FILES"]) self.ConfigRenderer.return_value = 'test-config' def test_pauses_services(self): diff --git a/unit_tests/test_rabbit_utils.py b/unit_tests/test_rabbit_utils.py index 5ff5746f..4d3873b3 100644 --- a/unit_tests/test_rabbit_utils.py +++ b/unit_tests/test_rabbit_utils.py @@ -915,15 +915,18 @@ class UtilsTests(CharmTestCase): "rabbit@juju-devel3-machine-42", rabbit_utils.check_cluster_memberships()) + @mock.patch.object(rabbit_utils, 'cmp_pkgrevno') @mock.patch('rabbitmq_context.psutil.NUM_CPUS', 2) @mock.patch('rabbitmq_context.relation_ids') @mock.patch('rabbitmq_context.config') - def test_render_rabbitmq_env(self, mock_config, mock_relation_ids): + def test_render_rabbitmq_env(self, mock_config, mock_relation_ids, + mock_cmp_pkgrevno): + mock_cmp_pkgrevno.return_value = 0 mock_relation_ids.return_value = [] mock_config.return_value = 3 with mock.patch('rabbit_utils.render') as mock_render: ctxt = {rabbit_utils.ENV_CONF: - rabbit_utils.CONFIG_FILES[rabbit_utils.ENV_CONF]} + rabbit_utils.CONFIG_FILES()[rabbit_utils.ENV_CONF]} rabbit_utils.ConfigRenderer(ctxt).write( rabbit_utils.ENV_CONF) diff --git a/unit_tests/test_rabbitmq_context.py b/unit_tests/test_rabbitmq_context.py index 0072bb72..1105df1f 100644 --- a/unit_tests/test_rabbitmq_context.py +++ b/unit_tests/test_rabbitmq_context.py @@ -47,7 +47,6 @@ class TestRabbitMQSSLContext(unittest.TestCase): @mock.patch("rabbitmq_context.ssl_utils.get_ssl_mode") def test_context_ssl_on(self, get_ssl_mode, reconfig_ssl, close_port, config, gr, pw, exists, chown, chmod, open_port): - exists.return_value = True get_ssl_mode.return_value = ("on", "on")