From 629c55234864f135c47d5a321ea222d9eb14f20d Mon Sep 17 00:00:00 2001
From: Ian Barwick <barwick@gmail.com>
Date: Fri, 15 Feb 2019 19:48:12 +0900
Subject: [PATCH] primary unregister: ensure correct behaviour when executed on
 a witness

Fixes GitHub #548.
---
 HISTORY                         |  2 ++
 doc/appendix-release-notes.sgml |  9 ++++++++-
 repmgr-action-primary.c         | 34 +++++++++++++++++++++++++++++----
 3 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/HISTORY b/HISTORY
index 1bf78664..7138b9e8 100644
--- a/HISTORY
+++ b/HISTORY
@@ -12,6 +12,8 @@
           data directory on the demotion candidate; GitHub #523 (Ian)
         repmgr: ensure "standby switchover" verifies replication connection
 		  exists; GitHub #519 (Ian)
+		repmgr: ensure "primary unregister" behaves correctly when executed
+		  on a witness server; GitHub #548 (Ian)
         repmgr: when executing "standby follow" and "node rejoin", check that
           it will actually be possible to stream from the target node (Ian)
         repmgr: "standby switchover": improve handling of connection URIs when
diff --git a/doc/appendix-release-notes.sgml b/doc/appendix-release-notes.sgml
index e675ddec..0114d95e 100644
--- a/doc/appendix-release-notes.sgml
+++ b/doc/appendix-release-notes.sgml
@@ -173,7 +173,7 @@ REPMGRD_OPTS="--daemonize=false"</programlisting>
           <listitem>
             <para>
               &repmgr;: when executing <command><link linkend="repmgr-witness-register">repmgr witness register</link></command>,
-              chech the node to connected is actually the primary (i.e. not the witness server).  (GitHub #528).
+              chech the node to connected is actually the primary (i.e. not the witness server).  GitHub #528.
             </para>
           </listitem>
 
@@ -207,6 +207,13 @@ REPMGRD_OPTS="--daemonize=false"</programlisting>
             </para>
 		  </listitem>
 
+          <listitem>
+            <para>
+              ensure <command><link linkend="repmgr-primary-unregister">repmgr primary unregister</link></command>
+			  behaves correctly when executed on a witness server. GitHub #548.
+            </para>
+		  </listitem>
+
         </itemizedlist>
       </para>
     </sect2>
diff --git a/repmgr-action-primary.c b/repmgr-action-primary.c
index 6e4d8ded..8176b03f 100644
--- a/repmgr-action-primary.c
+++ b/repmgr-action-primary.c
@@ -251,6 +251,7 @@ do_primary_unregister(void)
 	PGconn	   *primary_conn = NULL;
 	PGconn	   *local_conn = NULL;
 	t_node_info local_node_info = T_NODE_INFO_INITIALIZER;
+	t_node_info primary_node_info = T_NODE_INFO_INITIALIZER;
 
 	t_node_info *target_node_info_ptr = NULL;
 	PGconn	   *target_node_conn = NULL;
@@ -271,8 +272,6 @@ do_primary_unregister(void)
 
 	if (PQstatus(primary_conn) != CONNECTION_OK)
 	{
-		t_node_info primary_node_info = T_NODE_INFO_INITIALIZER;
-
 		log_error(_("unable to connect to primary server"));
 
 		if (get_primary_node_record(local_conn, &primary_node_info) == true)
@@ -291,10 +290,19 @@ do_primary_unregister(void)
 	/* Local connection no longer required */
 	PQfinish(local_conn);
 
+	if (get_primary_node_record(primary_conn, &primary_node_info) == false)
+	{
+		log_error(_("unable to retrieve record for primary node"));
+		PQfinish(primary_conn);
+		exit(ERR_BAD_CONFIG);
+	}
 
 	/* Target node is local node? */
-	if (target_node_info.node_id == UNKNOWN_NODE_ID
-		|| target_node_info.node_id == config_file_options.node_id)
+	if (target_node_info.node_id == UNKNOWN_NODE_ID)
+	{
+		target_node_info_ptr = &primary_node_info;
+	}
+	else if (target_node_info.node_id == config_file_options.node_id)
 	{
 		target_node_info_ptr = &local_node_info;
 	}
@@ -304,6 +312,24 @@ do_primary_unregister(void)
 		target_node_info_ptr = &target_node_info;
 	}
 
+	/*
+	 * Sanity-check the target node is not a witness
+	 */
+
+	if (target_node_info_ptr->type == WITNESS)
+	{
+		log_error(_("node %s (id: %i) is a witness server, unable to unregister"),
+					  target_node_info_ptr->node_name,
+					  target_node_info_ptr->node_id);
+		if (target_node_info_ptr->type == STANDBY)
+		{
+			log_hint(_("the node can be unregistered with \"repmgr witness unregister\""));
+		}
+
+		PQfinish(primary_conn);
+		exit(ERR_BAD_CONFIG);
+	}
+
 	/*
 	 * Check for downstream nodes - if any still defined, we won't be able to
 	 * delete the node record due to foreign key constraints.