From 7ff27d9e10acb657fb4b0a73136e6c806412fe79 Mon Sep 17 00:00:00 2001 From: Igor Yanchenko <1504692+yanchenko-igor@users.noreply.github.com> Date: Wed, 11 Dec 2019 12:26:17 +0100 Subject: [PATCH] Make sure unix_socket_directories and stats_temp_directory exist (#1293) Upon the start of Patroni and Postgres make sure that unix_socket_directories and stats_temp_directory exist or try to create them. Patroni will exit if failed to create them. Close https://github.com/zalando/patroni/issues/863 --- patroni/postgresql/__init__.py | 2 ++ patroni/postgresql/config.py | 16 +++++++++++++++- patroni/utils.py | 26 ++++++++++++++++++++++++++ tests/__init__.py | 3 ++- tests/test_utils.py | 25 ++++++++++++++++++++++++- 5 files changed, 69 insertions(+), 3 deletions(-) diff --git a/patroni/postgresql/__init__.py b/patroni/postgresql/__init__.py index be864fba..45ede6a3 100644 --- a/patroni/postgresql/__init__.py +++ b/patroni/postgresql/__init__.py @@ -60,6 +60,7 @@ class Postgresql(object): self._pending_restart = False self._connection = Connection() self.config = ConfigHandler(self, config) + self.config.check_directories() self._bin_dir = config.get('bin_dir') or '' self.bootstrap = Bootstrap(self) @@ -407,6 +408,7 @@ class Postgresql(object): self._pending_restart = False configuration = self.config.effective_configuration + self.config.check_directories() self.config.write_postgresql_conf(configuration) self.config.resolve_connection_addresses() self.config.replace_pg_hba() diff --git a/patroni/postgresql/config.py b/patroni/postgresql/config.py index db9926fd..70eafcca 100644 --- a/patroni/postgresql/config.py +++ b/patroni/postgresql/config.py @@ -10,7 +10,8 @@ from six.moves.urllib_parse import urlparse, parse_qsl, unquote from urllib3.response import HTTPHeaderDict from ..dcs import slot_name_from_member_name, RemoteMember -from ..utils import compare_values, parse_bool, parse_int, split_host_port, uri +from ..utils import compare_values, parse_bool, parse_int, split_host_port, uri, \ + validate_directory, is_subpath logger = logging.getLogger(__name__) @@ -347,6 +348,19 @@ class ConfigHandler(object): self._server_parameters = self.get_server_parameters(self._config) self._adjust_recovery_parameters() + def try_to_create_dir(self, d, msg): + d = os.path.join(self._postgresql._data_dir, d) + if (not is_subpath(self._postgresql._data_dir, d) or not self._postgresql.data_directory_empty()): + validate_directory(d, msg) + + def check_directories(self): + if "unix_socket_directories" in self._server_parameters: + for d in self._server_parameters["unix_socket_directories"].split(","): + self.try_to_create_dir(d.strip(), "'{}' is defined in unix_socket_directories, {}") + if "stats_temp_directory" in self._server_parameters: + self.try_to_create_dir(self._server_parameters["stats_temp_directory"], + "'{}' is defined in stats_temp_directory, {}") + @property def _configuration_to_save(self): configuration = [os.path.basename(self._postgresql_conf)] diff --git a/patroni/utils.py b/patroni/utils.py index 837243f7..dde96fd4 100644 --- a/patroni/utils.py +++ b/patroni/utils.py @@ -1,7 +1,9 @@ import logging +import os import platform import random import re +import tempfile import time from dateutil import tz @@ -416,3 +418,27 @@ def cluster_as_json(cluster): if cluster.failover.candidate: ret['scheduled_switchover']['to'] = cluster.failover.candidate return ret + + +def is_subpath(d1, d2): + real_d1 = os.path.realpath(d1) + os.path.sep + real_d2 = os.path.realpath(os.path.join(real_d1, d2)) + return os.path.commonprefix([real_d1, real_d2 + os.path.sep]) == real_d1 + + +def validate_directory(d, msg="{} {}"): + if not os.path.exists(d): + try: + os.makedirs(d) + except OSError as e: + logger.error(e) + raise PatroniException(msg.format(d, "couldn't create the directory")) + elif os.path.isdir(d): + try: + fd, tmpfile = tempfile.mkstemp(dir=d) + os.close(fd) + os.remove(tmpfile) + except OSError: + raise PatroniException(msg.format(d, "the directory is not writable")) + else: + raise PatroniException(msg.format(d, "is not a directory")) diff --git a/tests/__init__.py b/tests/__init__.py index 2781d0a4..2ab7bc33 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -164,7 +164,8 @@ class PostgresInit(unittest.TestCase): 'search_path': 'public', 'hot_standby': 'on', 'max_wal_senders': 5, 'wal_keep_segments': 8, 'wal_log_hints': 'on', 'max_locks_per_transaction': 64, 'max_worker_processes': 8, 'max_connections': 100, 'max_prepared_transactions': 0, - 'track_commit_timestamp': 'off', 'unix_socket_directories': '/tmp', 'trigger_file': 'bla'} + 'track_commit_timestamp': 'off', 'unix_socket_directories': '/tmp', 'trigger_file': 'bla', + 'stats_temp_directory': '/tmp'} @patch('psycopg2.connect', psycopg2_connect) @patch.object(ConfigHandler, 'write_postgresql_conf', Mock()) diff --git a/tests/test_utils.py b/tests/test_utils.py index 4bc194dc..ba3405cc 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -2,7 +2,7 @@ import unittest from mock import Mock, patch from patroni.exceptions import PatroniException -from patroni.utils import Retry, RetryFailedError, polling_loop +from patroni.utils import Retry, RetryFailedError, polling_loop, validate_directory class TestUtils(unittest.TestCase): @@ -10,6 +10,29 @@ class TestUtils(unittest.TestCase): def test_polling_loop(self): self.assertEqual(list(polling_loop(0.001, interval=0.001)), [0]) + @patch('os.path.exists', Mock(return_value=True)) + @patch('os.path.isdir', Mock(return_value=True)) + @patch('tempfile.mkstemp', Mock(return_value=("", ""))) + @patch('os.remove', Mock(side_effect=Exception)) + def test_validate_directory_writable(self): + self.assertRaises(Exception, validate_directory, "/tmp") + + @patch('os.path.exists', Mock(return_value=True)) + @patch('os.path.isdir', Mock(return_value=True)) + @patch('tempfile.mkstemp', Mock(side_effect=OSError)) + def test_validate_directory_not_writable(self): + self.assertRaises(PatroniException, validate_directory, "/tmp") + + @patch('os.path.exists', Mock(return_value=False)) + @patch('os.makedirs', Mock(side_effect=OSError)) + def test_validate_directory_couldnt_create(self): + self.assertRaises(PatroniException, validate_directory, "/tmp") + + @patch('os.path.exists', Mock(return_value=True)) + @patch('os.path.isdir', Mock(return_value=False)) + def test_validate_directory_is_not_a_directory(self): + self.assertRaises(PatroniException, validate_directory, "/tmp") + @patch('time.sleep', Mock()) class TestRetrySleeper(unittest.TestCase):