Replace repmgr.conf item "upstream_node_id" with --upstream-node-id

This is only relevant when cloning a standby and the node's upstream
can change after failover/switchover etc., so no point keeping the
original value around in the configuration file.

config.c

@@ -205,7 +205,6 @@ _parse_config(t_configuration_options *options, ItemList *error_list, ItemList *
 
 	/* node information */
 	options->node_id = UNKNOWN_NODE_ID;
-	options->upstream_node_id = NO_UPSTREAM_NODE;
 	memset(options->node_name, 0, sizeof(options->node_name));
 	memset(options->conninfo, 0, sizeof(options->conninfo));
 	memset(options->pg_bindir, 0, sizeof(options->pg_bindir));
@@ -335,8 +334,6 @@ _parse_config(t_configuration_options *options, ItemList *error_list, ItemList *
 		}
 		else if (strcmp(name, "node_name") == 0)
 			strncpy(options->node_name, value, MAXLEN);
-		else if (strcmp(name, "upstream_node_id") == 0)
-			options->upstream_node_id = repmgr_atoi(value, name, error_list, 1);
 		else if (strcmp(name, "conninfo") == 0)
 			strncpy(options->conninfo, value, MAXLEN);
 		else if (strcmp(name, "replication_user") == 0)
@@ -486,7 +483,7 @@ _parse_config(t_configuration_options *options, ItemList *error_list, ItemList *
 		else if (strcmp(name, "upstream_node") == 0)
 		{
 			item_list_append(warning_list,
-							 _("parameter \"upstream_node\" has been renamed to \"upstream_node_id\""));
+							 _("parameter \"upstream_node\" has been removed; use \"--upstream-node-id\" when cloning a standby"));
 			known_parameter = false;
 		}
 		else if (strcmp(name, "loglevel") == 0)

config.h

@@ -51,7 +51,6 @@ typedef struct
 {
 	/* node information */
     int			node_id;
-	int			upstream_node_id;
 	char		node_name[MAXLEN];
 	char		conninfo[MAXLEN];
 	char		replication_user[MAXLEN];
@@ -118,7 +117,7 @@ typedef struct
 
 #define T_CONFIGURATION_OPTIONS_INITIALIZER { \
 		/* node information */ \
-		UNKNOWN_NODE_ID, NO_UPSTREAM_NODE, "", "", "", "", REPLICATION_TYPE_PHYSICAL, \
+		UNKNOWN_NODE_ID, "", "", "", "", REPLICATION_TYPE_PHYSICAL, \
 		/* log settings */ \
 		"", "", "", DEFAULT_LOG_STATUS_INTERVAL,	\
 		/* standby clone settings */ \

doc/changes-in-repmgr4.md

@@ -5,10 +5,16 @@ New command line options
 - `--dry-run`: repmgr will attempt to perform the action as far as possible
    without making any changes to the database
 
+- `--upstream-node-id`: use to specify the upstream node the standby will
+  connect later stream from, when cloning a standby. This replaces the configuration
+  file parameter `upstream_node`, as the upstream node is set when the standby
+  is initially cloned, but can change over the lifetime of an installation (due
+  to failovers, switchovers etc.) so it's pointless/confusing keeping the original
+  value around in the config file.
+
 Changed command line options
 ----------------------------
 
-
 ### repmgr
 
 - `--replication-user` is now passed when registering the master server (and

repmgr-action-primary.c

@@ -139,15 +139,6 @@ do_primary_register(void)
 		node_info.node_id = config_file_options.node_id;
 	}
 
-	/* if upstream_node_id set, warn that it will be ignored */
-	if (config_file_options.upstream_node_id != NO_UPSTREAM_NODE)
-	{
-		log_warning(_("primary node %i is configured with \"upstream_node_id\" set to %i"),
-					node_info.node_id,
-					config_file_options.upstream_node_id);
-		log_detail(_("the value set for \"upstream_node_id\" will be ignored"));
-	}
-
 	/* set type to "primary", active to "true" and unset upstream_node_id*/
 	node_info.type = PRIMARY;
 	node_info.upstream_node_id = NO_UPSTREAM_NODE;

repmgr-action-standby.c

@@ -680,12 +680,12 @@ do_standby_register(void)
 	 * If it doesn't exist, and --force set, create a minimal inactive record
 	 */
 
-	if (config_file_options.upstream_node_id != NO_UPSTREAM_NODE)
+	if (runtime_options.upstream_node_id != NO_UPSTREAM_NODE)
 	{
 		RecordStatus upstream_record_status;
 
 		upstream_record_status = get_node_record(primary_conn,
-												 config_file_options.upstream_node_id,
+												 runtime_options.upstream_node_id,
 												 &node_record);
 
 		if (upstream_record_status != RECORD_FOUND)
@@ -695,7 +695,7 @@ do_standby_register(void)
 			if (!runtime_options.force)
 			{
 				log_error(_("no record found for upstream node %i"),
-						  config_file_options.upstream_node_id);
+						  runtime_options.upstream_node_id);
 				/* footgun alert - only do this if you know what you're doing */
 				log_hint(_("use option -F/--force to create a dummy upstream record"));
 				PQfinish(primary_conn);
@@ -705,9 +705,9 @@ do_standby_register(void)
 			}
 
 			log_notice(_("creating placeholder record for upstream node %i"),
-					   config_file_options.upstream_node_id);
+					   runtime_options.upstream_node_id);
 
-			upstream_node_record.node_id = config_file_options.upstream_node_id;
+			upstream_node_record.node_id = runtime_options.upstream_node_id;
 			upstream_node_record.type = STANDBY;
 			upstream_node_record.upstream_node_id = NO_UPSTREAM_NODE;
 			strncpy(upstream_node_record.conninfo, runtime_options.upstream_conninfo, MAXLEN);
@@ -730,12 +730,12 @@ do_standby_register(void)
 			if (record_created == false)
 			{
 				upstream_record_status = get_node_record(primary_conn,
-														 config_file_options.upstream_node_id,
+														 runtime_options.upstream_node_id,
 														 &node_record);
 				if (upstream_record_status != RECORD_FOUND)
 				{
 					log_error(_("unable to create placeholder record for upstream node %i"),
-							  config_file_options.upstream_node_id);
+							  runtime_options.upstream_node_id);
 					PQfinish(primary_conn);
 					if (PQstatus(conn) == CONNECTION_OK)
 						PQfinish(conn);
@@ -743,7 +743,7 @@ do_standby_register(void)
 				}
 
 				log_info(_("a record for upstream node %i was already created"),
-						 config_file_options.upstream_node_id);
+						 runtime_options.upstream_node_id);
 			}
 
 		}
@@ -755,7 +755,7 @@ do_standby_register(void)
 			if (!runtime_options.force)
 			{
 				log_error(_("record for upstream node %i is marked as inactive"),
-						  config_file_options.upstream_node_id);
+						  runtime_options.upstream_node_id);
 				log_hint(_("use option -F/--force to register a standby with an inactive upstream node"));
 				PQfinish(primary_conn);
 				if (PQstatus(conn) == CONNECTION_OK)
@@ -768,7 +768,7 @@ do_standby_register(void)
 			 */
 			log_notice(_("registering node %i with inactive upstream node %i"),
 					   config_file_options.node_id,
-					   config_file_options.upstream_node_id);
+					   runtime_options.upstream_node_id);
 		}
 	}
 
@@ -777,7 +777,7 @@ do_standby_register(void)
 
 	node_record.node_id = config_file_options.node_id;
 	node_record.type = STANDBY;
-	node_record.upstream_node_id = config_file_options.upstream_node_id;
+	node_record.upstream_node_id = runtime_options.upstream_node_id;
 	node_record.priority = config_file_options.priority;
 	node_record.active = true;
 
@@ -1736,10 +1736,10 @@ check_source_server()
 	/*
 	 * Attempt to find the upstream node record
 	 */
-	if (config_file_options.upstream_node_id == NO_UPSTREAM_NODE)
+	if (runtime_options.upstream_node_id == NO_UPSTREAM_NODE)
 		upstream_node_id = get_primary_node_id(source_conn);
 	else
-		upstream_node_id = config_file_options.upstream_node_id;
+		upstream_node_id = runtime_options.upstream_node_id;
 
 	record_status = get_node_record(source_conn, upstream_node_id, &node_record);
 

repmgr-client-global.h

@@ -61,6 +61,7 @@ typedef struct
 	char		recovery_min_apply_delay[MAXLEN];
 	char		replication_user[MAXLEN];
 	char		upstream_conninfo[MAXLEN];
+	int			upstream_node_id;
 	bool		use_recovery_conninfo_password;
 	char		wal_keep_segments[MAXLEN];
 	bool		without_barman;
@@ -90,7 +91,7 @@ typedef struct
 		/* node options */ \
 		UNKNOWN_NODE_ID, "", "", \
 		/* standby clone options */ \
-		false, CONFIG_FILE_SAMEPATH, false, false, false, "", "", "", false, "", false, \
+		false, CONFIG_FILE_SAMEPATH, false, false, false, "", "", "", NO_UPSTREAM_NODE, false, "", false, \
 		/* standby register options */ \
 		false, 0, \
 		/* event options */ \

repmgr-client.c

@@ -354,6 +354,10 @@ main(int argc, char **argv)
 				strncpy(runtime_options.upstream_conninfo, optarg, MAXLEN);
 				break;
 
+			case OPT_UPSTREAM_NODE_ID:
+				//strncpy(runtime_options.upstream_conninfo, optarg, MAXLEN);
+				break;
+
 			case OPT_USE_RECOVERY_CONNINFO_PASSWORD:
 				runtime_options.use_recovery_conninfo_password = true;
 				break;

repmgr-client.h

@@ -58,6 +58,7 @@
 #define OPT_LIMIT						   21
 #define OPT_ALL							   22
 #define OPT_DRY_RUN						   23
+#define OPT_UPSTREAM_NODE_ID			   24
 /* deprecated since 3.3 */
 #define OPT_DATA_DIR					   998
 #define OPT_NO_CONNINFO_PASSWORD		   999
@@ -103,6 +104,7 @@ static struct option long_options[] =
 	{"recovery-min-apply-delay", required_argument, NULL, OPT_RECOVERY_MIN_APPLY_DELAY},
 	{"replication-user", required_argument, NULL, OPT_REPLICATION_USER},
 	{"upstream-conninfo", required_argument, NULL, OPT_UPSTREAM_CONNINFO},
+	{"upstream-node-id", required_argument, NULL, OPT_UPSTREAM_NODE_ID},
 	{"use-recovery-conninfo-password", no_argument, NULL, OPT_USE_RECOVERY_CONNINFO_PASSWORD},
 	{"without-barman", no_argument, NULL, OPT_WITHOUT_BARMAN},