Allow for long rabbitmq node names

In some environments, the call to socket.gethostname resolves to the
fqdn. This causes the charm to use the fqdn for the rabbit hostname,
but rabbit always uses the equivalent of `hostname -s` by default. The
charm already uses the results of socket.gethostname() in relation data
everywhere, and some users have applied a workaround of
RABBITMQ_USE_LONGNAME to the rabbitmq-env.conf file in order to allow
the nodes to start and the charm to manage it.

This change detects the scenario where the socket.gethostname() call
results in a name that contains a '.' and treats this as the long node
name scenario. It will detect this on the install hook and render the
necessary configuration files and restart the rabbit service to ensure
that the node specified by the fqdn is used. This allows for the charm
to manage the rabbit cluster in these particular scenarios.

What this change does not deal with, is the potential that the hostname
resolution can change from hostname to fqdn (or vice versa). This would
require that the node be gracefully renamed in the rabbitmq-cluster and
is a much bigger/more involved change. As the current charm makes
assumptions that the hostname is stable, this is left to future
enhancements for the charm.

Related-Bug: #1976523
Change-Id: I27013e0f998fd0f0edf31b029a857af652ec6e8e

files/check_rabbitmq_cluster.py

@@ -97,7 +97,7 @@
         )
         sys.exit(3)
 
-    if(partitions > 0 or cluster < 0):
+    if (partitions > 0 or cluster < 0):
         print(
             "CRITICAL: %d partitions detected, %d nodes online."
         ) % (partitions, cluster)

hooks/rabbit_utils.py

@@ -948,7 +948,7 @@ def wait_app():
         pid_file = run_dir + 'pid'
     else:
         pid_file = '/var/lib/rabbitmq/mnesia/rabbit@' \
-                   + socket.gethostname() + '.pid'
+                   + get_unit_hostname() + '.pid'
     log('Waiting for rabbitmq app to start: {}'.format(pid_file), DEBUG)
     try:
         rabbitmqctl('wait', pid_file)
@@ -1705,6 +1705,16 @@ def get_unit_hostname():
     return socket.gethostname()
 
 
+def use_long_node_name():
+    """Return True if the rabbitmq server should use the long node name.
+
+    Determines if the socket.gethostname() returns an FQDN and configures
+    the node to use the long node name if this is the case.
+    """
+    node_name = get_unit_hostname()
+    return len(node_name.split('.')) > 1
+
+
 def is_sufficient_peers():
     """Sufficient number of expected peers to build a complete cluster
 

hooks/rabbitmq_context.py

@@ -269,6 +269,9 @@ def __call__(self):
             key = 'RABBITMQ_SERVER_START_ARGS'
             context['settings'][key] = "'-proto_dist inet6_tcp'"
 
+        if rabbit_utils.use_long_node_name():
+            context['settings']['RABBITMQ_USE_LONGNAME'] = "true"
+
         # TODO: this is legacy HA and should be removed since it is now
         # deprecated.
         if relation_ids('ha'):

hooks/rabbitmq_server_relations.py

@@ -144,6 +144,13 @@ def install():
             rabbit.CLUSTER_MODE_FOR_INSTALL))
         leader_set({rabbit.CLUSTER_MODE_KEY: rabbit.CLUSTER_MODE_FOR_INSTALL})
     rabbit.install_or_upgrade_packages()
+    # In the scenario that the long hostname is used, configure the
+    # rabbitmq-env configuration file and restart the rabbitmq-service to
+    # cause the node to use the new name.
+    if rabbit.use_long_node_name():
+        log("Long name is in use for node... configuring for long node names")
+        rabbit.ConfigRenderer(rabbit.CONFIG_FILES()).write_all()
+        service_restart('rabbitmq-server')
 
 
 def manage_restart(coordinate_restart=True):

tox.ini

@@ -64,7 +64,7 @@ deps = -r{toxinidir}/requirements.txt
 
 [testenv:pep8]
 basepython = python3
-deps = flake8==3.9.2
+deps = flake8==7.1.1
        git+https://github.com/juju/charm-tools.git
 commands = flake8 {posargs} hooks unit_tests tests actions lib files
            charm-proof

unit_tests/test_rabbit_utils.py

@@ -87,17 +87,18 @@ def test_write_all(self, log, render):
         self.assertTrue(log.called)
 
 
-RABBITMQCTL_CLUSTERSTATUS_RUNNING = b"""Cluster status of node 'rabbit@juju-devel3-machine-19' ...
-[{nodes,
-    [{disc,
+RABBITMQCTL_CLUSTERSTATUS_RUNNING = \
+    b"""Cluster status of node 'rabbit@juju-devel3-machine-19' ...
+    [{nodes,
+        [{disc,
+            ['rabbit@juju-devel3-machine-14','rabbit@juju-devel3-machine-19']},
+         {ram,
+            ['rabbit@juju-devel3-machine-42']}]},
+     {running_nodes,
         ['rabbit@juju-devel3-machine-14','rabbit@juju-devel3-machine-19']},
-     {ram,
-        ['rabbit@juju-devel3-machine-42']}]},
- {running_nodes,
-    ['rabbit@juju-devel3-machine-14','rabbit@juju-devel3-machine-19']},
- {cluster_name,<<"[email protected]">>},
- {partitions,[]}]
- """
+     {cluster_name,<<"[email protected]">>},
+     {partitions,[]}]
+     """
 
 RABBITMQCTL_CLUSTERSTATUS_RUNNING_382 = b"""
 {"running_nodes": [
@@ -111,12 +112,13 @@ def test_write_all(self, log, render):
 """
 
 
-RABBITMQCTL_CLUSTERSTATUS_SOLO = b"""Cluster status of node 'rabbit@juju-devel3-machine-14' ...
-[{nodes,[{disc,['rabbit@juju-devel3-machine-14']}]},
- {running_nodes,['rabbit@juju-devel3-machine-14']},
- {cluster_name,<<"[email protected]">>},
- {partitions,[]}]
- """
+RABBITMQCTL_CLUSTERSTATUS_SOLO = \
+    b"""Cluster status of node 'rabbit@juju-devel3-machine-14' ...
+    [{nodes,[{disc,['rabbit@juju-devel3-machine-14']}]},
+     {running_nodes,['rabbit@juju-devel3-machine-14']},
+     {cluster_name,<<"[email protected]">>},
+     {partitions,[]}]
+     """
 
 
 RABBITMQCTL_CLUSTERSTATUS_SOLO_382 = b"""
@@ -454,6 +456,14 @@ def test_get_node_hostname(self, mock_get_hostname):
                          'juju-devel3-machine-13')
         mock_get_hostname.assert_called_with('192.168.20.50', fqdn=False)
 
+    @mock.patch('socket.gethostname')
+    def test_use_long_node_name(self, mock_gethostname):
+        mock_gethostname.return_value = 'foo.bar'
+        self.assertTrue(rabbit_utils.use_long_node_name())
+
+        mock_gethostname.return_value = 'foo'
+        self.assertFalse(rabbit_utils.use_long_node_name())
+
     @mock.patch('rabbit_utils.peer_retrieve')
     def test_leader_node(self, mock_peer_retrieve):
         mock_peer_retrieve.return_value = 'juju-devel3-machine-15'

unit_tests/test_rabbitmq_context.py

@@ -374,3 +374,31 @@ def fake_config(key):
         self.assertEqual(ctxt['settings'],
                          {'RABBITMQ_SERVER_ADDITIONAL_ERL_ARGS':
                           "'+A 480'"})
+
+    @mock.patch.object(rabbitmq_context.psutil, 'NUM_CPUS', 48)
+    @mock.patch.object(rabbitmq_context, 'relation_ids', lambda *args: [])
+    @mock.patch.object(rabbitmq_context.rabbit_utils, 'use_long_node_name')
+    @mock.patch.object(rabbitmq_context, 'service_name')
+    @mock.patch.object(rabbitmq_context, 'config')
+    def test_rabbitmqenv_long_nodename(self, mock_config, mock_service_name,
+                                       use_long_node_name):
+        config = dict()
+
+        def fake_config(key):
+            return config.get(key)
+
+        mock_config.side_effect = fake_config
+        mock_service_name.return_value = 'svc_bar'
+        use_long_node_name.return_value = True
+
+        ctxt = rabbitmq_context.RabbitMQEnvContext()()
+        self.assertEqual(ctxt['settings'], {
+            'RABBITMQ_SERVER_ADDITIONAL_ERL_ARGS': "'+A 1024'",
+            'RABBITMQ_USE_LONGNAME': "true",
+        })
+
+        use_long_node_name.return_value = False
+        ctxt = rabbitmq_context.RabbitMQEnvContext()()
+        self.assertEqual(ctxt['settings'], {
+            'RABBITMQ_SERVER_ADDITIONAL_ERL_ARGS': "'+A 1024'",
+        })

unit_tests/test_rabbitmq_server_relations.py

@@ -44,6 +44,7 @@
     'related_units',
     'coordinator',
     'deferred_events',
+    'log',
 ]
 
 
@@ -58,6 +59,55 @@ def tearDown(self):
         shutil.rmtree(self.tmp_dir)
         super(RelationUtil, self).tearDown()
 
+    @patch('charmhelpers.contrib.hardening.harden.config')
+    @patch('rabbitmq_server_relations.config')
+    @patch('rabbitmq_server_relations.pre_install_hooks')
+    @patch('rabbitmq_server_relations.add_source')
+    @patch('rabbitmq_server_relations.leader_get')
+    @patch('rabbitmq_server_relations.leader_set')
+    @patch('rabbitmq_server_relations.service_restart')
+    @patch('rabbitmq_server_relations.rabbit')
+    def test_install_shortnames(self, rabbit, service_restart, leader_set,
+                                leader_get, add_source, pre_install_hooks,
+                                config, _):
+        config.side_effect = ['', '']
+        self.is_leader.return_value = False
+        rabbit.use_long_node_name.return_value = False
+
+        rabbitmq_server_relations.install()
+
+        pre_install_hooks.assert_called_once()
+        add_source.assert_called_once_with('', '')
+        leader_set.assert_not_called()
+        leader_get.assert_not_called()
+        rabbit.install_or_upgrade_packages.assert_called()
+        rabbit.ConfigRender.write_all.assert_not_called()
+        service_restart.assert_not_called()
+
+    @patch('charmhelpers.contrib.hardening.harden.config')
+    @patch('rabbitmq_server_relations.config')
+    @patch('rabbitmq_server_relations.pre_install_hooks')
+    @patch('rabbitmq_server_relations.add_source')
+    @patch('rabbitmq_server_relations.leader_get')
+    @patch('rabbitmq_server_relations.leader_set')
+    @patch('rabbitmq_server_relations.service_restart')
+    @patch('rabbitmq_server_relations.rabbit')
+    def test_install_longnames(self, rabbit, service_restart, leader_set,
+                               leader_get, add_source, pre_install_hooks,
+                               config, _):
+        config.side_effect = ['', '']
+        self.is_leader.return_value = False
+        rabbit.use_long_node_name.return_value = True
+
+        rabbitmq_server_relations.install()
+
+        pre_install_hooks.assert_called_once()
+        add_source.assert_called_once_with('', '')
+        leader_set.assert_not_called()
+        leader_get.assert_not_called()
+        rabbit.install_or_upgrade_packages.assert_called_once()
+        service_restart.assert_called_once_with('rabbitmq-server')
+
     @patch('rabbitmq_server_relations.is_hook_allowed')
     @patch('rabbitmq_server_relations.rabbit.leader_node_is_ready')
     @patch('rabbitmq_server_relations.peer_store_and_set')