From b6264b77c4e1c13b9dd0bea19228c102fdc1878b Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Wed, 30 Jan 2019 10:57:06 +0900 Subject: [PATCH] repmgr: mandate explicit configuration for "daemon (start|stop)" The initial implementation was designed to fall back to "manual" start/stop of repmgrd if the "repmgrd_service_..._command" parameters were not set. However on reflection, this is too much of a potential footgun, so we will mandate provision of these parameters. --- doc/repmgr-daemon-start.sgml | 24 +++--- doc/repmgr-daemon-stop.sgml | 24 +++--- doc/repmgrd-configuration.sgml | 14 ++++ repmgr-action-daemon.c | 133 +++++++-------------------------- 4 files changed, 63 insertions(+), 132 deletions(-) diff --git a/doc/repmgr-daemon-start.sgml b/doc/repmgr-daemon-start.sgml index f10df9fc..0d52e426 100644 --- a/doc/repmgr-daemon-start.sgml +++ b/doc/repmgr-daemon-start.sgml @@ -23,13 +23,7 @@ This command starts the repmgrd daemon on the local node. - - - If &repmgr; was installed from packages, it is essential - that repmgrd is configured to use the - package-provided service start/stop commands. See below for details. - - + @@ -41,10 +35,6 @@ if &repmgr; was installed from a package, this will be the service command defined by the package. For more details see Appendix: &repmgr; package details. - - If repmgrd_service_start_command is not set, - &repmgr; will default to executing repmgrd directly. - @@ -94,6 +84,16 @@ + + + + + repmgrd_service_start_command is not defined in + repmgr.conf. + + + + @@ -109,7 +109,7 @@ See also - + , diff --git a/doc/repmgr-daemon-stop.sgml b/doc/repmgr-daemon-stop.sgml index 197fbb87..a57f4cf1 100644 --- a/doc/repmgr-daemon-stop.sgml +++ b/doc/repmgr-daemon-stop.sgml @@ -23,13 +23,7 @@ This command stops the repmgrd daemon on the local node. - - - If &repmgr; was installed from packages, it is essential - that repmgrd is configured to use the - package-provided service start/stop commands. See below for details. - - + @@ -41,10 +35,6 @@ if &repmgr; was installed from a package, this will be the service command defined by the package. For more details see Appendix: &repmgr; package details. - - If repmgrd_service_stop_command is not set, - &repmgr; will default to executing repmgrd directly. - @@ -94,6 +84,16 @@ + + + + + repmgrd_service_stop_command is not defined in + repmgr.conf. + + + + @@ -109,7 +109,7 @@ See also - + , diff --git a/doc/repmgrd-configuration.sgml b/doc/repmgrd-configuration.sgml index 34035d8a..5dba56dd 100644 --- a/doc/repmgrd-configuration.sgml +++ b/doc/repmgrd-configuration.sgml @@ -363,6 +363,20 @@ See appendix for details of service commands for different distributions. + + The commands repmgr daemon start and + repmgr daemon stop can be used + as convenience wrappers to start and stop repmgrd. + + + + repmgr daemon start and + repmgr daemon stop require + that the appropriate start/stop commands are configured as + repmgrd_service_start_command and repmgrd_service_stop_command + in repmgr.conf. + + repmgrd can be started manually like this: diff --git a/repmgr-action-daemon.c b/repmgr-action-daemon.c index 73c9b449..18bfd4d8 100644 --- a/repmgr-action-daemon.c +++ b/repmgr-action-daemon.c @@ -394,16 +394,16 @@ do_daemon_start(void) PQExpBufferData output_buf; bool success; - /* - * if local connection available, check if repmgr.so is installed, and - * whether repmgrd is running - */ + if (config_file_options.repmgrd_service_start_command[0] == '\0') + { + log_error(_("\"repmgrd_service_start_command\" is not set")); + log_hint(_("set \"repmgrd_service_start_command\" in \"repmgr.conf\"")); + exit(ERR_BAD_CONFIG); + } + log_verbose(LOG_INFO, _("connecting to local node")); - if (strlen(config_file_options.conninfo)) - conn = establish_db_connection(config_file_options.conninfo, false); - else - conn = establish_db_connection_by_params(&source_conninfo, false); + conn = establish_db_connection(config_file_options.conninfo, false); if (PQstatus(conn) != CONNECTION_OK) { @@ -413,6 +413,10 @@ do_daemon_start(void) exit(ERR_REPMGRD_SERVICE); } + /* + * if local connection available, check if repmgr.so is installed, and + * whether repmgrd is running + */ check_shared_library(conn); if (is_repmgrd_running(conn) == true) @@ -424,25 +428,10 @@ do_daemon_start(void) PQfinish(conn); + initPQExpBuffer(&repmgrd_command); - - if (config_file_options.repmgrd_service_start_command[0] != '\0') - { - appendPQExpBufferStr(&repmgrd_command, - config_file_options.repmgrd_service_start_command); - } - else - { - /* - * repmgr will attempt to construct appropriate commands, but - * usually it's preferable for them to be explicitly defined, - * particularly if repmgr is installed from packages. - */ - log_warning(_("\"repmgrd_service_start_command\" is not set")); - log_hint(_("specify appropriate repmgrd start and stop commands in \"repmgr.conf\" for reliable operation")); - - make_repmgrd_path(&repmgrd_command); - } + appendPQExpBufferStr(&repmgrd_command, + config_file_options.repmgrd_service_start_command); if (runtime_options.dry_run == true) { @@ -479,16 +468,20 @@ void do_daemon_stop(void) bool success; pid_t pid = UNKNOWN_PID; + if (config_file_options.repmgrd_service_start_command[0] == '\0') + { + log_error(_("\"repmgrd_service_stop_command\" is not set")); + log_hint(_("set \"repmgrd_service_stop_command\" in \"repmgr.conf\"")); + exit(ERR_BAD_CONFIG); + } + /* * if local connection available, check if repmgr.so is installed, and * whether repmgrd is running */ log_verbose(LOG_INFO, _("connecting to local node")); - if (strlen(config_file_options.conninfo)) - conn = establish_db_connection(config_file_options.conninfo, false); - else - conn = establish_db_connection_by_params(&source_conninfo, false); + conn = establish_db_connection(config_file_options.conninfo, false); if (PQstatus(conn) != CONNECTION_OK) { @@ -514,84 +507,8 @@ void do_daemon_stop(void) initPQExpBuffer(&repmgrd_command); - if (config_file_options.repmgrd_service_start_command[0] != '\0') - { - appendPQExpBufferStr(&repmgrd_command, - config_file_options.repmgrd_service_stop_command); - } - else - { - log_warning(_("\"repmgrd_service_stop_command\" is not set")); - log_hint(_("specify appropriate repmgrd start and stop commands in \"repmgr.conf\" for reliable operation")); - - /* PID not known - attempt to retrieve repmgrd default PID */ - if (pid == UNKNOWN_PID) - { - PQExpBufferData repmgrd_pid_command; - char pidfile[MAXPGPATH] = ""; - int pidfile_pathlen; - struct stat stat_pidfile; - - initPQExpBuffer(&repmgrd_pid_command); - - make_repmgrd_path(&repmgrd_pid_command); - appendPQExpBufferStr(&repmgrd_pid_command, " --show-pid-file 2>/dev/null"); - - initPQExpBuffer(&output_buf); - log_debug("%s", repmgrd_pid_command.data); - success = local_command(repmgrd_pid_command.data, &output_buf); - termPQExpBuffer(&repmgrd_pid_command); - - if (success == false) - { - log_error(_("unable to execute \"repmgrd --show-pid-file\"")); - - if (output_buf.data[0] != '\0') - log_detail("%s", output_buf.data); - - termPQExpBuffer(&output_buf); - exit(ERR_REPMGRD_SERVICE); - } - - if (output_buf.data[0] == '\0') - { - log_error(_("\"repmgrd --show-pid-file\" did not return a file")); - termPQExpBuffer(&output_buf); - exit(ERR_REPMGRD_SERVICE); - } - - pidfile_pathlen = strlen(output_buf.data); - - if (pidfile_pathlen > MAXPGPATH) - pidfile_pathlen = MAXPGPATH; - else - pidfile_pathlen --; - - strncpy(pidfile, output_buf.data, pidfile_pathlen); - - /* check if pid file actually exists */ - - if (stat(pidfile, &stat_pidfile) != 0) - { - log_error(_("PID file \"%s\" not found"), - pidfile); - log_detail("%s", strerror(errno)); - - if (config_file_options.repmgrd_pid_file[0] == '\0') - log_hint(_("set \"repmgrd_pid_file\" in \"%s\""), config_file_path); - exit(ERR_REPMGRD_SERVICE); - } - - appendPQExpBuffer(&repmgrd_command, - "kill `cat %s`", pidfile); - termPQExpBuffer(&output_buf); - } - else - { - appendPQExpBuffer(&repmgrd_command, - "kill %i", pid); - } - } + appendPQExpBufferStr(&repmgrd_command, + config_file_options.repmgrd_service_stop_command); if (runtime_options.dry_run == true) {