Some improvements to patronictl (#571)

* Use scope from config file when listing members

* Add version command to patronictl

* Only delete leader on shutdown when we have the lock to avoid exceptions when leader key does not exist

* Add a timestamp option to list command.

* YAML format for patronictl output

* Fix API request to get version

patroni/ctl.py

@@ -31,6 +31,7 @@ from patroni.dcs import get_dcs as _get_dcs
 from patroni.exceptions import PatroniException
 from patroni.postgresql import Postgresql
 from patroni.utils import patch_config
+from patroni.version import __version__
 from prettytable import PrettyTable
 from six.moves.urllib_parse import urlparse
 from six import text_type
@@ -96,7 +97,7 @@ def store_config(config, path):
         yaml.dump(config, fd)
 
 
-option_format = click.option('--format', '-f', 'fmt', help='Output format (pretty, json)', default='pretty')
+option_format = click.option('--format', '-f', 'fmt', help='Output format (pretty, json, yaml)', default='pretty')
 option_watchrefresh = click.option('-w', '--watch', type=float, help='Auto update the screen every X seconds')
 option_watch = click.option('-W', is_flag=True, help='Auto update the screen every 2 seconds')
 option_force = click.option('--force', is_flag=True, help='Do not ask for confirmation at any point')
@@ -149,9 +150,12 @@ def print_output(columns, rows=None, alignment=None, fmt='pretty', header=True,
         click.echo(t)
         return
 
-    if fmt == 'json':
+    if fmt in ['json', 'yaml']:
         elements = [dict(zip(columns, r)) for r in rows]
-        click.echo(json.dumps(elements))
+        if fmt == 'json':
+            click.echo(json.dumps(elements))
+        elif fmt == 'yaml':
+            click.echo(yaml.safe_dump(elements, encoding=None, allow_unicode=True, width=200))
 
     if fmt == 'tsv':
         if columns is not None and header:
@@ -698,19 +702,26 @@ def output_members(cluster, name, extended=False, fmt='pretty'):
 @ctl.command('list', help='List the Patroni members for a given Patroni')
 @click.argument('cluster_names', nargs=-1)
 @click.option('--extended', '-e', help='Show some extra information', is_flag=True)
+@click.option('--timestamp', '-t', help='Print timestamp', is_flag=True)
 @option_format
 @option_watch
 @option_watchrefresh
 @click.pass_obj
-def members(obj, cluster_names, fmt, watch, w, extended):
+def members(obj, cluster_names, fmt, watch, w, extended, timestamp):
     if not cluster_names:
-        logging.warning('Listing members: No cluster names were provided')
-        return
+        if 'scope' not in obj:
+            logging.warning('Listing members: No cluster names were provided')
+            return
+        else:
+            cluster_names = [obj['scope']]
 
     for cluster_name in cluster_names:
         dcs = get_dcs(obj, cluster_name)
 
         for _ in watching(w, watch):
+            if timestamp:
+                click.echo(datetime.datetime.now().strftime("%Y-%m-%d %H:%M:%S"))
+
             cluster = dcs.get_cluster()
             output_members(cluster, cluster_name, extended, fmt)
 
@@ -1043,3 +1054,36 @@ def show_config(obj, cluster_name):
     cluster = get_dcs(obj, cluster_name).get_cluster()
 
     click.echo(format_config_for_editing(cluster.config.data))
+
+
+@ctl.command('version', help='Output version of patronictl command or a running Patroni instance')
+@click.argument('cluster_name', required=False)
+@click.argument('member_names', nargs=-1)
+@click.pass_obj
+def version(obj, cluster_name, member_names):
+    click.echo("patronictl version {0}".format(__version__))
+
+    if not cluster_name:
+        return
+
+    click.echo("")
+    cluster = get_dcs(obj, cluster_name).get_cluster()
+    for m in cluster.members:
+        if m.api_url:
+            if not member_names or m.name in member_names:
+                try:
+                    response = request_patroni(m, 'get', 'patroni')
+                    data = response.json()
+                    version = data.get('patroni', {}).get('version')
+                    pg_version = data.get('server_version')
+                    pg_version_str = " PostgreSQL {0}".format(format_pg_version(pg_version)) if pg_version else ""
+                    click.echo("{0}: Patroni {1}{2}".format(m.name, version, pg_version_str))
+                except Exception as e:
+                    click.echo("{0}: failed to get version: {1}".format(m.name, e))
+
+
+def format_pg_version(version):
+    if version < 100000:
+        return "{0}.{1}.{2}".format(version // 10000, version // 100 % 100, version % 100)
+    else:
+        return "{0}.{1}".format(version // 10000, version % 100)

patroni/ha.py

@@ -1109,7 +1109,8 @@ class Ha(object):
             disable_wd = self.watchdog.disable if self.watchdog.is_running else None
             self.while_not_sync_standby(lambda: self.state_handler.stop(checkpoint=False, on_safepoint=disable_wd))
             if not self.state_handler.is_running():
-                self.dcs.delete_leader()
+                if self.has_lock():
+                    self.dcs.delete_leader()
             else:
                 # XXX: what about when Patroni is started as the wrong user that has access to the watchdog device
                 # but cannot shut down PostgreSQL. Root would be the obvious example. Would be nice to not kill the

tests/test_ctl.py

@@ -9,7 +9,7 @@ from datetime import datetime, timedelta
 from mock import patch, Mock
 from patroni.ctl import ctl, members, store_config, load_config, output_members, request_patroni, get_dcs, parse_dcs, \
     get_all_members, get_any_member, get_cursor, query_member, configure, PatroniCtlException, apply_config_changes, \
-    format_config_for_editing, show_diff, invoke_editor
+    format_config_for_editing, show_diff, invoke_editor, format_pg_version
 from patroni.dcs.etcd import Client, Failover
 from patroni.utils import tzutc
 from psycopg2 import OperationalError
@@ -70,6 +70,7 @@ class TestCtl(unittest.TestCase):
         cluster = get_cluster_initialized_with_leader(Failover(1, 'foo', 'bar', scheduled_at))
         self.assertIsNone(output_members(cluster, name='abc', fmt='pretty'))
         self.assertIsNone(output_members(cluster, name='abc', fmt='json'))
+        self.assertIsNone(output_members(cluster, name='abc', fmt='yaml'))
         self.assertIsNone(output_members(cluster, name='abc', fmt='tsv'))
 
     @patch('patroni.ctl.get_dcs')
@@ -383,7 +384,7 @@ class TestCtl(unittest.TestCase):
         cluster = get_cluster_initialized_with_leader(sync=('leader', 'other'))
         mock_get_dcs.return_value.get_cluster = Mock(return_value=cluster)
 
-        result = self.runner.invoke(ctl, ['list', 'dummy', '--extended'])
+        result = self.runner.invoke(ctl, ['list', 'dummy', '--extended', '--timestamp'])
         assert '2100' in result.output
         assert 'Scheduled restart' in result.output
 
@@ -509,3 +510,21 @@ class TestCtl(unittest.TestCase):
         self.runner.invoke(ctl, ['edit-config', 'dummy', '--force', '--apply', '-'], input='foo: bar')
         mock_get_dcs.return_value.set_config_value = Mock(return_value=True)
         self.runner.invoke(ctl, ['edit-config', 'dummy', '--force', '--apply', '-'], input='foo: bar')
+
+    @patch('patroni.ctl.get_dcs')
+    def test_version(self, mock_get_dcs):
+        mock_get_dcs.return_value = self.e
+        mock_get_dcs.return_value.get_cluster = get_cluster_initialized_with_leader
+        with patch('patroni.ctl.request_patroni') as mocked:
+            result = self.runner.invoke(ctl, ['version'])
+            assert 'patronictl version' in result.output
+            mocked.return_value.json = lambda: {'patroni': {'version': '1.2.3'}, 'server_version': 100001}
+            result = self.runner.invoke(ctl, ['version', 'dummy'])
+            assert '1.2.3' in result.output
+        with patch('requests.get', Mock(side_effect=Exception)):
+            result = self.runner.invoke(ctl, ['version', 'dummy'])
+            assert 'failed to get version' in result.output
+
+    def test_format_pg_version(self):
+        self.assertEquals(format_pg_version(100001), '10.1')
+        self.assertEquals(format_pg_version(90605), '9.6.5')