From 318ca6be38b4fdadcc290b7cb52d0dcbaf48030f Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Mon, 20 Jun 2016 15:16:22 +0200
Subject: [PATCH 01/21] Implement scheduling and deleting a restart.

The scheduled restart API extends the already existing restart
endpoint by processing the parameters in the request body.

Only one scheduled restart at a time is support. DELETE method
on the /restart endpoint is used to remove an existing restart.
---
 patroni/__init__.py |   1 +
 patroni/api.py      | 111 +++++++++++++++++++++++++++++++++-----------
 patroni/ha.py       |  19 +++++++-
 tests/test_api.py   |  41 +++++++++++++++-
 tests/test_ha.py    |  12 ++++-
 5 files changed, 153 insertions(+), 31 deletions(-)

diff --git a/patroni/__init__.py b/patroni/__init__.py
index c48eab40..763bd24d 100644
--- a/patroni/__init__.py
+++ b/patroni/__init__.py
@@ -29,6 +29,7 @@ class Patroni(object):
         self.tags = self.get_tags()
         self.nap_time = self.config['loop_wait']
         self.next_run = time.time()
+        self.scheduled_restart = {}
 
         self._reload_config_scheduled = False
         self._received_sighup = False
diff --git a/patroni/api.py b/patroni/api.py
index 7000fc2f..36b950a1 100644
--- a/patroni/api.py
+++ b/patroni/api.py
@@ -7,6 +7,7 @@ import time
 import dateutil
 import datetime
 import pytz
+import re
 
 from patroni.exceptions import PostgresConnectionException
 from patroni.utils import deep_compare, patch_config, Retry, RetryFailedError
@@ -63,6 +64,8 @@ class RestApiHandler(BaseHTTPRequestHandler):
         if patroni.postgresql.pending_restart:
             response['pending_restart'] = True
         response['patroni'] = {'version': patroni.version, 'scope': patroni.postgresql.scope}
+        if patroni.scheduled_restart and isinstance(patroni.scheduled_restart, dict):
+            response['scheduled_restart'] = patroni.scheduled_restart
         self._write_json_response(status_code, response)
 
     def do_GET(self, write_status_code_only=False):
@@ -86,7 +89,7 @@ class RestApiHandler(BaseHTTPRequestHandler):
                 status_code = 503
         elif 'role' in response and response['role'] in path:
             status_code = 503 if response['role'] != 'master' and patroni.noloadbalance else 200
-        elif patroni.ha.restart_scheduled() and patroni.postgresql.role == 'master' and 'master' in path:
+        elif patroni.ha.immediate_restart_scheduled() and patroni.postgresql.role == 'master' and 'master' in path:
             # exceptional case for master node when the postgres is being restarted via API
             status_code = 200
         else:
@@ -112,9 +115,9 @@ class RestApiHandler(BaseHTTPRequestHandler):
         else:
             self.send_error(502)
 
-    def _read_json_content(self):
+    def _read_json_content(self, body_is_optional=False):
         if 'content-length' not in self.headers:
-            return self.send_error(411)
+            return self.send_error(411) if body_is_optional else None
         try:
             content_length = int(self.headers.get('content-length'))
             request = json.loads(self.rfile.read(content_length).decode('utf-8'))
@@ -162,17 +165,80 @@ class RestApiHandler(BaseHTTPRequestHandler):
             response = str(e)
         self._write_response(status_code, response)
 
+    @staticmethod
+    def parse_schedule(schedule, action):
+        """ parses the given schedule and validates at """
+        error = None
+        scheduled_at = None
+        try:
+            scheduled_at = dateutil.parser.parse(schedule)
+            if scheduled_at.tzinfo is None:
+                error = 'Timezone information is mandatory for the scheduled {0}'.format(action)
+                status_code = 400
+            elif scheduled_at < datetime.datetime.now(pytz.utc):
+                error = 'Cannot schedule {0} in the past'.format(action)
+                status_code = 422
+            else:
+                status_code = None
+        except (ValueError, TypeError):
+            logger.exception('Invalid scheduled %s time: %s', action, schedule)
+            error = 'Unable to parse scheduled timestamp. It should be in an unambiguous format, e.g. ISO 8601'
+            status_code = 422
+        return (status_code, error, scheduled_at)
+
     @check_auth
     def do_POST_restart(self):
         status_code = 500
         data = 'restart failed'
-        try:
-            status, data = self.server.patroni.ha.restart()
-            status_code = 200 if status else 503
-        except Exception:
-            logger.exception('Exception during restart')
+        request = self._read_json_content()
+        if not request:  # unconditional restart
+            try:
+                status, data = self.server.patroni.ha.restart()
+                status_code = 200 if status else 503
+            except Exception:
+                logger.exception('Exception during restart')
+        else:
+            logger.info("received scheduled restart request: {0}".format(request))
+            for k in request:
+                if k == 'schedule':
+                    (_, data, request[k]) = self.parse_schedule(request[k], "restart")
+                    if _:
+                        status_code = _
+                        break
+                elif k == 'role':
+                    if request[k] not in ('master', 'replica'):
+                        status_code = 400
+                        data = "PostgreSQL role should be either master or replica"
+                        break
+                elif k == 'postgres_version':
+                    if not re.match(r'([1-9][0-9]?\.){2}[1-9][0-9]?$', request[k]):
+                        status_code = 400
+                        data = "PostgreSQL version should be in the first.major.minor format"
+                        break
+                else:
+                    status_code = 400
+                    data = "Unknown filter for the scheduled restart: {0}".format(k)
+                    break
+            else:
+                if 'schedule' not in request:
+                    data = "Schedule required for the scheduled restart"
+                    status_code = 400
+                else:
+                    if self.server.patroni.ha.schedule_future_restart(request):
+                        data = "Restart scheduled"
+                        status_code = 202
+                    else:
+                        data = "Another restart is already scheduled"
+                        status_code = 409
         self._write_response(status_code, data)
 
+    @check_auth
+    def do_DELETE_restart(self):
+        self.server.patroni.ha.delete_future_restart()
+        data = "scheduled restart deleted"
+        code = 200
+        self._write_response(code, data)
+
     @check_auth
     def do_POST_reinitialize(self):
         ha = self.server.patroni.ha
@@ -244,25 +310,16 @@ class RestApiHandler(BaseHTTPRequestHandler):
         data = ''
         if leader or candidate:
             if scheduled_at:
-                try:
-                    scheduled_at = dateutil.parser.parse(scheduled_at)
-                    if scheduled_at.tzinfo is None:
-                        data = 'Timezone information is mandatory for scheduled_at'
-                        status_code = 400
-                    elif scheduled_at < datetime.datetime.now(pytz.utc):
-                        data = 'Cannot schedule failover in the past'
-                        status_code = 422
-                    elif self.server.patroni.dcs.manual_failover(leader, candidate, scheduled_at=scheduled_at):
-                        self.server.patroni.dcs.event.set()
-                        data = 'Failover scheduled'
-                        status_code = 202
-                    else:
-                        data = 'failed to write failover key into DCS'
-                        status_code = 503
-                except (ValueError, TypeError):
-                    logger.exception('Invalid scheduled failover time: %s', request['scheduled_at'])
-                    data = 'Unable to parse scheduled timestamp. It should be in an unambiguous format, e.g. ISO 8601'
-                    status_code = 422
+                (_, data, scheduled_at) = self.parse_schedule(scheduled_at, "failover")
+                if _:
+                    status_code = _
+                elif self.server.patroni.dcs.manual_failover(leader, candidate, scheduled_at=scheduled_at):
+                    self.server.patroni.dcs.event.set()
+                    data = 'Failover scheduled'
+                    status_code = 202
+                else:
+                    data = 'failed to write failover key into DCS'
+                    status_code = 503
             else:
                 data = self.is_failover_possible(cluster, leader, candidate)
                 if not data:
diff --git a/patroni/ha.py b/patroni/ha.py
index 0352b270..cef579de 100644
--- a/patroni/ha.py
+++ b/patroni/ha.py
@@ -66,6 +66,11 @@ class Ha(object):
                 data['xlog_location'] = self.state_handler.xlog_position()
             except:
                 pass
+        if self.patroni.scheduled_restart:
+            scheduled_restart_data = self.patroni.scheduled_restart.copy()
+            scheduled_restart_data['schedule'] = scheduled_restart_data['schedule'].isoformat()
+            data['scheduled_restart'] = scheduled_restart_data
+
         self.dcs.touch_member(json.dumps(data, separators=(',', ':')))
 
     def clone(self, clone_member=None, msg='(without leader)'):
@@ -372,7 +377,19 @@ class Ha(object):
         with self._async_executor:
             return self._async_executor.schedule(action)
 
-    def restart_scheduled(self):
+    def schedule_future_restart(self, restart_data):
+        if isinstance(restart_data, dict):
+            with self._async_executor:
+                if not self.patroni.scheduled_restart:
+                    self.patroni.scheduled_restart = restart_data
+                    return True
+        return False
+
+    def delete_future_restart(self):
+        with self._async_executor:
+            self.patroni.scheduled_restart = {}
+
+    def immediate_restart_scheduled(self):
         return self._async_executor.scheduled_action == 'restart'
 
     def schedule_reinitialize(self):
diff --git a/tests/test_api.py b/tests/test_api.py
index 8483299b..d0974e22 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -1,3 +1,4 @@
+
 import json
 import psycopg2
 import unittest
@@ -39,13 +40,21 @@ class MockHa(object):
         return (True, '')
 
     @staticmethod
-    def restart_scheduled():
+    def immediate_restart_scheduled():
         return False
 
+    @staticmethod
+    def delete_future_restart():
+        return True
+
     @staticmethod
     def fetch_nodes_statuses(members):
         return [[None, True, None, None, {}]]
 
+    @staticmethod
+    def schedule_future_restart(data):
+        return True
+
 
 class MockPatroni(object):
 
@@ -57,6 +66,7 @@ class MockPatroni(object):
     tags = {}
     version = '0.00'
     noloadbalance = Mock(return_value=False)
+    scheduled_restart = {'schedule': '2016-08-29 12:45TZ+1'}
 
     @staticmethod
     def sighup_handler():
@@ -104,7 +114,7 @@ class TestRestApiHandler(unittest.TestCase):
         MockPatroni.dcs.cluster = None
         with patch.object(RestApiHandler, 'get_postgresql_status', Mock(return_value={'role': 'master'})):
             MockRestApiServer(RestApiHandler, 'GET /master')
-        with patch.object(MockHa, 'restart_scheduled', Mock(return_value=True)):
+        with patch.object(MockHa, 'immediate_restart_scheduled', Mock(return_value=True)):
             MockRestApiServer(RestApiHandler, 'GET /master')
         self.assertIsNotNone(MockRestApiServer(RestApiHandler, 'GET /master'))
 
@@ -167,6 +177,33 @@ class TestRestApiHandler(unittest.TestCase):
         with patch.object(MockHa, 'restart', Mock(side_effect=Exception)):
             MockRestApiServer(RestApiHandler, request)
 
+        post = request + '\nContent-Length: '
+        # wrong version
+        request = post + '84\n\n{"schedule": "2016-08-20 12:45TZ+1", "role": "unknown", "postgres_version": "9.5.3"}'
+        MockRestApiServer(RestApiHandler, request)
+        # wrong role
+        request = post + '85\n\n{"schedule": "2016-08-20 12:45TZ+1", "role": "master", "postgres_version": "9.5.3.1"}'
+        MockRestApiServer(RestApiHandler, request)
+        # unknown filter
+        request = post + '55\n\n{"schedule": "2016-08-29 12:45TZ+1", "batman": "lives"}'
+        MockRestApiServer(RestApiHandler, request)
+        # incorrect schedule
+        request = post + '55\n\n{"schedule": "2016-08-42 12:45TZ+1", "role": "master"}'
+        MockRestApiServer(RestApiHandler, request)
+        # everything fine, but the schedule is missing
+        request = post + '47\n\n{"role": "master", "postgres_version": "9.5.2"}'
+        MockRestApiServer(RestApiHandler, request)
+        for retval in (True, False):
+            with patch.object(MockHa, 'schedule_future_restart', Mock(return_value=retval)):
+                request = post + '36\n\n{"schedule": "2016-08-29 12:45TZ+1"}'
+                MockRestApiServer(RestApiHandler, request)
+
+    def test_do_DELETE_restart(self):
+        for retval in (True, False):
+            with patch.object(MockHa, 'delete_future_restart', Mock(return_value=retval)):
+                request = 'DELETE /restart HTTP/1.0' + self._authorization
+                self.assertIsNotNone(MockRestApiServer(RestApiHandler, request))
+
     @patch.object(MockHa, 'dcs')
     def test_do_POST_reinitialize(self, dcs):
         cluster = dcs.get_cluster.return_value
diff --git a/tests/test_ha.py b/tests/test_ha.py
index 4f691212..71e7087f 100644
--- a/tests/test_ha.py
+++ b/tests/test_ha.py
@@ -1,4 +1,5 @@
 import datetime
+import dateutil
 import etcd
 import os
 import pytz
@@ -80,6 +81,7 @@ zookeeper:
         self.replicatefrom = None
         self.api.connection_string = 'http://127.0.0.1:8008'
         self.clonefrom = None
+        self.scheduled_restart = {'schedule': dateutil.parser.parse('2016-08-29 12:45TZ+1')}
 
 
 def run_async(func, args=()):
@@ -282,7 +284,7 @@ class TestHa(unittest.TestCase):
 
     def test_restart_in_progress(self):
         self.ha._async_executor.schedule('restart', True)
-        self.assertTrue(self.ha.restart_scheduled())
+        self.assertTrue(self.ha.immediate_restart_scheduled())
         self.assertEquals(self.ha.run_cycle(), 'not healthy enough for leader race')
 
         self.ha.cluster = get_cluster_initialized_with_leader()
@@ -395,3 +397,11 @@ class TestHa(unittest.TestCase):
         self.assertEqual(self.ha.post_recover(), 'failed to start postgres')
         self.p.is_running = true
         self.assertIsNone(self.ha.post_recover())
+
+    def test_schedule_future_restart(self):
+        self.ha.patroni.scheduled_restart = {}
+        self.ha.schedule_future_restart({'schedule': '2016-08-30 12:45TZ+1"'})
+        self.ha.schedule_future_restart({'schedule': '2016-08-30 12:45TZ+1"'})
+
+    def test_delete_future_restarts(self):
+        self.ha.delete_future_restart()

From 80b5a370b0fc54a1fb52d7b980b8294f1e2a8c40 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Mon, 20 Jun 2016 15:21:33 +0200
Subject: [PATCH 02/21] API support restarts when a "pending restart" flag is
 set.

---
 patroni/api.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/patroni/api.py b/patroni/api.py
index 36b950a1..ba37d5a0 100644
--- a/patroni/api.py
+++ b/patroni/api.py
@@ -215,7 +215,7 @@ class RestApiHandler(BaseHTTPRequestHandler):
                         status_code = 400
                         data = "PostgreSQL version should be in the first.major.minor format"
                         break
-                else:
+                elif k != 'with_pending_restart_flag':
                     status_code = 400
                     data = "Unknown filter for the scheduled restart: {0}".format(k)
                     break

From 70195bec2d409137d5cfce8aed706384a88bc47c Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Tue, 21 Jun 2016 10:56:18 +0200
Subject: [PATCH 03/21] Handle empty body correctly when reading requests.

---
 patroni/api.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/patroni/api.py b/patroni/api.py
index 02d8b168..a6a14dbe 100644
--- a/patroni/api.py
+++ b/patroni/api.py
@@ -120,8 +120,10 @@ class RestApiHandler(BaseHTTPRequestHandler):
             return self.send_error(411) if body_is_optional else None
         try:
             content_length = int(self.headers.get('content-length'))
+            if content_length == 0 and body_is_optional:
+                return {}
             request = json.loads(self.rfile.read(content_length).decode('utf-8'))
-            if isinstance(request, dict) and request:
+            if isinstance(request, dict) and (request or body_is_optional):
                 return request
         except Exception:
             logger.exception('Bad request')
@@ -190,7 +192,7 @@ class RestApiHandler(BaseHTTPRequestHandler):
     def do_POST_restart(self):
         status_code = 500
         data = 'restart failed'
-        request = self._read_json_content()
+        request = self._read_json_content(body_is_optional=True)
         if not request:  # unconditional restart
             try:
                 status, data = self.server.patroni.ha.restart()

From 6a8bfdeb7626fc92016efe0d8bd60fb7f91e07f9 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Tue, 21 Jun 2016 11:02:10 +0200
Subject: [PATCH 04/21] Decouple the schedule check from the failover.

---
 patroni/ha.py | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/patroni/ha.py b/patroni/ha.py
index cef579de..65565342 100644
--- a/patroni/ha.py
+++ b/patroni/ha.py
@@ -291,31 +291,41 @@ class Ha(object):
         else:
             self.state_handler.follow(None, None)
 
-    def process_manual_failover_from_leader(self):
-        failover = self.cluster.failover
-
-        if failover.scheduled_at:
-            # If the failover is in the far future, we shouldn't do anything and just return.
+    def should_run_scheduled_action(self, action_name, scheduled_at, cleanup_fn):
+        if scheduled_at:
+            # If the faildover is in the far future, we shouldn't do anything and just return.
             # If the failover is in the past, we consider the value to be stale and we remove
             # the value.
             # If the value is close to now, we initiate the failover
             now = datetime.datetime.now(pytz.utc)
             try:
-                delta = (failover.scheduled_at - now).total_seconds()
+                delta = (scheduled_at - now).total_seconds()
 
                 if delta > self.patroni.nap_time:
-                    logging.info('Awaiting failover at %s (in %.0f seconds)', failover.scheduled_at.isoformat(), delta)
-                    return
+                    logging.info('Awaiting {0} at %s (in %.0f seconds)'.format(action_name), scheduled_at.isoformat(), delta)
+                    return False
                 elif delta < - int(self.patroni.nap_time * 1.5):
-                    logger.warning('Found a stale failover value, cleaning up: %s', failover.scheduled_at)
+                    logger.warning('Found a stale {0} value, cleaning up: %s'.format(action_name), scheduled_at.isoformat())
+                    cleanup_fn()
                     self.dcs.manual_failover('', '', index=self.cluster.failover.index)
-                    return
+                    return None
 
                 # The value is very close to now
                 sleep(max(delta, 0))
-                logger.info('Manual scheduled failover at {}'.format(failover.scheduled_at.isoformat()))
+                logger.info('Manual scheduled {0} at %s'.format(action_name), scheduled_at.isoformat())
+                return True
             except TypeError:
-                logger.warning('Incorrect value in of scheduled_at: %s', failover.scheduled_at)
+                logger.warning('Incorrect value in of scheduled_at: %s', scheduled_at)
+                cleanup_fn()
+        return None
+
+    def process_manual_failover_from_leader(self):
+        failover = self.cluster.failover
+
+        if (failover.scheduled_at and not
+            self.should_run_scheduled_action("failover", failover.scheduled_at, lambda:
+                                             self.dcs.manual_failover('', '', index=self.cluster.failover.index))):
+            return
 
         if not failover.leader or failover.leader == self.state_handler.name:
             if not failover.candidate or failover.candidate != self.state_handler.name:

From e5cf06101a4e140d38b79aa62ba31107d9d4b30d Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Tue, 21 Jun 2016 11:19:33 +0200
Subject: [PATCH 05/21] Fix line is too long warnings.

---
 patroni/ha.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/patroni/ha.py b/patroni/ha.py
index 65565342..0b6f3b15 100644
--- a/patroni/ha.py
+++ b/patroni/ha.py
@@ -302,10 +302,12 @@ class Ha(object):
                 delta = (scheduled_at - now).total_seconds()
 
                 if delta > self.patroni.nap_time:
-                    logging.info('Awaiting {0} at %s (in %.0f seconds)'.format(action_name), scheduled_at.isoformat(), delta)
+                    logging.info('Awaiting {0} at %s (in %.0f seconds)'.format(action_name),
+                                 scheduled_at.isoformat(), delta)
                     return False
                 elif delta < - int(self.patroni.nap_time * 1.5):
-                    logger.warning('Found a stale {0} value, cleaning up: %s'.format(action_name), scheduled_at.isoformat())
+                    logger.warning('Found a stale {0} value, cleaning up: %s'.format(action_name),
+                                   scheduled_at.isoformat())
                     cleanup_fn()
                     self.dcs.manual_failover('', '', index=self.cluster.failover.index)
                     return None

From 29845dd383a86ecf6806c4841ef080aacaccbb1d Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Thu, 23 Jun 2016 10:43:54 +0200
Subject: [PATCH 06/21] Restart the node according to the schedule.

The scheduled restart data structures are now independent of those
used by the normal restarts. This would be fixed in subsequent
commits.
Add the behave tests, that cover the POST /restart (but not DELETE).
---
 features/patroni_api.feature  | 13 +++++++++++
 features/steps/patroni_api.py | 23 ++++++++++++++++---
 patroni/api.py                |  5 +++--
 patroni/ha.py                 | 42 +++++++++++++++++++++++++++++++++--
 patroni/postgresql.py         | 26 ++++++++++++++++++++++
 tests/test_api.py             |  3 ++-
 6 files changed, 104 insertions(+), 8 deletions(-)

diff --git a/features/patroni_api.feature b/features/patroni_api.feature
index 6bbdbeb1..934e8b76 100644
--- a/features/patroni_api.feature
+++ b/features/patroni_api.feature
@@ -72,3 +72,16 @@ Scenario: check the scheduled failover
         And postgres0 role is the primary after 5 seconds
         And postgres1 role is the secondary after 10 seconds
 	And replication works from postgres0 to postgres1 after 25 seconds
+
+Scenario: check the scheduled restart
+	Given I issue a PATCH request to http://127.0.0.1:8008/config with {"postgresql": {"parameters": {"checkpoint_warning": "60s"}}}
+	Then I receive a response code 200
+		And Response on GET http://127.0.0.1:8008/patroni contains pending_restart after 5 seconds
+	Given I issue a scheduled restart at http://127.0.0.1:8008 in 1 seconds with {"role": "replica"}
+	Then I receive a response code 202
+		And I sleep for 10 seconds
+		And Response on GET http://127.0.0.1:8008/patroni contains pending_restart after 10 seconds
+	Given I issue a scheduled restart at http://127.0.0.1:8008 in 1 seconds with {"with_pending_restart_flag": "True"}
+	Then I receive a response code 202
+		And Response on GET http://127.0.0.1:8008/patroni does not contain pending_restart after 10 seconds
+
diff --git a/features/steps/patroni_api.py b/features/steps/patroni_api.py
index 90c38564..4d2f1709 100644
--- a/features/steps/patroni_api.py
+++ b/features/steps/patroni_api.py
@@ -103,18 +103,35 @@ def scheduled_failover(context, at_url, from_host, to_host, in_seconds):
     """.format(at_url, from_host, to_host, datetime.now(pytz.utc) + timedelta(seconds=int(in_seconds))))
 
 
+@step('I issue a scheduled restart at {url:url} in {in_seconds:d} seconds with {data}')
+def scheduled_restart(context, url, in_seconds, data):
+    data = data and json.loads(data) or {}
+    restart_options = ['"schedule": "{0}"'.format((datetime.now(pytz.utc) + timedelta(seconds=int(in_seconds))).isoformat())]
+    for key in data:
+        if key == 'schedule':
+            continue
+        restart_options.append('"{0}": "{1}"'.format(key, data[key]))
+    context.execute_steps(u"""Given I issue a POST request to {0}/restart with {{{1}}}""".
+                          format(url, ','.join(restart_options)))
+
+
 @step('I add tag {tag:w} {value:w} to {pg_name:w} config')
 def add_tag_to_config(context, tag, value, pg_name):
     context.pctl.add_tag_to_config(pg_name, tag, value)
 
 
 @then('Response on GET {url} contains {value} after {timeout:d} seconds')
-def check_http_response(context, url, value, timeout):
+def check_http_response(context, url, value, timeout, negate=False):
     for _ in range(int(timeout)):
         r = requests.get(url)
-        if value in r.content.decode('utf-8'):
+        if (value in r.content.decode('utf-8')) != negate:
             break
         time.sleep(1)
     else:
         assert False,\
-            "Value {0} is not present in response after {1} seconds".format(value, timeout)
+            "Value {0} is {0} present in response after {1} seconds".format("not" if not negate else "", value, timeout)
+
+
+@then('Response on GET {url} does not contain {value} after {timeout:d} seconds')
+def check_not_in_http_response(context, url, value, timeout):
+    check_http_response(context, url, value, timeout, negate=True)
diff --git a/patroni/api.py b/patroni/api.py
index a6a14dbe..b91eea30 100644
--- a/patroni/api.py
+++ b/patroni/api.py
@@ -65,7 +65,8 @@ class RestApiHandler(BaseHTTPRequestHandler):
             response['pending_restart'] = True
         response['patroni'] = {'version': patroni.version, 'scope': patroni.postgresql.scope}
         if patroni.scheduled_restart and isinstance(patroni.scheduled_restart, dict):
-            response['scheduled_restart'] = patroni.scheduled_restart
+            response['scheduled_restart'] = patroni.scheduled_restart.copy()
+            response['scheduled_restart']['schedule'] = (response['scheduled_restart']['schedule']).isoformat()
         self._write_json_response(status_code, response)
 
     def do_GET(self, write_status_code_only=False):
@@ -236,7 +237,7 @@ class RestApiHandler(BaseHTTPRequestHandler):
 
     @check_auth
     def do_DELETE_restart(self):
-        self.server.patroni.ha.delete_future_restart()
+        self.server.patroni.ha.delete_future_restart(take_lock=True)
         data = "scheduled restart deleted"
         code = 200
         self._write_response(code, data)
diff --git a/patroni/ha.py b/patroni/ha.py
index 0b6f3b15..8c282c95 100644
--- a/patroni/ha.py
+++ b/patroni/ha.py
@@ -385,6 +385,36 @@ class Ha(object):
         return self.follow('demoting self because i do not have the lock and i was a leader',
                            'no action.  i am a secondary and i am following a leader', False)
 
+    def evaluate_scheduled_restart(self):
+        # restart if we need to
+        restart_data = self.future_restart_scheduled()
+        if (restart_data and
+           self.should_run_scheduled_action('restart', restart_data['schedule'], self.delete_future_restart)):
+            try:
+                reason_to_cancel = ""
+                # checking the restart filters here seem to be less ugly than moving them into the
+                # run_scheduled_action.
+                if restart_data.get('role') and restart_data['role'] != self.state_handler.role:
+                    reason_to_cancel = "host role mismatch"
+
+                if (restart_data.get('postgres_version') and
+                   self.state_hander.postgres_version_to_int(restart_data['postgres_version']) <=
+                   int(self.state_hander.server_version)):
+                    reason_to_cancel = "postgres version mismatch"
+
+                if 'with_pending_restart_flag' in restart_data and not self.state_handler.pending_restart:
+                    reason_to_cancel = "pending restart flag is not set"
+
+                if not reason_to_cancel:
+                    if self.state_handler.restart():
+                        logger.info("Scheduled restart successfull")
+                    else:
+                        logger.warning("Scheduled restart failed")
+                else:
+                    logger.info("not proceeding with the scheduled restart: {0}".format(reason_to_cancel))
+            finally:
+                self.delete_future_restart()
+
     def schedule(self, action):
         with self._async_executor:
             return self._async_executor.schedule(action)
@@ -397,13 +427,20 @@ class Ha(object):
                     return True
         return False
 
-    def delete_future_restart(self):
-        with self._async_executor:
+    def delete_future_restart(self, take_lock=False):
+        if take_lock:
+            with self._async_executor:
+                self.patroni.scheduled_restart = {}
+        else:
             self.patroni.scheduled_restart = {}
 
     def immediate_restart_scheduled(self):
         return self._async_executor.scheduled_action == 'restart'
 
+    def future_restart_scheduled(self):
+        return self.patroni.scheduled_restart.copy() if (self.patroni.scheduled_restart and
+                                                         isinstance(self.patroni.scheduled_restart, dict)) else None
+
     def schedule_reinitialize(self):
         return self.schedule('reinitialize')
 
@@ -517,6 +554,7 @@ class Ha(object):
                 if self.cluster.is_unlocked():
                     return self.process_unhealthy_cluster()
                 else:
+                    self.evaluate_scheduled_restart()
                     return self.process_healthy_cluster()
             finally:
                 # we might not have a valid PostgreSQL connection here if another thread
diff --git a/patroni/postgresql.py b/patroni/postgresql.py
index 4016c6d7..8951e4c3 100644
--- a/patroni/postgresql.py
+++ b/patroni/postgresql.py
@@ -912,3 +912,29 @@ $$""".format(name, ' '.join(options)), name, password, password)
                 time.sleep(5)
 
         return ret
+
+    @staticmethod
+    def postgres_version_to_int(pg_version):
+        """ Convert the server_version to integer
+
+        >>> Postgresql.postgres_version_to_int('9.5.3')
+        90503
+        >>> Postgresql.postgres_version_to_int('9.3.13')
+        90313
+        >>> Postgresql.postgres_version_to_int('10.1')
+        100100
+        """
+        components = pg_version.split('.')
+
+        result = []
+        if len(components) < 2 or len(components) > 3:
+            raise Exception("Invalid PostgreSQL format: X.Y or X.Y.Z is accepted: {0}".format(pg_version))
+        if len(components) == 2:
+            # new style verion numbers, i.e. 10.1
+            components.append('0')
+        try:
+            result = [c if int(c) > 10 else '0{0}'.format(c) for c in components]
+            result = int(''.join(result))
+        except ValueError:
+            raise Exception("Exception when parsing PostgreSQL version: {0}".format(pg_version))
+        return result
diff --git a/tests/test_api.py b/tests/test_api.py
index d0974e22..6b9f93de 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -3,6 +3,7 @@ import json
 import psycopg2
 import unittest
 
+import dateutil.parser
 from mock import Mock, patch
 from patroni.api import RestApiHandler, RestApiServer
 from patroni.dcs import ClusterConfig, Member
@@ -66,7 +67,7 @@ class MockPatroni(object):
     tags = {}
     version = '0.00'
     noloadbalance = Mock(return_value=False)
-    scheduled_restart = {'schedule': '2016-08-29 12:45TZ+1'}
+    scheduled_restart = {'schedule': dateutil.parser.parse('2016-08-29 12:45TZ+1')}
 
     @staticmethod
     def sighup_handler():

From 568eb730bcd3c80e1a7746f577b00f960d057201 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Fri, 24 Jun 2016 17:39:04 +0200
Subject: [PATCH 07/21] Clear the scheduled restart after the normal one.

Make sure the scheduled restart flag is cleared when the
postmaster_start_time changes since the time restart was scheduled.

Additionally, separate the logic of checking the restart conditions
into the function in order to support conditions for the normal
restart as well.
---
 patroni/api.py        |  2 ++
 patroni/ha.py         | 50 ++++++++++++++++++++++++++++---------------
 patroni/postgresql.py |  7 ++++++
 tests/test_api.py     |  7 +++++-
 tests/test_ha.py      |  5 ++++-
 5 files changed, 52 insertions(+), 19 deletions(-)

diff --git a/patroni/api.py b/patroni/api.py
index b91eea30..3fbab534 100644
--- a/patroni/api.py
+++ b/patroni/api.py
@@ -66,6 +66,7 @@ class RestApiHandler(BaseHTTPRequestHandler):
         response['patroni'] = {'version': patroni.version, 'scope': patroni.postgresql.scope}
         if patroni.scheduled_restart and isinstance(patroni.scheduled_restart, dict):
             response['scheduled_restart'] = patroni.scheduled_restart.copy()
+            del response['scheduled_restart']['postmaster_start_time']
             response['scheduled_restart']['schedule'] = (response['scheduled_restart']['schedule']).isoformat()
         self._write_json_response(status_code, response)
 
@@ -227,6 +228,7 @@ class RestApiHandler(BaseHTTPRequestHandler):
                     data = "Schedule required for the scheduled restart"
                     status_code = 400
                 else:
+                    request['postmaster_start_time'] = self.server.patroni.ha.state_handler.postmaster_start_time()
                     if self.server.patroni.ha.schedule_future_restart(request):
                         data = "Restart scheduled"
                         status_code = 202
diff --git a/patroni/ha.py b/patroni/ha.py
index 8c282c95..6ac09533 100644
--- a/patroni/ha.py
+++ b/patroni/ha.py
@@ -388,33 +388,49 @@ class Ha(object):
     def evaluate_scheduled_restart(self):
         # restart if we need to
         restart_data = self.future_restart_scheduled()
+        if restart_data:
+            recent_time = self.state_handler.postmaster_start_time()
+            request_time = restart_data['postmaster_start_time']
+            # check if postmaster start time has changed since the last restart
+            if recent_time and request_time and recent_time != request_time:
+                logger.info("Cancelling scheduled restart: postgres restart has already happened at {0}".
+                            format(recent_time))
+                self.delete_future_restart()
+                return
+
         if (restart_data and
            self.should_run_scheduled_action('restart', restart_data['schedule'], self.delete_future_restart)):
             try:
-                reason_to_cancel = ""
-                # checking the restart filters here seem to be less ugly than moving them into the
-                # run_scheduled_action.
-                if restart_data.get('role') and restart_data['role'] != self.state_handler.role:
-                    reason_to_cancel = "host role mismatch"
-
-                if (restart_data.get('postgres_version') and
-                   self.state_hander.postgres_version_to_int(restart_data['postgres_version']) <=
-                   int(self.state_hander.server_version)):
-                    reason_to_cancel = "postgres version mismatch"
-
-                if 'with_pending_restart_flag' in restart_data and not self.state_handler.pending_restart:
-                    reason_to_cancel = "pending restart flag is not set"
-
-                if not reason_to_cancel:
+                if self.scheduled_restart_matches(restart_data.get('role'),
+                                                  restart_data.get('postgres_version'),
+                                                  ('with_pending_restart_flag' in restart_data)):
                     if self.state_handler.restart():
                         logger.info("Scheduled restart successfull")
                     else:
                         logger.warning("Scheduled restart failed")
-                else:
-                    logger.info("not proceeding with the scheduled restart: {0}".format(reason_to_cancel))
             finally:
                 self.delete_future_restart()
 
+    def scheduled_restart_matches(self, role, postgres_version, pending_restart):
+        reason_to_cancel = ""
+        # checking the restart filters here seem to be less ugly than moving them into the
+        # run_scheduled_action.
+        if role and role != self.state_handler.role:
+            reason_to_cancel = "host role mismatch"
+
+        if (postgres_version and
+           self.state_hander.postgres_version_to_int(postgres_version) <= int(self.state_hander.server_version)):
+            reason_to_cancel = "postgres version mismatch"
+
+        if pending_restart and not self.state_handler.pending_restart:
+            reason_to_cancel = "pending restart flag is not set"
+
+        if not reason_to_cancel:
+            return True
+        else:
+            logger.info("not proceeding with the scheduled restart: {0}".format(reason_to_cancel))
+        return False
+
     def schedule(self, action):
         with self._async_executor:
             return self._async_executor.schedule(action)
diff --git a/patroni/postgresql.py b/patroni/postgresql.py
index 8951e4c3..1120f842 100644
--- a/patroni/postgresql.py
+++ b/patroni/postgresql.py
@@ -806,6 +806,13 @@ $$""".format(name, ' '.join(options)), name, password, password)
             self._replication_slots = [r[0] for r in cursor]
             self._schedule_load_slots = False
 
+    def postmaster_start_time(self):
+        try:
+            cursor = self.query("""SELECT to_char(pg_postmaster_start_time(), 'YYYY-MM-DD HH24:MI:SS.MS TZ')""")
+            return cursor.fetchone()[0]
+        except psycopg2.Error:
+            return None
+
     def sync_replication_slots(self, cluster):
         if self.use_slots:
             try:
diff --git a/tests/test_api.py b/tests/test_api.py
index 6b9f93de..e50b58bb 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -26,6 +26,10 @@ class MockPostgresql(object):
     def connection():
         return psycopg2_connect()
 
+    @staticmethod
+    def postmaster_start_time():
+        return '2016-08-20 12:00TZ+1'
+
 
 class MockHa(object):
 
@@ -67,7 +71,8 @@ class MockPatroni(object):
     tags = {}
     version = '0.00'
     noloadbalance = Mock(return_value=False)
-    scheduled_restart = {'schedule': dateutil.parser.parse('2016-08-29 12:45TZ+1')}
+    scheduled_restart = {'schedule': dateutil.parser.parse('2016-08-29 12:45TZ+1'),
+                         'postmaster_start_time': '2016-08-20 12:00TZ+1'}
 
     @staticmethod
     def sighup_handler():
diff --git a/tests/test_ha.py b/tests/test_ha.py
index 71e7087f..b8386fee 100644
--- a/tests/test_ha.py
+++ b/tests/test_ha.py
@@ -81,7 +81,8 @@ zookeeper:
         self.replicatefrom = None
         self.api.connection_string = 'http://127.0.0.1:8008'
         self.clonefrom = None
-        self.scheduled_restart = {'schedule': dateutil.parser.parse('2016-08-29 12:45TZ+1')}
+        self.scheduled_restart = {'schedule': dateutil.parser.parse('2016-08-29 12:45TZ+1'),
+                                  'postmaster_start_time': '2016-08-20 12:00TZ+1'}
 
 
 def run_async(func, args=()):
@@ -119,6 +120,7 @@ class TestHa(unittest.TestCase):
                                                 'hot_standby': 'on', 'max_wal_senders': 5, 'wal_keep_segments': 8}})
             self.p.set_state('running')
             self.p.set_role('replica')
+            self.p.postmaster_start_time = MagicMock(return_value="2016-08-20 12:00TZ+1")
             self.p.check_replication_lag = true
             self.p.can_create_replica_without_replication_connection = MagicMock(return_value=False)
             self.e = get_dcs({'etcd': {'ttl': 30, 'host': 'ok:2379', 'scope': 'test',
@@ -128,6 +130,7 @@ class TestHa(unittest.TestCase):
             self.ha.old_cluster = self.e.get_cluster()
             self.ha.cluster = get_cluster_not_initialized_without_leader()
             self.ha.load_cluster_from_dcs = Mock()
+            #self.ha.evaluate_scheduled_restart = true
 
     def test_update_lock(self):
         self.p.last_operation = Mock(side_effect=PostgresException(''))

From 854ff27e56873ec6c014e50c53c1920680334a84 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Mon, 27 Jun 2016 09:50:09 +0200
Subject: [PATCH 08/21] Allow normal conditional restarts.

In addition, use the RLock instead of the Lock in async executor
to make sure the lock can be acquired more than once from a single
thread.
---
 patroni/api.py            | 74 +++++++++++++++++++--------------------
 patroni/async_executor.py |  7 ++--
 patroni/ha.py             | 28 ++++++++-------
 3 files changed, 56 insertions(+), 53 deletions(-)

diff --git a/patroni/api.py b/patroni/api.py
index 3fbab534..c6b5e225 100644
--- a/patroni/api.py
+++ b/patroni/api.py
@@ -195,51 +195,51 @@ class RestApiHandler(BaseHTTPRequestHandler):
         status_code = 500
         data = 'restart failed'
         request = self._read_json_content(body_is_optional=True)
-        if not request:  # unconditional restart
-            try:
-                status, data = self.server.patroni.ha.restart()
-                status_code = 200 if status else 503
-            except Exception:
-                logger.exception('Exception during restart')
-        else:
-            logger.info("received scheduled restart request: {0}".format(request))
-            for k in request:
-                if k == 'schedule':
-                    (_, data, request[k]) = self.parse_schedule(request[k], "restart")
-                    if _:
-                        status_code = _
-                        break
-                elif k == 'role':
-                    if request[k] not in ('master', 'replica'):
-                        status_code = 400
-                        data = "PostgreSQL role should be either master or replica"
-                        break
-                elif k == 'postgres_version':
-                    if not re.match(r'([1-9][0-9]?\.){2}[1-9][0-9]?$', request[k]):
-                        status_code = 400
-                        data = "PostgreSQL version should be in the first.major.minor format"
-                        break
-                elif k != 'with_pending_restart_flag':
-                    status_code = 400
-                    data = "Unknown filter for the scheduled restart: {0}".format(k)
+        request = request or {}
+        if request:
+            logger.debug("received restart request: {0}".format(request))
+        for k in request:
+            if k == 'schedule':
+                (_, data, request[k]) = self.parse_schedule(request[k], "restart")
+                if _:
+                    status_code = _
                     break
-            else:
-                if 'schedule' not in request:
+            elif k == 'role':
+                if request[k] not in ('master', 'replica'):
+                    status_code = 400
+                    data = "PostgreSQL role should be either master or replica"
+                    break
+            elif k == 'postgres_version':
+                if not re.match(r'([1-9][0-9]?\.){2}[1-9][0-9]?$', request[k]):
+                    status_code = 400
+                    data = "PostgreSQL version should be in the first.major.minor format"
+                    break
+            elif k != 'with_pending_restart_flag':
+                status_code = 400
+                data = "Unknown filter for the scheduled restart: {0}".format(k)
+                break
+        else:
+            if 'schedule' not in request:
+                try:
+                    status, data = self.server.patroni.ha.restart()
+                    status_code = 200 if status else 503
+                except Exception:
+                    logger.exception('Exception during restart')
                     data = "Schedule required for the scheduled restart"
                     status_code = 400
+            else:
+                request['postmaster_start_time'] = self.server.patroni.ha.state_handler.postmaster_start_time()
+                if self.server.patroni.ha.schedule_future_restart(request):
+                    data = "Restart scheduled"
+                    status_code = 202
                 else:
-                    request['postmaster_start_time'] = self.server.patroni.ha.state_handler.postmaster_start_time()
-                    if self.server.patroni.ha.schedule_future_restart(request):
-                        data = "Restart scheduled"
-                        status_code = 202
-                    else:
-                        data = "Another restart is already scheduled"
-                        status_code = 409
+                    data = "Another restart is already scheduled"
+                    status_code = 409
         self._write_response(status_code, data)
 
     @check_auth
     def do_DELETE_restart(self):
-        self.server.patroni.ha.delete_future_restart(take_lock=True)
+        self.server.patroni.ha.delete_future_restart()
         data = "scheduled restart deleted"
         code = 200
         self._write_response(code, data)
diff --git a/patroni/async_executor.py b/patroni/async_executor.py
index e009ab19..640ef992 100644
--- a/patroni/async_executor.py
+++ b/patroni/async_executor.py
@@ -1,5 +1,5 @@
 import logging
-from threading import Lock, Thread
+from threading import RLock, Thread
 
 logger = logging.getLogger(__name__)
 
@@ -8,9 +8,9 @@ class AsyncExecutor(object):
 
     def __init__(self):
         self._busy = False
-        self._thread_lock = Lock()
+        self._thread_lock = RLock()
         self._scheduled_action = None
-        self._scheduled_action_lock = Lock()
+        self._scheduled_action_lock = RLock()
 
     @property
     def busy(self):
@@ -32,6 +32,7 @@ class AsyncExecutor(object):
     def reset_scheduled_action(self):
         with self._scheduled_action_lock:
             self._scheduled_action = None
+            self._busy = False
 
     def run(self, func, args=()):
         try:
diff --git a/patroni/ha.py b/patroni/ha.py
index 6ac09533..b24bef94 100644
--- a/patroni/ha.py
+++ b/patroni/ha.py
@@ -401,17 +401,15 @@ class Ha(object):
         if (restart_data and
            self.should_run_scheduled_action('restart', restart_data['schedule'], self.delete_future_restart)):
             try:
-                if self.scheduled_restart_matches(restart_data.get('role'),
-                                                  restart_data.get('postgres_version'),
-                                                  ('with_pending_restart_flag' in restart_data)):
-                    if self.state_handler.restart():
+                    ret, message = self.restart(restart_data)
+                    if ret:
                         logger.info("Scheduled restart successfull")
                     else:
-                        logger.warning("Scheduled restart failed")
+                        logger.warning("Scheduled restart: {0}".format(message))
             finally:
                 self.delete_future_restart()
 
-    def scheduled_restart_matches(self, role, postgres_version, pending_restart):
+    def restart_matches(self, role, postgres_version, pending_restart):
         reason_to_cancel = ""
         # checking the restart filters here seem to be less ugly than moving them into the
         # run_scheduled_action.
@@ -428,7 +426,7 @@ class Ha(object):
         if not reason_to_cancel:
             return True
         else:
-            logger.info("not proceeding with the scheduled restart: {0}".format(reason_to_cancel))
+            logger.info("not proceeding with the restart: {0}".format(reason_to_cancel))
         return False
 
     def schedule(self, action):
@@ -443,11 +441,8 @@ class Ha(object):
                     return True
         return False
 
-    def delete_future_restart(self, take_lock=False):
-        if take_lock:
-            with self._async_executor:
-                self.patroni.scheduled_restart = {}
-        else:
+    def delete_future_restart(self):
+        with self._async_executor:
             self.patroni.scheduled_restart = {}
 
     def immediate_restart_scheduled(self):
@@ -463,7 +458,14 @@ class Ha(object):
     def reinitialize_scheduled(self):
         return self._async_executor.scheduled_action == 'reinitialize'
 
-    def restart(self):
+    def restart(self, restart_data=None):
+        """ conditional and unconditional restart """
+        if (restart_data and isinstance(restart_data, dict) and
+            not self.restart_matches(restart_data.get('role'),
+                                     restart_data.get('postgres_version'),
+                                     ('with_pending_restart_flag' in restart_data))):
+            return (False, "restart conditions are not satisfied")
+
         with self._async_executor:
             prev = self._async_executor.schedule('restart', True)
             if prev is not None:

From d2832ee43bc3fa8efdd39fda13d44df6c66573c9 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Tue, 28 Jun 2016 16:54:20 +0200
Subject: [PATCH 09/21] Address the code review.

Fix return  value in the should_run_scheduled_action and the comments.
Correct the json composition in the scheduled_restart test.
Fix the delete in case there is no scheduled restart.
Fix the usage of format in the logger output.
Fix the indentation in the evaluate_scheduled_restart.
Fix the condition related to the body_is_optional in the do_POST_restart.
Fix a few typos in the error messages.
Fix the _read_json_content
Make the scheduled restart unit-tests a bit less ugly
---
 features/steps/patroni_api.py |  9 ++------
 patroni/api.py                | 16 ++++++++------
 patroni/ha.py                 | 40 ++++++++++++++++++++---------------
 tests/test_api.py             | 21 ++++++++++--------
 4 files changed, 47 insertions(+), 39 deletions(-)

diff --git a/features/steps/patroni_api.py b/features/steps/patroni_api.py
index 4d2f1709..28aaa18a 100644
--- a/features/steps/patroni_api.py
+++ b/features/steps/patroni_api.py
@@ -106,13 +106,8 @@ def scheduled_failover(context, at_url, from_host, to_host, in_seconds):
 @step('I issue a scheduled restart at {url:url} in {in_seconds:d} seconds with {data}')
 def scheduled_restart(context, url, in_seconds, data):
     data = data and json.loads(data) or {}
-    restart_options = ['"schedule": "{0}"'.format((datetime.now(pytz.utc) + timedelta(seconds=int(in_seconds))).isoformat())]
-    for key in data:
-        if key == 'schedule':
-            continue
-        restart_options.append('"{0}": "{1}"'.format(key, data[key]))
-    context.execute_steps(u"""Given I issue a POST request to {0}/restart with {{{1}}}""".
-                          format(url, ','.join(restart_options)))
+    data.update(schedule='{0}'.format((datetime.now(pytz.utc) + timedelta(seconds=int(in_seconds))).isoformat()))
+    context.execute_steps(u"""Given I issue a POST request to {0}/restart with {1}""".format(url, json.dumps(data)))
 
 
 @step('I add tag {tag:w} {value:w} to {pg_name:w} config')
diff --git a/patroni/api.py b/patroni/api.py
index c6b5e225..c9d64408 100644
--- a/patroni/api.py
+++ b/patroni/api.py
@@ -119,7 +119,7 @@ class RestApiHandler(BaseHTTPRequestHandler):
 
     def _read_json_content(self, body_is_optional=False):
         if 'content-length' not in self.headers:
-            return self.send_error(411) if body_is_optional else None
+            return self.send_error(411) if not body_is_optional else {}
         try:
             content_length = int(self.headers.get('content-length'))
             if content_length == 0 and body_is_optional:
@@ -195,7 +195,9 @@ class RestApiHandler(BaseHTTPRequestHandler):
         status_code = 500
         data = 'restart failed'
         request = self._read_json_content(body_is_optional=True)
-        request = request or {}
+        if request is None:
+            # failed to parse the json
+            return
         if request:
             logger.debug("received restart request: {0}".format(request))
         for k in request:
@@ -225,7 +227,6 @@ class RestApiHandler(BaseHTTPRequestHandler):
                     status_code = 200 if status else 503
                 except Exception:
                     logger.exception('Exception during restart')
-                    data = "Schedule required for the scheduled restart"
                     status_code = 400
             else:
                 request['postmaster_start_time'] = self.server.patroni.ha.state_handler.postmaster_start_time()
@@ -239,9 +240,12 @@ class RestApiHandler(BaseHTTPRequestHandler):
 
     @check_auth
     def do_DELETE_restart(self):
-        self.server.patroni.ha.delete_future_restart()
-        data = "scheduled restart deleted"
-        code = 200
+        if self.server.patroni.ha.delete_future_restart():
+            data = "scheduled restart deleted"
+            code = 200
+        else:
+            data = "no restarts are scheduled"
+            code = 404
         self._write_response(code, data)
 
     @check_auth
diff --git a/patroni/ha.py b/patroni/ha.py
index b24bef94..d989ae06 100644
--- a/patroni/ha.py
+++ b/patroni/ha.py
@@ -174,7 +174,7 @@ class Ha(object):
             tags = json.get('tags', dict())
             return (member, True, not is_master, xlog_location, tags)
         except:
-            logging.exception('request failed: GET %s', member.api_url)
+            logger.exception('request failed: GET %s', member.api_url)
         return (member, False, None, 0, {})
 
     def fetch_nodes_statuses(self, members):
@@ -293,33 +293,35 @@ class Ha(object):
 
     def should_run_scheduled_action(self, action_name, scheduled_at, cleanup_fn):
         if scheduled_at:
-            # If the faildover is in the far future, we shouldn't do anything and just return.
-            # If the failover is in the past, we consider the value to be stale and we remove
+            # If the scheduled action is in the far future, we shouldn't do anything and just return.
+            # If the scheduled action is in the past, we consider the value to be stale and we remove
             # the value.
-            # If the value is close to now, we initiate the failover
+            # If the value is close to now, we initiate the scheduled action
+            # Additionally, if the scheduled action cannot be executed altogether, i.e. there is an error
+            # or the action is in the past - we take care of cleaning it up.
             now = datetime.datetime.now(pytz.utc)
             try:
                 delta = (scheduled_at - now).total_seconds()
 
                 if delta > self.patroni.nap_time:
-                    logging.info('Awaiting {0} at %s (in %.0f seconds)'.format(action_name),
-                                 scheduled_at.isoformat(), delta)
+                    logger.info('Awaiting %s at %s (in %.0f seconds)',
+                                action_name, scheduled_at.isoformat(), delta)
                     return False
                 elif delta < - int(self.patroni.nap_time * 1.5):
-                    logger.warning('Found a stale {0} value, cleaning up: %s'.format(action_name),
-                                   scheduled_at.isoformat())
+                    logger.warning('Found a stale %s value, cleaning up: %s',
+                                   action_name, scheduled_at.isoformat())
                     cleanup_fn()
                     self.dcs.manual_failover('', '', index=self.cluster.failover.index)
-                    return None
+                    return False
 
                 # The value is very close to now
                 sleep(max(delta, 0))
                 logger.info('Manual scheduled {0} at %s'.format(action_name), scheduled_at.isoformat())
                 return True
             except TypeError:
-                logger.warning('Incorrect value in of scheduled_at: %s', scheduled_at)
+                logger.warning('Incorrect value of scheduled_at: %s', scheduled_at)
                 cleanup_fn()
-        return None
+        return False
 
     def process_manual_failover_from_leader(self):
         failover = self.cluster.failover
@@ -401,11 +403,11 @@ class Ha(object):
         if (restart_data and
            self.should_run_scheduled_action('restart', restart_data['schedule'], self.delete_future_restart)):
             try:
-                    ret, message = self.restart(restart_data)
-                    if ret:
-                        logger.info("Scheduled restart successfull")
-                    else:
-                        logger.warning("Scheduled restart: {0}".format(message))
+                ret, message = self.restart(restart_data)
+                if ret:
+                    logger.info("Scheduled restart successful")
+                else:
+                    logger.warning("Scheduled restart: {0}".format(message))
             finally:
                 self.delete_future_restart()
 
@@ -442,8 +444,12 @@ class Ha(object):
         return False
 
     def delete_future_restart(self):
+        ret = False
         with self._async_executor:
-            self.patroni.scheduled_restart = {}
+            if self.patroni.scheduled_restart:
+                self.patroni.scheduled_restart = {}
+                ret = True
+        return ret
 
     def immediate_restart_scheduled(self):
         return self._async_executor.scheduled_action == 'restart'
diff --git a/tests/test_api.py b/tests/test_api.py
index e50b58bb..9a722b42 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -72,7 +72,7 @@ class MockPatroni(object):
     version = '0.00'
     noloadbalance = Mock(return_value=False)
     scheduled_restart = {'schedule': dateutil.parser.parse('2016-08-29 12:45TZ+1'),
-                         'postmaster_start_time': '2016-08-20 12:00TZ+1'}
+                         'postmaster_start_time': postgresql.postmaster_start_time()}
 
     @staticmethod
     def sighup_handler():
@@ -184,24 +184,27 @@ class TestRestApiHandler(unittest.TestCase):
             MockRestApiServer(RestApiHandler, request)
 
         post = request + '\nContent-Length: '
-        # wrong version
-        request = post + '84\n\n{"schedule": "2016-08-20 12:45TZ+1", "role": "unknown", "postgres_version": "9.5.3"}'
-        MockRestApiServer(RestApiHandler, request)
+
+        def make_request(request):
+            return '{0}{1}\n\n{2}'.format(post, len(request), request)
         # wrong role
-        request = post + '85\n\n{"schedule": "2016-08-20 12:45TZ+1", "role": "master", "postgres_version": "9.5.3.1"}'
+        request = make_request('{"schedule": "2016-08-20 12:45TZ+1", "role": "unknown", "postgres_version": "9.5.3"}')
+        MockRestApiServer(RestApiHandler, request)
+        # wrong version
+        request = make_request('{"schedule": "2016-08-20 12:45TZ+1", "role": "master", "postgres_version": "9.5.3.1"}')
         MockRestApiServer(RestApiHandler, request)
         # unknown filter
-        request = post + '55\n\n{"schedule": "2016-08-29 12:45TZ+1", "batman": "lives"}'
+        request = make_request('{"schedule": "2016-08-29 12:45TZ+1", "batman": "lives"}')
         MockRestApiServer(RestApiHandler, request)
         # incorrect schedule
-        request = post + '55\n\n{"schedule": "2016-08-42 12:45TZ+1", "role": "master"}'
+        request = make_request('{"schedule": "2016-08-42 12:45TZ+1", "role": "master"}')
         MockRestApiServer(RestApiHandler, request)
         # everything fine, but the schedule is missing
-        request = post + '47\n\n{"role": "master", "postgres_version": "9.5.2"}'
+        request = make_request('{"role": "master", "postgres_version": "9.5.2"}')
         MockRestApiServer(RestApiHandler, request)
         for retval in (True, False):
             with patch.object(MockHa, 'schedule_future_restart', Mock(return_value=retval)):
-                request = post + '36\n\n{"schedule": "2016-08-29 12:45TZ+1"}'
+                request = make_request('{"schedule": "2016-08-29 12:45TZ+1"}')
                 MockRestApiServer(RestApiHandler, request)
 
     def test_do_DELETE_restart(self):

From 7a1e2e0c72c35a998bc08df16aa349dd44eebe90 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Tue, 28 Jun 2016 17:11:13 +0200
Subject: [PATCH 10/21] Fix the assert message.

---
 features/steps/patroni_api.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/features/steps/patroni_api.py b/features/steps/patroni_api.py
index 28aaa18a..dd260aab 100644
--- a/features/steps/patroni_api.py
+++ b/features/steps/patroni_api.py
@@ -124,7 +124,7 @@ def check_http_response(context, url, value, timeout, negate=False):
         time.sleep(1)
     else:
         assert False,\
-            "Value {0} is {0} present in response after {1} seconds".format("not" if not negate else "", value, timeout)
+            "Value {0} is {1} present in response after {2} seconds".format(value, "not" if not negate else "", timeout)
 
 
 @then('Response on GET {url} does not contain {value} after {timeout:d} seconds')

From 36a86c67d09a8ef63c909965f1a258dfff191efe Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Mon, 4 Jul 2016 15:46:22 +0200
Subject: [PATCH 11/21] Enable the conditions on normal restart.

---
 patroni/api.py | 2 +-
 patroni/ha.py  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/patroni/api.py b/patroni/api.py
index c9d64408..207dd746 100644
--- a/patroni/api.py
+++ b/patroni/api.py
@@ -223,7 +223,7 @@ class RestApiHandler(BaseHTTPRequestHandler):
         else:
             if 'schedule' not in request:
                 try:
-                    status, data = self.server.patroni.ha.restart()
+                    status, data = self.server.patroni.ha.restart(request)
                     status_code = 200 if status else 503
                 except Exception:
                     logger.exception('Exception during restart')
diff --git a/patroni/ha.py b/patroni/ha.py
index d989ae06..1da78027 100644
--- a/patroni/ha.py
+++ b/patroni/ha.py
@@ -419,7 +419,7 @@ class Ha(object):
             reason_to_cancel = "host role mismatch"
 
         if (postgres_version and
-           self.state_hander.postgres_version_to_int(postgres_version) <= int(self.state_hander.server_version)):
+           self.state_handler.postgres_version_to_int(postgres_version) <= int(self.state_handler.server_version)):
             reason_to_cancel = "postgres version mismatch"
 
         if pending_restart and not self.state_handler.pending_restart:

From 8834f929aa2f3d421a54e4390ac753436bc1e227 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Tue, 5 Jul 2016 10:07:29 +0200
Subject: [PATCH 12/21] Improve the unit tests/coverage.

---
 patroni/postgresql.py    | 10 +++++++++-
 tests/test_api.py        | 10 ++++++++++
 tests/test_ha.py         | 19 ++++++++++++++++---
 tests/test_postgresql.py |  6 ++++++
 4 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/patroni/postgresql.py b/patroni/postgresql.py
index 1120f842..a0424e3a 100644
--- a/patroni/postgresql.py
+++ b/patroni/postgresql.py
@@ -930,6 +930,14 @@ $$""".format(name, ' '.join(options)), name, password, password)
         90313
         >>> Postgresql.postgres_version_to_int('10.1')
         100100
+        >>> Postgresql.postgres_version_to_int('10')
+        Traceback (most recent call last):
+            ...
+        Exception: Invalid PostgreSQL format: X.Y or X.Y.Z is accepted: 10
+        >>> Postgresql.postgres_version_to_int('a.b.c')
+        Traceback (most recent call last):
+            ...
+        Exception: Invalid PostgreSQL version: a.b.c
         """
         components = pg_version.split('.')
 
@@ -943,5 +951,5 @@ $$""".format(name, ' '.join(options)), name, password, password)
             result = [c if int(c) > 10 else '0{0}'.format(c) for c in components]
             result = int(''.join(result))
         except ValueError:
-            raise Exception("Exception when parsing PostgreSQL version: {0}".format(pg_version))
+            raise Exception("Invalid PostgreSQL version: {0}".format(pg_version))
         return result
diff --git a/tests/test_api.py b/tests/test_api.py
index 9a722b42..44e1662d 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -187,6 +187,13 @@ class TestRestApiHandler(unittest.TestCase):
 
         def make_request(request):
             return '{0}{1}\n\n{2}'.format(post, len(request), request)
+
+        # empty request
+        request = make_request('')
+        MockRestApiServer(RestApiHandler, request)
+        # invalid request
+        request = make_request('foobar=baz')
+        MockRestApiServer(RestApiHandler, request)
         # wrong role
         request = make_request('{"schedule": "2016-08-20 12:45TZ+1", "role": "unknown", "postgres_version": "9.5.3"}')
         MockRestApiServer(RestApiHandler, request)
@@ -206,6 +213,9 @@ class TestRestApiHandler(unittest.TestCase):
             with patch.object(MockHa, 'schedule_future_restart', Mock(return_value=retval)):
                 request = make_request('{"schedule": "2016-08-29 12:45TZ+1"}')
                 MockRestApiServer(RestApiHandler, request)
+            with patch.object(MockHa, 'restart', Mock(return_value=(retval, "foo"))):
+                request = make_request('{"role": "master", "postgres_version": "9.5.2"}')
+                MockRestApiServer(RestApiHandler, request)
 
     def test_do_DELETE_restart(self):
         for retval in (True, False):
diff --git a/tests/test_ha.py b/tests/test_ha.py
index b8386fee..0029a80c 100644
--- a/tests/test_ha.py
+++ b/tests/test_ha.py
@@ -130,7 +130,6 @@ class TestHa(unittest.TestCase):
             self.ha.old_cluster = self.e.get_cluster()
             self.ha.cluster = get_cluster_not_initialized_without_leader()
             self.ha.load_cluster_from_dcs = Mock()
-            #self.ha.evaluate_scheduled_restart = true
 
     def test_update_lock(self):
         self.p.last_operation = Mock(side_effect=PostgresException(''))
@@ -403,8 +402,22 @@ class TestHa(unittest.TestCase):
 
     def test_schedule_future_restart(self):
         self.ha.patroni.scheduled_restart = {}
-        self.ha.schedule_future_restart({'schedule': '2016-08-30 12:45TZ+1"'})
-        self.ha.schedule_future_restart({'schedule': '2016-08-30 12:45TZ+1"'})
+        # do the restart 2 times. The first one should succeed, the second one should fail
+        self.assertTrue(self.ha.schedule_future_restart({'schedule': '2016-08-30 12:45TZ+1"'}))
+        self.assertFalse(self.ha.schedule_future_restart({'schedule': '2016-08-30 12:45TZ+1"'}))
 
     def test_delete_future_restarts(self):
         self.ha.delete_future_restart()
+
+    def test_evaluate_scheduled_restart(self):
+        self.p.postmaster_start_time = Mock(return_value='2016-08-31 12:45TZ+1')
+        with patch.object(self.ha,
+                          'future_restart_scheduled', Mock(return_value={'postmaster_start_time': '2016-08-30 12:45TZ+1',
+                                                                         'schedule': '2016-08-31 12:45TZ+1'})):
+            self.ha.evaluate_scheduled_restart()
+        with patch.object(self.ha,
+                          'future_restart_scheduled', Mock(return_value={'postmaster_start_time': '2016-08-31 12:45TZ+1',
+                                                                         'schedule': '2016-08-31 12:45TZ+1'})):
+            with patch.object(self.ha,
+                              'should_run_scheduled_action', Mock(return_value=True)):
+                self.ha.evaluate_scheduled_restart()
diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py
index 42dcab0a..0ef36441 100644
--- a/tests/test_postgresql.py
+++ b/tests/test_postgresql.py
@@ -518,3 +518,9 @@ class TestPostgresql(unittest.TestCase):
             self.assertEquals(self.p.get_major_version(), 9.4)
         with patch.object(builtins, 'open', Mock(side_effect=Exception)):
             self.assertEquals(self.p.get_major_version(), 0.0)
+
+    def test_postmaster_start_time(self):
+        with patch.object(MockCursor, "fetchone", Mock(return_value=('foo', True, '', '', '', '', False))):
+            self.assertEqual(self.p.postmaster_start_time(), 'foo')
+        with patch.object(MockCursor, "execute", side_effect=psycopg2.Error):
+            self.assertIsNone(self.p.postmaster_start_time())

From 6da2eecb9067ed950073151c443e4600b9fdff66 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Mon, 11 Jul 2016 11:51:07 +0300
Subject: [PATCH 13/21] Increase the test coverage.

---
 tests/test_ha.py | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tests/test_ha.py b/tests/test_ha.py
index 0029a80c..bb6a0155 100644
--- a/tests/test_ha.py
+++ b/tests/test_ha.py
@@ -283,6 +283,8 @@ class TestHa(unittest.TestCase):
         self.assertEquals(self.ha.restart(), (False, 'restart failed'))
         self.ha.schedule_reinitialize()
         self.assertEquals(self.ha.restart(), (False, 'reinitialize already in progress'))
+        with patch.object(self.ha, "restart_matches", return_value=False):
+            self.assertEquals(self.ha.restart({'foo': 'bar'}), (False, "restart conditions are not satisfied"))
 
     def test_restart_in_progress(self):
         self.ha._async_executor.schedule('restart', True)
@@ -421,3 +423,15 @@ class TestHa(unittest.TestCase):
             with patch.object(self.ha,
                               'should_run_scheduled_action', Mock(return_value=True)):
                 self.ha.evaluate_scheduled_restart()
+                with patch.object(self.ha, 'restart', Mock(return_value=(False, "Test"))):
+                    self.ha.evaluate_scheduled_restart()
+
+    def test_restart_matches(self):
+        self.p._role = 'replica'
+        self.p.server_version = 90500
+        self.p._pending_restart = True
+        self.assertFalse(self.ha.restart_matches("master", "9.5.2", True))
+        self.assertFalse(self.ha.restart_matches("replica", "9.4.3", True))
+        self.p._pending_restart = False
+        self.assertFalse(self.ha.restart_matches("replica", "9.5.2", True))
+        self.assertTrue(self.ha.restart_matches("replica", "9.5.2", False))

From b17483b7dd161450c0b10aeaa7343bf8715a7a87 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Mon, 11 Jul 2016 15:21:31 +0200
Subject: [PATCH 14/21] Fix the PG version regex.

---
 patroni/api.py   | 2 +-
 tests/test_ha.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/patroni/api.py b/patroni/api.py
index fb9fae99..c142c3b3 100644
--- a/patroni/api.py
+++ b/patroni/api.py
@@ -212,7 +212,7 @@ class RestApiHandler(BaseHTTPRequestHandler):
                     data = "PostgreSQL role should be either master or replica"
                     break
             elif k == 'postgres_version':
-                if not re.match(r'([1-9][0-9]?\.){2}[1-9][0-9]?$', request[k]):
+                if not re.match(r'[1-9][0-9]?(\.(0|([1-9][0-9]?))){2}$', request[k]):
                     status_code = 400
                     data = "PostgreSQL version should be in the first.major.minor format"
                     break
diff --git a/tests/test_ha.py b/tests/test_ha.py
index bb6a0155..cd0cd4ae 100644
--- a/tests/test_ha.py
+++ b/tests/test_ha.py
@@ -430,7 +430,7 @@ class TestHa(unittest.TestCase):
         self.p._role = 'replica'
         self.p.server_version = 90500
         self.p._pending_restart = True
-        self.assertFalse(self.ha.restart_matches("master", "9.5.2", True))
+        self.assertFalse(self.ha.restart_matches("master", "9.5.0", True))
         self.assertFalse(self.ha.restart_matches("replica", "9.4.3", True))
         self.p._pending_restart = False
         self.assertFalse(self.ha.restart_matches("replica", "9.5.2", True))

From bf95b754896401d4ed5cde79d0023e0e56559b46 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Mon, 11 Jul 2016 18:20:15 +0200
Subject: [PATCH 15/21] Use the parameter that really sets the pending_restart
 flag.

---
 features/patroni_api.feature | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/features/patroni_api.feature b/features/patroni_api.feature
index 8f5c415e..57810173 100644
--- a/features/patroni_api.feature
+++ b/features/patroni_api.feature
@@ -79,7 +79,7 @@ Scenario: check the scheduled failover
 	And replication works from postgres0 to postgres1 after 25 seconds
 
 Scenario: check the scheduled restart
-	Given I issue a PATCH request to http://127.0.0.1:8008/config with {"postgresql": {"parameters": {"checkpoint_warning": "60s"}}}
+	Given I issue a PATCH request to http://127.0.0.1:8008/config with {"postgresql": {"parameters": {"superuser_reserved_connections": "6"}}}
 	Then I receive a response code 200
 		And Response on GET http://127.0.0.1:8008/patroni contains pending_restart after 5 seconds
 	Given I issue a scheduled restart at http://127.0.0.1:8008 in 1 seconds with {"role": "replica"}

From ec160f0d59960a2c881242ab3930c5cb71f8c349 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Mon, 11 Jul 2016 18:20:42 +0200
Subject: [PATCH 16/21] Do not send 2 quotes for the empty request, instead,
 send None.

---
 patroni/ctl.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/patroni/ctl.py b/patroni/ctl.py
index a0f0c565..e45a77a4 100644
--- a/patroni/ctl.py
+++ b/patroni/ctl.py
@@ -127,7 +127,7 @@ def post_patroni(member, endpoint, content, headers=None):
         headers['Content-Type'] = 'application/json'
     return requests.post('{0}://{1}/{2}'.format(url.scheme, url.netloc, endpoint),
                          headers=headers,
-                         data=json.dumps(content), timeout=60)
+                         data=json.dumps(content) if content else None, timeout=60)
 
 
 def print_output(columns, rows=None, alignment=None, fmt='pretty', header=True, delimiter='\t'):

From 3181c4e59f2581ef13630eb17f34e95e6ff2fc03 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Tue, 12 Jul 2016 20:25:01 +0200
Subject: [PATCH 17/21] Code review, asynchronous restarts.

- Make the restart initiated by the schedule asynchronous
- Fix the placeholders in logs.
- Fix the regexp to detect the PostgreSQL version.
---
 patroni/api.py    |  2 +-
 patroni/ha.py     | 50 +++++++++++++++++++++++++++--------------------
 tests/test_api.py |  4 ++--
 tests/test_ha.py  | 13 ++++++++----
 4 files changed, 41 insertions(+), 28 deletions(-)

diff --git a/patroni/api.py b/patroni/api.py
index c142c3b3..d89ec409 100644
--- a/patroni/api.py
+++ b/patroni/api.py
@@ -91,7 +91,7 @@ class RestApiHandler(BaseHTTPRequestHandler):
                 status_code = 503
         elif 'role' in response and response['role'] in path:
             status_code = 503 if response['role'] != 'master' and patroni.noloadbalance else 200
-        elif patroni.ha.immediate_restart_scheduled() and patroni.postgresql.role == 'master' and 'master' in path:
+        elif patroni.ha.restart_scheduled() and patroni.postgresql.role == 'master' and 'master' in path:
             # exceptional case for master node when the postgres is being restarted via API
             status_code = 200
         else:
diff --git a/patroni/ha.py b/patroni/ha.py
index f45a4585..f0803708 100644
--- a/patroni/ha.py
+++ b/patroni/ha.py
@@ -388,19 +388,18 @@ class Ha(object):
             request_time = restart_data['postmaster_start_time']
             # check if postmaster start time has changed since the last restart
             if recent_time and request_time and recent_time != request_time:
-                logger.info("Cancelling scheduled restart: postgres restart has already happened at {0}".
-                            format(recent_time))
+                logger.info("Cancelling scheduled restart: postgres restart has already happened at %s", recent_time)
                 self.delete_future_restart()
-                return
+                return None
 
         if (restart_data and
            self.should_run_scheduled_action('restart', restart_data['schedule'], self.delete_future_restart)):
             try:
-                ret, message = self.restart(restart_data)
-                if ret:
-                    logger.info("Scheduled restart successful")
-                else:
-                    logger.warning("Scheduled restart: {0}".format(message))
+                ret, message = self.restart(restart_data, run_async=True)
+                if not ret:
+                    logger.warning("Scheduled restart: %s", message)
+                    return None
+                return message
             finally:
                 self.delete_future_restart()
 
@@ -421,12 +420,12 @@ class Ha(object):
         if not reason_to_cancel:
             return True
         else:
-            logger.info("not proceeding with the restart: {0}".format(reason_to_cancel))
+            logger.info("not proceeding with the restart: %s", reason_to_cancel)
         return False
 
-    def schedule(self, action):
+    def schedule(self, action, immediate=False):
         with self._async_executor:
-            return self._async_executor.schedule(action)
+            return self._async_executor.schedule(action, immediate)
 
     def schedule_future_restart(self, restart_data):
         if isinstance(restart_data, dict):
@@ -444,9 +443,6 @@ class Ha(object):
                 ret = True
         return ret
 
-    def immediate_restart_scheduled(self):
-        return self._async_executor.scheduled_action == 'restart'
-
     def future_restart_scheduled(self):
         return self.patroni.scheduled_restart.copy() if (self.patroni.scheduled_restart and
                                                          isinstance(self.patroni.scheduled_restart, dict)) else None
@@ -457,7 +453,13 @@ class Ha(object):
     def reinitialize_scheduled(self):
         return self._async_executor.scheduled_action == 'reinitialize'
 
-    def restart(self, restart_data=None):
+    def schedule_restart(self, immediate=False):
+        return self.schedule('restart', immediate)
+
+    def restart_scheduled(self):
+        return self._async_executor.scheduled_action == 'restart'
+
+    def restart(self, restart_data=None, run_async=False):
         """ conditional and unconditional restart """
         if (restart_data and isinstance(restart_data, dict) and
             not self.restart_matches(restart_data.get('role'),
@@ -466,13 +468,17 @@ class Ha(object):
             return (False, "restart conditions are not satisfied")
 
         with self._async_executor:
-            prev = self._async_executor.schedule('restart', True)
+            prev = self.schedule_restart(immediate=(not run_async))
             if prev is not None:
                 return (False, prev + ' already in progress')
-        if self._async_executor.run(self.state_handler.restart):
-            return (True, 'restarted successfully')
-        else:
-            return (False, 'restart failed')
+            if not run_async:
+                if self._async_executor.run(self.state_handler.restart):
+                    return (True, 'restarted successfully')
+                else:
+                    return (False, 'restart failed')
+            else:
+                self._async_executor.run_async(self.state_handler.restart)
+                return (True, "restart initiated")
 
     def reinitialize(self, cluster):
         self.state_handler.stop('immediate')
@@ -572,7 +578,9 @@ class Ha(object):
                 if self.cluster.is_unlocked():
                     return self.process_unhealthy_cluster()
                 else:
-                    self.evaluate_scheduled_restart()
+                    msg = self.evaluate_scheduled_restart()
+                    if msg is not None:
+                        return msg
                     return self.process_healthy_cluster()
             finally:
                 # we might not have a valid PostgreSQL connection here if another thread
diff --git a/tests/test_api.py b/tests/test_api.py
index 44e1662d..022b68e8 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -45,7 +45,7 @@ class MockHa(object):
         return (True, '')
 
     @staticmethod
-    def immediate_restart_scheduled():
+    def restart_scheduled():
         return False
 
     @staticmethod
@@ -120,7 +120,7 @@ class TestRestApiHandler(unittest.TestCase):
         MockPatroni.dcs.cluster = None
         with patch.object(RestApiHandler, 'get_postgresql_status', Mock(return_value={'role': 'master'})):
             MockRestApiServer(RestApiHandler, 'GET /master')
-        with patch.object(MockHa, 'immediate_restart_scheduled', Mock(return_value=True)):
+        with patch.object(MockHa, 'restart_scheduled', Mock(return_value=True)):
             MockRestApiServer(RestApiHandler, 'GET /master')
         self.assertIsNotNone(MockRestApiServer(RestApiHandler, 'GET /master'))
 
diff --git a/tests/test_ha.py b/tests/test_ha.py
index cd0cd4ae..a141cddb 100644
--- a/tests/test_ha.py
+++ b/tests/test_ha.py
@@ -288,7 +288,7 @@ class TestHa(unittest.TestCase):
 
     def test_restart_in_progress(self):
         self.ha._async_executor.schedule('restart', True)
-        self.assertTrue(self.ha.immediate_restart_scheduled())
+        self.assertTrue(self.ha.restart_scheduled())
         self.assertEquals(self.ha.run_cycle(), 'not healthy enough for leader race')
 
         self.ha.cluster = get_cluster_initialized_with_leader()
@@ -416,15 +416,20 @@ class TestHa(unittest.TestCase):
         with patch.object(self.ha,
                           'future_restart_scheduled', Mock(return_value={'postmaster_start_time': '2016-08-30 12:45TZ+1',
                                                                          'schedule': '2016-08-31 12:45TZ+1'})):
-            self.ha.evaluate_scheduled_restart()
+            self.assertIsNone(self.ha.evaluate_scheduled_restart())
         with patch.object(self.ha,
                           'future_restart_scheduled', Mock(return_value={'postmaster_start_time': '2016-08-31 12:45TZ+1',
                                                                          'schedule': '2016-08-31 12:45TZ+1'})):
             with patch.object(self.ha,
                               'should_run_scheduled_action', Mock(return_value=True)):
-                self.ha.evaluate_scheduled_restart()
+                self.assertIsNotNone(self.ha.evaluate_scheduled_restart())
                 with patch.object(self.ha, 'restart', Mock(return_value=(False, "Test"))):
-                    self.ha.evaluate_scheduled_restart()
+                    self.assertIsNone(self.ha.evaluate_scheduled_restart())
+
+    def test_scheduled_restart(self):
+        self.ha.cluster = get_cluster_initialized_with_leader()
+        with patch.object(self.ha, "evaluate_scheduled_restart", Mock(return_value="restart scheduled")):
+            self.assertEquals(self.ha.run_cycle(), "restart scheduled")
 
     def test_restart_matches(self):
         self.p._role = 'replica'

From ffd27b57052b6e075afcf45ac309fd6f7e14b7f0 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Wed, 13 Jul 2016 11:07:37 +0200
Subject: [PATCH 18/21] Rename with_pending_restart to restart_pending.

---
 features/patroni_api.feature | 2 +-
 patroni/api.py               | 2 +-
 patroni/ha.py                | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/features/patroni_api.feature b/features/patroni_api.feature
index 57810173..bf1e2fa9 100644
--- a/features/patroni_api.feature
+++ b/features/patroni_api.feature
@@ -86,7 +86,7 @@ Scenario: check the scheduled restart
 	Then I receive a response code 202
 		And I sleep for 10 seconds
 		And Response on GET http://127.0.0.1:8008/patroni contains pending_restart after 10 seconds
-	Given I issue a scheduled restart at http://127.0.0.1:8008 in 1 seconds with {"with_pending_restart_flag": "True"}
+	Given I issue a scheduled restart at http://127.0.0.1:8008 in 1 seconds with {"restart_pending": "True"}
 	Then I receive a response code 202
 		And Response on GET http://127.0.0.1:8008/patroni does not contain pending_restart after 10 seconds
 
diff --git a/patroni/api.py b/patroni/api.py
index d89ec409..756031f4 100644
--- a/patroni/api.py
+++ b/patroni/api.py
@@ -216,7 +216,7 @@ class RestApiHandler(BaseHTTPRequestHandler):
                     status_code = 400
                     data = "PostgreSQL version should be in the first.major.minor format"
                     break
-            elif k != 'with_pending_restart_flag':
+            elif k != 'restart_pending':
                 status_code = 400
                 data = "Unknown filter for the scheduled restart: {0}".format(k)
                 break
diff --git a/patroni/ha.py b/patroni/ha.py
index f0803708..64105067 100644
--- a/patroni/ha.py
+++ b/patroni/ha.py
@@ -464,7 +464,7 @@ class Ha(object):
         if (restart_data and isinstance(restart_data, dict) and
             not self.restart_matches(restart_data.get('role'),
                                      restart_data.get('postgres_version'),
-                                     ('with_pending_restart_flag' in restart_data))):
+                                     ('restart_pending' in restart_data))):
             return (False, "restart conditions are not satisfied")
 
         with self._async_executor:

From 6c9ffa4d3cab1ea1193520dbb21deeb8bd577dcf Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Thu, 14 Jul 2016 16:39:35 +0200
Subject: [PATCH 19/21] Address the code review

In particular, replace the fixed dates for the future actions
in the unit tests with those that depend on the current date,
avoiding the "timebomb" effect.
---
 patroni/ha.py    |  9 ++++-----
 tests/test_ha.py | 30 +++++++++++++++++++-----------
 2 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/patroni/ha.py b/patroni/ha.py
index 64105067..4db0ef3f 100644
--- a/patroni/ha.py
+++ b/patroni/ha.py
@@ -428,11 +428,10 @@ class Ha(object):
             return self._async_executor.schedule(action, immediate)
 
     def schedule_future_restart(self, restart_data):
-        if isinstance(restart_data, dict):
-            with self._async_executor:
-                if not self.patroni.scheduled_restart:
-                    self.patroni.scheduled_restart = restart_data
-                    return True
+        with self._async_executor:
+            if not self.patroni.scheduled_restart:
+                self.patroni.scheduled_restart = restart_data
+                return True
         return False
 
     def delete_future_restart(self):
diff --git a/tests/test_ha.py b/tests/test_ha.py
index a141cddb..6a788e5b 100644
--- a/tests/test_ha.py
+++ b/tests/test_ha.py
@@ -1,5 +1,4 @@
 import datetime
-import dateutil
 import etcd
 import os
 import pytz
@@ -47,6 +46,9 @@ def get_cluster_initialized_with_only_leader(failover=None):
     l = get_cluster_initialized_without_leader(leader=True, failover=failover).leader
     return get_cluster(True, l, [l], failover)
 
+future_restart_time = datetime.datetime.now(pytz.utc) + datetime.timedelta(days=5)
+postmaster_start_time = datetime.datetime.now(pytz.utc)
+
 
 class MockPatroni(object):
 
@@ -81,8 +83,8 @@ zookeeper:
         self.replicatefrom = None
         self.api.connection_string = 'http://127.0.0.1:8008'
         self.clonefrom = None
-        self.scheduled_restart = {'schedule': dateutil.parser.parse('2016-08-29 12:45TZ+1'),
-                                  'postmaster_start_time': '2016-08-20 12:00TZ+1'}
+        self.scheduled_restart = {'schedule': future_restart_time,
+                                  'postmaster_start_time': str(postmaster_start_time)}
 
 
 def run_async(func, args=()):
@@ -120,7 +122,7 @@ class TestHa(unittest.TestCase):
                                                 'hot_standby': 'on', 'max_wal_senders': 5, 'wal_keep_segments': 8}})
             self.p.set_state('running')
             self.p.set_role('replica')
-            self.p.postmaster_start_time = MagicMock(return_value="2016-08-20 12:00TZ+1")
+            self.p.postmaster_start_time = MagicMock(return_value=str(postmaster_start_time))
             self.p.check_replication_lag = true
             self.p.can_create_replica_without_replication_connection = MagicMock(return_value=False)
             self.e = get_dcs({'etcd': {'ttl': 30, 'host': 'ok:2379', 'scope': 'test',
@@ -405,25 +407,31 @@ class TestHa(unittest.TestCase):
     def test_schedule_future_restart(self):
         self.ha.patroni.scheduled_restart = {}
         # do the restart 2 times. The first one should succeed, the second one should fail
-        self.assertTrue(self.ha.schedule_future_restart({'schedule': '2016-08-30 12:45TZ+1"'}))
-        self.assertFalse(self.ha.schedule_future_restart({'schedule': '2016-08-30 12:45TZ+1"'}))
+        self.assertTrue(self.ha.schedule_future_restart({'schedule': str(future_restart_time)}))
+        self.assertFalse(self.ha.schedule_future_restart({'schedule': str(future_restart_time)}))
 
     def test_delete_future_restarts(self):
         self.ha.delete_future_restart()
 
     def test_evaluate_scheduled_restart(self):
-        self.p.postmaster_start_time = Mock(return_value='2016-08-31 12:45TZ+1')
+        self.p.postmaster_start_time = Mock(return_value=str(postmaster_start_time))
+        # restart while the postmaster has been already restarted, fails
         with patch.object(self.ha,
-                          'future_restart_scheduled', Mock(return_value={'postmaster_start_time': '2016-08-30 12:45TZ+1',
-                                                                         'schedule': '2016-08-31 12:45TZ+1'})):
+                          'future_restart_scheduled',
+                          Mock(return_value={'postmaster_start_time':
+                                             str(postmaster_start_time - datetime.timedelta(days=1)),
+                                             'schedule': str(future_restart_time)})):
             self.assertIsNone(self.ha.evaluate_scheduled_restart())
         with patch.object(self.ha,
-                          'future_restart_scheduled', Mock(return_value={'postmaster_start_time': '2016-08-31 12:45TZ+1',
-                                                                         'schedule': '2016-08-31 12:45TZ+1'})):
+                          'future_restart_scheduled',
+                          Mock(return_value={'postmaster_start_time': str(postmaster_start_time),
+                                             'schedule': str(future_restart_time)})):
             with patch.object(self.ha,
                               'should_run_scheduled_action', Mock(return_value=True)):
+                # restart in the future, ok
                 self.assertIsNotNone(self.ha.evaluate_scheduled_restart())
                 with patch.object(self.ha, 'restart', Mock(return_value=(False, "Test"))):
+                    # restart in the future, bit the actual restart failed
                     self.assertIsNone(self.ha.evaluate_scheduled_restart())
 
     def test_scheduled_restart(self):

From 13b4306f40a67d2a69e6cfc3e1fc8e35036a8954 Mon Sep 17 00:00:00 2001
From: Oleksii Kliukin <oleksii.kliukin@zalando.de>
Date: Thu, 14 Jul 2016 16:53:02 +0200
Subject: [PATCH 20/21] Remove one more occurrence of the time bomb

---
 tests/test_api.py | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/tests/test_api.py b/tests/test_api.py
index 022b68e8..51bdf9b3 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -1,9 +1,9 @@
-
+import datetime
 import json
 import psycopg2
+import pytz
 import unittest
 
-import dateutil.parser
 from mock import Mock, patch
 from patroni.api import RestApiHandler, RestApiServer
 from patroni.dcs import ClusterConfig, Member
@@ -12,6 +12,10 @@ from six.moves import BaseHTTPServer
 from test_postgresql import psycopg2_connect, MockCursor
 
 
+future_restart_time = datetime.datetime.now(pytz.utc) + datetime.timedelta(days=5)
+postmaster_start_time = datetime.datetime.now(pytz.utc)
+
+
 class MockPostgresql(object):
 
     name = 'test'
@@ -28,7 +32,7 @@ class MockPostgresql(object):
 
     @staticmethod
     def postmaster_start_time():
-        return '2016-08-20 12:00TZ+1'
+        return str(postmaster_start_time)
 
 
 class MockHa(object):
@@ -71,7 +75,7 @@ class MockPatroni(object):
     tags = {}
     version = '0.00'
     noloadbalance = Mock(return_value=False)
-    scheduled_restart = {'schedule': dateutil.parser.parse('2016-08-29 12:45TZ+1'),
+    scheduled_restart = {'schedule': future_restart_time,
                          'postmaster_start_time': postgresql.postmaster_start_time()}
 
     @staticmethod

From f7c44945b7d102244d358bfae893b0ee58f5528c Mon Sep 17 00:00:00 2001
From: Feike Steenbergen <feike.steenbergen@zalando.de>
Date: Mon, 18 Jul 2016 10:35:23 +0200
Subject: [PATCH 21/21] Fix > 9 PostgreSQL version numbering

---
 patroni/postgresql.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/patroni/postgresql.py b/patroni/postgresql.py
index 602cc5e5..d8bb1b60 100644
--- a/patroni/postgresql.py
+++ b/patroni/postgresql.py
@@ -981,7 +981,7 @@ $$""".format(name, ' '.join(options)), name, password, password)
         >>> Postgresql.postgres_version_to_int('9.3.13')
         90313
         >>> Postgresql.postgres_version_to_int('10.1')
-        100100
+        100001
         >>> Postgresql.postgres_version_to_int('10')
         Traceback (most recent call last):
             ...
@@ -997,8 +997,8 @@ $$""".format(name, ' '.join(options)), name, password, password)
         if len(components) < 2 or len(components) > 3:
             raise Exception("Invalid PostgreSQL format: X.Y or X.Y.Z is accepted: {0}".format(pg_version))
         if len(components) == 2:
-            # new style verion numbers, i.e. 10.1
-            components.append('0')
+            # new style verion numbers, i.e. 10.1 becomes 100001
+            components.insert(1, '0')
         try:
             result = [c if int(c) > 10 else '0{0}'.format(c) for c in components]
             result = int(''.join(result))