From b4755f9f8178df720eda19474cedec90e9fbc741 Mon Sep 17 00:00:00 2001 From: Maurice Makaay Date: Mon, 2 Dec 2019 14:13:05 +0100 Subject: [PATCH] Node config now checks for existence of the pgbouncer_config path. --- pgbouncemgr/node_config.py | 18 ++++++++++++++--- tests/test_node_config.py | 17 ++++++++++++++-- tests/test_state.py | 38 ++++++++++++++++++++--------------- tests/testfiles/pgbouncer.ini | 1 + 4 files changed, 53 insertions(+), 21 deletions(-) create mode 100644 tests/testfiles/pgbouncer.ini diff --git a/pgbouncemgr/node_config.py b/pgbouncemgr/node_config.py index 0050d3c..3b10ecd 100644 --- a/pgbouncemgr/node_config.py +++ b/pgbouncemgr/node_config.py @@ -1,12 +1,24 @@ +import os +from pgbouncemgr.config import InvalidConfigValue + class NodeConfig(): def __init__(self, node_id): self.node_id = node_id - self.pgbouncer_config = None + self._pgbouncer_config = None self.host = None self.port = None - def set_pgbouncer_config(self, path): - self.pgbouncer_config = path + @property + def pgbouncer_config(self): + return self._pgbouncer_config + + @pgbouncer_config.setter + def pgbouncer_config(self, path): + if not os.path.exists(path): + raise InvalidConfigValue( + "pgbouncer_config path must exist", + "nodes[%s]" % self.node_id, "pgbouncer_config", path) + self._pgbouncer_config = path def export(self): return { diff --git a/tests/test_node_config.py b/tests/test_node_config.py index b60212d..78554bc 100644 --- a/tests/test_node_config.py +++ b/tests/test_node_config.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- +import os import unittest from pgbouncemgr.node_config import * @@ -13,7 +14,19 @@ class NodeConfigTests(unittest.TestCase): self.assertEqual(None, config.host) self.assertEqual(None, config.port) - def test_SetPgbouncerConfig_UpdatesConfig(self): + def test_WithNonExistentFile_SetPgbouncerConfig_RaisesException(self): config = NodeConfig("test") - config.set_pgbouncer_config("/path/to/config") + with self.assertRaises(InvalidConfigValue) as context: + config.pgbouncer_config = "/path/to/non-existent/config.ini" + self.assertIn("key=nodes[test].pgbouncer_config", str(context.exception)) + self.assertIn("/non-existent/config.ini", str(context.exception)) + + def test_WithExistingFile_SetPgbouncerConfig_UpdatesConfig(self): + config = NodeConfig("test") + ini = os.path.join( + os.path.dirname(os.path.realpath(__file__)), + "testfiles", "pgbouncer.ini") + config.pgbouncer_config = ini + + self.assertEqual(ini, config.pgbouncer_config) diff --git a/tests/test_state.py b/tests/test_state.py index daa8f11..fea6413 100644 --- a/tests/test_state.py +++ b/tests/test_state.py @@ -1,9 +1,15 @@ # -*- coding: utf-8 -*- +import os import unittest from pgbouncemgr.state import * +PGBOUNCER_CONFIG = os.path.join( + os.path.dirname(os.path.realpath(__file__)), + "testfiles", "pgbouncer.ini") + + class StateTests(unittest.TestCase): def test_GivenFreshState_DefaultsAreSetCorrectly(self): state = State() @@ -170,7 +176,7 @@ class NodeCollectionTests(unittest.TestCase): state = State() node = state.add_node(1) node.timeline_id = 1 - node.config.pgbouncer_config = "/some/path/to/config" + node.config.pgbouncer_config = PGBOUNCER_CONFIG with self.assertRaises(NodeCannotBePromoted) as context: node.promote() self.assertIn("Node '1'", str(context.exception)) @@ -180,7 +186,7 @@ class NodeCollectionTests(unittest.TestCase): state = State() node = state.add_node(1) node.system_id = "system" - node.config.pgbouncer_config = "/some/path/to/config" + node.config.pgbouncer_config = PGBOUNCER_CONFIG with self.assertRaises(NodeCannotBePromoted) as context: node.promote() self.assertIn("Node '1'", str(context.exception)) @@ -199,7 +205,7 @@ class NodeCollectionTests(unittest.TestCase): def test_SetLeaderNode_SetsLeaderNode_WithUnknownLeaderStatus(self): state = State() node = state.add_node(1) - node.config.pgbouncer_config = "/some/path/to/config" + node.config.pgbouncer_config = PGBOUNCER_CONFIG node.system_id = "SuperCluster" node.timeline_id = " 005 " node.promote() @@ -214,7 +220,7 @@ class NodeCollectionTests(unittest.TestCase): def test_SetLeaderNode_ToSameLeader_ResetsLeaderNode_WithUnknownLeaderStatus(self): state = State() node = state.add_node(1) - node.config.pgbouncer_config = "/some/path/to/config" + node.config.pgbouncer_config = PGBOUNCER_CONFIG node.system_id = "1.2.3.4.5.6.7.8.9.10.11" node.timeline_id = 12 node.promote() @@ -230,11 +236,11 @@ class NodeCollectionTests(unittest.TestCase): def test_SetLeaderNode_ToNodeWithDifferentSystemId_RaisesException(self): state = State() node1 = state.add_node(1) - node1.config.pgbouncer_config = "/some/path/to/config1" + node1.config.pgbouncer_config = PGBOUNCER_CONFIG node1.system_id = "systemA" node1.timeline_id = 10 node2 = state.add_node(2) - node2.config.pgbouncer_config = "/some/path/to/config2" + node2.config.pgbouncer_config = PGBOUNCER_CONFIG node2.system_id = "systemB" node2.timeline_id = 10 node1.promote() @@ -245,11 +251,11 @@ class NodeCollectionTests(unittest.TestCase): def test_SetLeaderNode_ToNodeWithLowerTimelineId_RaisesException(self): state = State() node1 = state.add_node(1) - node1.config.pgbouncer_config = "/some/path/to/config1" + node1.config.pgbouncer_config = PGBOUNCER_CONFIG node1.system_id = "systemX" node1.timeline_id = 10 node2 = state.add_node(2) - node2.config.pgbouncer_config = "/some/path/to/config2" + node2.config.pgbouncer_config = PGBOUNCER_CONFIG node2.system_id = "systemX" node2.timeline_id = 9 node1.promote() @@ -260,15 +266,15 @@ class NodeCollectionTests(unittest.TestCase): def test_SetLeaderNode_ToNodeWithSameOrHigherTimelineId_IsOk(self): state = State() node1 = state.add_node(1) - node1.config.pgbouncer_config = "/some/path/to/config1" + node1.config.pgbouncer_config = PGBOUNCER_CONFIG node1.system_id = "systemX" node1.timeline_id = 42 node2 = state.add_node(2) - node2.config.pgbouncer_config = "/some/path/to/config2" + node2.config.pgbouncer_config = PGBOUNCER_CONFIG node2.system_id = "systemX" node2.timeline_id = 42 node3 = state.add_node(3) - node3.config.pgbouncer_config = "/some/path/to/config3" + node3.config.pgbouncer_config = PGBOUNCER_CONFIG node3.system_id = "systemX" node3.timeline_id = 43 state.modified = False @@ -405,14 +411,14 @@ class StateExportTests(unittest.TestCase): state = State() node1 = state.add_node(1) - node1.config.pgbouncer_config = "/some/path/to/config1" + node1.config.pgbouncer_config = PGBOUNCER_CONFIG node1.system_id = "System X" node1.timeline_id = 555 node1.set_status(NODE_PRIMARY) node1.set_error("Some error for 1") node2 = state.add_node(2) - node2.config.pgbouncer_config = "/some/path/to/config2" + node2.config.pgbouncer_config = PGBOUNCER_CONFIG node2.system_id = "System Y" node2.timeline_id = 1 node2.set_status(NODE_STANDBY) @@ -428,7 +434,7 @@ class StateExportTests(unittest.TestCase): self.assertEqual({ "system_id": "System X", "timeline_id": 555, - "pgbouncer_config": "/some/path/to/config1", + "pgbouncer_config": PGBOUNCER_CONFIG, "leader_node_id": 1, "leader_status": LEADER_CONNECTED, "leader_error": "Some error for leader connection", @@ -436,7 +442,7 @@ class StateExportTests(unittest.TestCase): 1: { "node_id": 1, "config": { - "pgbouncer_config": "/some/path/to/config1", + "pgbouncer_config": PGBOUNCER_CONFIG, "host": None, "port": None }, @@ -449,7 +455,7 @@ class StateExportTests(unittest.TestCase): 2: { "node_id": 2, "config": { - "pgbouncer_config": "/some/path/to/config2", + "pgbouncer_config": PGBOUNCER_CONFIG, "host": None, "port": None }, diff --git a/tests/testfiles/pgbouncer.ini b/tests/testfiles/pgbouncer.ini new file mode 100644 index 0000000..448adb0 --- /dev/null +++ b/tests/testfiles/pgbouncer.ini @@ -0,0 +1 @@ +# Empty pgbouncer config file.