diff --git a/patroni/ha.py b/patroni/ha.py index 3bb75b78..0019253d 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -96,7 +96,8 @@ class Ha: # try to see if we are the former master that crashed. If so - we likely need to run pg_rewind # in order to join the former standby being promoted. pg_controldata = self.state_handler.controldata() - if not has_lock and pg_controldata.get('Database cluster state', '') == 'in production': # crashed master + if not has_lock and pg_controldata and\ + pg_controldata.get('Database cluster state', '') == 'in production': # crashed master self.state_handler.require_rewind() # XXX: follow the leader calls stop, which might take quite some time. diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 2051d819..3ab8b345 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -164,9 +164,9 @@ class Postgresql: def delete_trigger_file(self): os.path.exists(self.trigger_file) and os.unlink(self.trigger_file) - def write_pgpass(self, record, append=False): + def write_pgpass(self, record): pgpass = 'pgpass' - with open(pgpass, 'w' if not append else 'a') as f: + with open(pgpass, 'w') as f: os.fchmod(f.fileno(), 0o600) f.write('{host}:{port}:*:{user}:{password}\n'.format(**record)) env = os.environ.copy() @@ -363,7 +363,7 @@ recovery_target_timeline = 'latest' r = parseurl(leader.conn_url) r.update(self.pg_rewind) r['user'] = r['username'] - env = self.write_pgpass(r, append=True) + env = self.write_pgpass(r) pc = "user={user} host={host} port={port} dbname=postgres sslmode=prefer sslcompression=1".format(**r) logger.info("running pg_rewind from {}".format(pc)) pg_rewind = ['pg_rewind', '-D', self.data_dir, '--source-server', pc] @@ -377,7 +377,7 @@ recovery_target_timeline = 'latest' def controldata(self): """ return the contents of pg_controldata, or non-True value if pg_controldata call failed """ - result = None + result = {} try: data = subprocess.check_output(['pg_controldata', self.data_dir]) if data: @@ -425,10 +425,10 @@ recovery_target_timeline = 'latest' for f in os.listdir(status_dir): path = os.path.join(status_dir, f) try: - if os.path.isfile(path): - os.remove(path) - elif os.path.islink(path): # should not happen, but just in case + if os.path.islink(path): os.unlink(path) + elif os.path.isfile(path): + os.remove(path) except: logger.exception("Unable to remove {}".format(path)) @@ -449,10 +449,10 @@ recovery_target_timeline = 'latest' # and not shutdown in recovery. We have to remove the recovery.conf if present # and start/shutdown in a single user mode to emulate this. # XXX: if recovery.conf is linked, it will be written anew as a normal file. - if os.path.isfile(self.recovery_conf): - os.remove(self.recovery_conf) - else: + if os.path.islink(self.recovery_conf): os.unlink(self.recovery_conf) + else: + os.remove(self.recovery_conf) # Archived segments might be useful to pg_rewind, # clean the flags that tell we should remove them. self.cleanup_archive_status() @@ -494,7 +494,7 @@ recovery_target_timeline = 'latest' return True ret = subprocess.call(self._pg_ctl + ['promote']) == 0 if ret: - self._role = 'master' + self.set_role('master') logger.info("cleared rewind flag after becoming the leader") self._need_rewind = False self.call_nowait(ACTION_ON_ROLE_CHANGE) diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 060302c7..53f41b98 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -337,7 +337,7 @@ class TestPostgresql(unittest.TestCase): subprocess.check_output = check_output_call_error data = self.p.controldata() - self.assertIsNone(data) + self.assertEquals(data, dict()) subprocess.check_output = check_output_generic_exception self.assertRaises(Exception, self.p.controldata()) @@ -394,6 +394,7 @@ class TestPostgresql(unittest.TestCase): mock_unlink.assert_not_called() mock_remove.reset_mock() + mock_file.return_value = False mock_link.return_value = True self.p.cleanup_archive_status() @@ -402,7 +403,9 @@ class TestPostgresql(unittest.TestCase): mock_unlink.reset_mock() mock_remove.reset_mock() + mock_file.side_effect = Exception("foo") + mock_link.side_effect = Exception("foo") self.p.cleanup_archive_status() mock_unlink.assert_not_called() mock_remove.assert_not_called()