Merge pull request #3665 from opensourcerouting/6.0/watchfrr-options

[6.0] fix watchfrr options

configure.ac

@@ -1974,6 +1974,7 @@ AC_SUBST(CFG_SBIN)
 AC_SUBST(CFG_STATE)
 AC_SUBST(CFG_MODULE)
 AC_DEFINE_UNQUOTED(MODULE_PATH, "$CFG_MODULE", path to modules)
+AC_DEFINE_UNQUOTED(WATCHFRR_SH_PATH, "${CFG_SBIN%/}/watchfrr.sh", path to watchfrr.sh)
 
 dnl ------------------------------------
 dnl Enable RPKI and add librtr to libs

doc/manpages/watchfrr.rst

@@ -22,21 +22,6 @@ In order to avoid restarting the daemons in quick succession, you can supply the
 
 OPTIONS
 =======
-The following 3 options specify scripts that |DAEMON| uses to perform start/stop/restart actions. These options are mandatory unless the --dry option is used:
-
-.. option:: -s command, --start-command command
-
-  Supply a Bourne shell command to start a single daemon. The command string should contain the '%s' placeholder to be sub‐ stituted with the daemon name.
-
-.. option:: -k command, --kill-command command
-
-   Supply a Bourne shell command to stop a single daemon. The command string should contain the '%s' placeholder to be substituted with the daemon name.
-
-.. option:: -r command, --restart command
-
-   Supply a Bourne shell command to restart a single daemon. The command string should contain the '%s' placeholder to be substituted with the daemon name.
-
-Other options:
 
 .. option:: --dry
 
@@ -92,6 +77,20 @@ Other options:
 
    Display the usage information and exit.
 
+The following 3 options specify scripts that |DAEMON| uses to perform start/stop/restart actions. Reasonable default values are built into watchfrr, so the use of these options should no longer be necessary:
+
+.. option:: -s command, --start-command command
+
+  Supply a Bourne shell command to start a single daemon. The command string should contain the '%s' placeholder to be substituted with the daemon name.
+
+.. option:: -k command, --kill-command command
+
+   Supply a Bourne shell command to stop a single daemon. The command string should contain the '%s' placeholder to be substituted with the daemon name.
+
+.. option:: -r command, --restart command
+
+   Supply a Bourne shell command to restart a single daemon. The command string should contain the '%s' placeholder to be substituted with the daemon name.
+
 PREVIOUS OPTIONS
 ================
 Prior versions of |DAEMON| supported some additional options that no longer exist:::

doc/user/setup.rst

@@ -70,13 +70,14 @@ This file has several parts. Here is an example:
    bfdd_options="  --daemon -A 127.0.0.1"
 
    # The list of daemons to watch is automatically generated by the init script.
-   watchfrr_enable=yes
-   watchfrr_options=(-d -r /usr/sbin/servicebBfrrbBrestartbB%s -s /usr/sbin/servicebBfrrbBstartbB%s -k /usr/sbin/servicebBfrrbBstopbB%s -b bB)
+   #watchfrr_options=""
 
-   # If valgrind_enable is 'yes' the frr daemons will be started via valgrind.
-   # The use case for doing so is tracking down memory leaks, etc in frr.
-   valgrind_enable=no
-   valgrind=/usr/bin/valgrind
+   # for debugging purposes, you can specify a "wrap" command to start instead
+   # of starting the daemon directly, e.g. to use valgrind on ospfd:
+   #   ospfd_wrap="/usr/bin/valgrind"
+   # or you can use "all_wrap" for all daemons, e.g. to use perf record:
+   #   all_wrap="/usr/bin/perf record --call-graph -"
+   # the normal daemon command is added to this at the end.
 
 Breaking this file down:
 
@@ -99,22 +100,8 @@ from the service script. Usually daemons will have ``--daemon`` and ``-A
 <address>`` specified in order to daemonize and listen for VTY commands on a
 particular address.
 
-::
-
-   # The list of daemons to watch is automatically generated by the init script.
-   watchfrr_enable=yes
-   watchfrr_options=(-d -r /usr/sbin/servicebBfrrbBrestartbB%s -s /usr/sbin/servicebBfrrbBstartbB%s -k /usr/sbin/servicebBfrrbBstopbB%s -b bB)
-
-Options for the ``watchfrr``, the watchdog daemon.
-
-::
-
-   valgrind_enable=no
-   valgrind=/usr/bin/valgrind
-
-Whether or not to start FRR daemons under Valgrind. This is primarily useful
-for gathering information for bug reports and for developers.
-``valgrind_enable`` should be ``no`` for production use.
+The remaining file content regarding `watchfrr_options` and `*_wrap` settings
+should not normally be needed;  refer to the comments in case they are.
 
 Services
 --------

tools/etc/frr/daemons

@@ -53,7 +53,7 @@ staticd_options="-A 127.0.0.1"
 bfdd_options="   -A 127.0.0.1"
 
 # The list of daemons to watch is automatically generated by the init script.
-watchfrr_options="-r '/usr/lib/frr/watchfrr.sh restart %s' -s '/usr/lib/frr/watchfrr.sh start %s' -k '/usr/lib/frr/watchfrr.sh stop %s'"
+#watchfrr_options=""
 
 # for debugging purposes, you can specify a "wrap" command to start instead
 # of starting the daemon directly, e.g. to use valgrind on ospfd:

tools/frrcommon.sh.in

@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 #
 # This is a "library" of sorts for use by the other FRR shell scripts.  It
 # has most of the daemon start/stop logic, but expects the following shell
@@ -288,6 +288,13 @@ load_old_config "$C_PATH/daemons.conf"
 load_old_config "/etc/default/frr"
 load_old_config "/etc/sysconfig/frr"
 
+if declare -p watchfrr_options | grep -q '^declare \-a'; then
+	log_warning_msg "watchfrr_options contains a bash array value." \
+		"The configured value is intentionally ignored since it is likely wrong." \
+		"Please remove or fix the setting."
+	unset watchfrr_options
+fi
+
 #
 # other defaults and dispatch
 #

tools/frrinit.sh.in

@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 #
 ### BEGIN INIT INFO
 # Provides: frr

tools/watchfrr.sh.in

@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 #
 # This is NOT the init script!  This is the watchfrr start/stop/restart
 # command handler, passed to watchfrr with the -s/-r/-k commands.  It is used

watchfrr/watchfrr.c

@@ -53,6 +53,10 @@
 #define DEFAULT_MIN_RESTART	60
 #define DEFAULT_MAX_RESTART	600
 
+#define DEFAULT_RESTART_CMD	WATCHFRR_SH_PATH " restart %s"
+#define DEFAULT_START_CMD	WATCHFRR_SH_PATH " start %s"
+#define DEFAULT_STOP_CMD	WATCHFRR_SH_PATH " stop %s"
+
 #define PING_TOKEN	"PING"
 
 DEFINE_MGROUP(WATCHFRR, "watchfrr")
@@ -124,6 +128,9 @@ static struct global_state {
 	.loglevel = DEFAULT_LOGLEVEL,
 	.min_restart_interval = DEFAULT_MIN_RESTART,
 	.max_restart_interval = DEFAULT_MAX_RESTART,
+	.restart_command = DEFAULT_RESTART_CMD,
+	.start_command = DEFAULT_START_CMD,
+	.stop_command = DEFAULT_STOP_CMD,
 };
 
 typedef enum {
@@ -227,14 +234,17 @@ Otherwise, the interval is doubled (but capped at the -M value).\n\n",
 -r, --restart	Supply a Bourne shell command to use to restart a single\n\
 		daemon.  The command string should include '%%s' where the\n\
 		name of the daemon should be substituted.\n\
+		(default: '%s')\n\
 -s, --start-command\n\
 		Supply a Bourne shell to command to use to start a single\n\
 		daemon.  The command string should include '%%s' where the\n\
 		name of the daemon should be substituted.\n\
+		(default: '%s')\n\
 -k, --kill-command\n\
 		Supply a Bourne shell to command to use to stop a single\n\
 		daemon.  The command string should include '%%s' where the\n\
 		name of the daemon should be substituted.\n\
+		(default: '%s')\n\
     --dry	Do not start or restart anything, just log.\n\
 -p, --pid-file	Set process identifier file name\n\
 		(default is %s/watchfrr.pid).\n\
@@ -247,7 +257,9 @@ Otherwise, the interval is doubled (but capped at the -M value).\n\n",
 -h, --help	Display this help and exit\n",
 		frr_vtydir, DEFAULT_LOGLEVEL, LOG_EMERG, LOG_DEBUG, LOG_DEBUG,
 		DEFAULT_MIN_RESTART, DEFAULT_MAX_RESTART, DEFAULT_PERIOD,
-		DEFAULT_TIMEOUT, DEFAULT_RESTART_TIMEOUT, frr_vtydir);
+		DEFAULT_TIMEOUT, DEFAULT_RESTART_TIMEOUT,
+		DEFAULT_RESTART_CMD, DEFAULT_START_CMD, DEFAULT_STOP_CMD,
+		frr_vtydir);
 }
 
 static pid_t run_background(char *shell_cmd)