Add -t config-test option

pqarmitage · pqarmitage · commit 96be59387bcb · 2018-06-17T00:43:44.000+01:00
Issue #389 has received increasing support to add a configuration validation option. This commit adds the -t/--commit-test option to report any detected configururation errors and exit. Errors are logged to the system log by default, but use of the -g and -G options can make the errors be logged to files. Signed-off-by: Quentin Armitage <quentin@armitage.org.uk>
diff --git a/TODO b/TODO
@@ -34,6 +34,9 @@ development:
 	* Allow dynamic definitions, e.g. $_VI_NAME
 	    Also $*_INSTANCE net_namspace $_INSTANCE
 	    so $* means only do if $_INSTANCE not blank. */
+	* ng-scheduler - would libev help, or use select with epoll to get
+	    micro-second timer
+
 
 
 Other issues awaiting resolution:
diff --git a/doc/keepalived.conf.SYNOPSIS b/doc/keepalived.conf.SYNOPSIS
@@ -1143,7 +1143,7 @@ virtual_server group <STRING>      {          # VS group declaration
               path <STRING>                   # Path
               digest <STRING>                 # Digest computed with genhash
               status_code <INTEGER>           # status code returned into the HTTP
-                                              #   header. I not specified, then any
+                                              #   header. If not specified, then any
                                               #   2xx code is accepted.
               virtualhost <STRING>            # VirtualHost string to use. If not set
                                               #  uses virtualhost from checker or real
diff --git a/keepalived/bfd/bfd_daemon.c b/keepalived/bfd/bfd_daemon.c
@@ -74,7 +74,6 @@ stop_bfd(int status)
 	thread_destroy_master(master);
 	free_parent_mallocs_exit();
 
-
 	/*
 	 * Reached when terminate signal catched.
 	 * finally return to parent process.
@@ -142,6 +141,12 @@ start_bfd(void)
 		init_global_data();
 	*/
 
+	/* If we are just testing the configuration, then we terminate now */
+	if (__test_bit(CONFIG_TEST_BIT, &debug)) {
+		stop_bfd(KEEPALIVED_EXIT_CONFIG_TEST);
+		return;
+	}
+
 	/* Set the process priority and non swappable if configured */
 // TODO - measure max stack usage
 	set_process_priorities(
@@ -247,7 +252,9 @@ bfd_respawn_thread(thread_t * thread)
 	}
 
 	/* We catch a SIGCHLD, handle it */
-	if (!__test_bit(DONT_RESPAWN_BIT, &debug)) {
+	if (__test_bit(CONFIG_TEST_BIT, &debug))
+		raise(SIGTERM);
+	else if (!__test_bit(DONT_RESPAWN_BIT, &debug)) {
 		log_message(LOG_ALERT, "BFD child process(%d) died: Respawning",
 			    pid);
 		start_bfd_child();
@@ -291,7 +298,7 @@ start_bfd_child(void)
 
 	/* Clear any child finder functions set in parent */
 	set_child_finder_name(NULL);
-	set_child_finder(NULL, NULL, NULL, NULL, NULL, 0);	/* Currently these won't be set */
+	destroy_child_finder();
 
 	prog_type = PROG_TYPE_BFD;
 
diff --git a/keepalived/check/check_daemon.c b/keepalived/check/check_daemon.c
@@ -153,7 +153,8 @@ checker_terminate_phase1(bool schedule_next_thread)
 		script_killall(master, SIGTERM, true);
 
 	/* Send shutdown messages */
-	if (!__test_bit(DONT_RELEASE_IPVS_BIT, &debug))
+	if (!__test_bit(DONT_RELEASE_IPVS_BIT, &debug) &&
+	    !__test_bit(CONFIG_TEST_BIT, &debug))
 		clear_services();
 
 	if (schedule_next_thread) {
@@ -227,6 +228,12 @@ start_check(list old_checkers_queue)
 	log_message(LOG_INFO, "Configuration is using : %zu Bytes", mem_allocated);
 #endif
 
+	/* If we are just testing the configuration, then we terminate now */
+	if (__test_bit(CONFIG_TEST_BIT, &debug)) {
+		stop_check(KEEPALIVED_EXIT_CONFIG_TEST);
+		return;
+	}
+
 	/* Initialize sub-system if any virtual servers are configured */
 	if ((!LIST_ISEMPTY(check_data->vs) || (reload && !LIST_ISEMPTY(old_check_data->vs))) &&
 	    ipvs_start() != IPVS_SUCCESS) {
@@ -384,7 +391,9 @@ check_respawn_thread(thread_t * thread)
 	}
 
 	/* We catch a SIGCHLD, handle it */
-	if (!__test_bit(DONT_RESPAWN_BIT, &debug)) {
+	if (!__test_bit(CONFIG_TEST_BIT, &debug))
+		raise(SIGTERM);
+	else if (!__test_bit(DONT_RESPAWN_BIT, &debug)) {
 		log_message(LOG_ALERT, "Healthcheck child process(%d) died: Respawning", pid);
 		start_check_child();
 	} else {
@@ -429,7 +438,7 @@ start_check_child(void)
 
 	/* Clear any child finder functions set in parent */
 	set_child_finder_name(NULL);
-	set_child_finder(NULL, NULL, NULL, NULL, NULL, 0);	/* Currently these won't be set */
+	destroy_child_finder();
 
 	prog_type = PROG_TYPE_CHECKER;
 
diff --git a/keepalived/core/main.c b/keepalived/core/main.c
@@ -305,6 +305,41 @@ make_pidfile_name(const char* start, const char* instance, const char* extn)
 	return name;
 }
 
+#ifndef _DEBUG_
+static void
+parent_child_remover(thread_t *thread)
+{
+
+        if (prog_type == PROG_TYPE_PARENT) {
+#ifdef _WITH_VRRP_
+                if (thread->u.c.pid == vrrp_child)
+                        vrrp_child = 0;
+#endif
+#ifdef _WITH_LVS_
+                if (thread->u.c.pid == checkers_child)
+                        checkers_child = 0;
+#endif
+#ifdef _WITH_BFD_
+                if (thread->u.c.pid == bfd_child)
+                        bfd_child = 0;
+#endif
+
+		if (__test_bit(CONFIG_TEST_BIT, &debug)) {
+#ifdef _WITH_VRRP_
+			if (vrrp_child == 0)
+#endif
+#ifdef _WITH_LVS_
+			if (checkers_child == 0)
+#endif
+#ifdef _WITH_BFD_
+			if (bfd_child == 0)
+#endif
+				raise(SIGTERM);
+		}
+	}
+}
+#endif
+
 #ifdef _WITH_VRRP_
 bool
 running_vrrp(void)
@@ -559,7 +594,7 @@ sigend(__attribute__((unused)) void *v, __attribute__((unused)) int sig)
 
 #ifdef _WITH_VRRP_
 	if (vrrp_child > 0) {
-		if (kill(vrrp_child, SIGTERM)) {
+		if (!__test_bit(CONFIG_TEST_BIT, &debug) && kill(vrrp_child, SIGTERM)) {
 			/* ESRCH means no such process */
 			if (errno == ESRCH)
 				vrrp_child = 0;
@@ -570,7 +605,7 @@ sigend(__attribute__((unused)) void *v, __attribute__((unused)) int sig)
 #endif
 #ifdef _WITH_LVS_
 	if (checkers_child > 0) {
-		if (kill(checkers_child, SIGTERM)) {
+		if (!__test_bit(CONFIG_TEST_BIT, &debug) && kill(checkers_child, SIGTERM)) {
 			if (errno == ESRCH)
 				checkers_child = 0;
 		}
@@ -580,7 +615,7 @@ sigend(__attribute__((unused)) void *v, __attribute__((unused)) int sig)
 #endif
 #ifdef _WITH_BFD_
 	if (bfd_child > 0) {
-		if (kill(bfd_child, SIGTERM)) {
+		if (!__test_bit(CONFIG_TEST_BIT, &debug) && kill(bfd_child, SIGTERM)) {
 			if (errno == ESRCH)
 				bfd_child = 0;
 		}
@@ -868,6 +903,7 @@ usage(const char *prog)
 								", JSON"
 #endif
 								"\n");
+	fprintf(stderr, "  -t, --config-test            Check the configuration for obvious errors\n");
 	fprintf(stderr, "  -v, --version                Display the version number\n");
 	fprintf(stderr, "  -h, --help                   Display this help message\n");
 }
@@ -935,6 +971,7 @@ parse_cmdline(int argc, char **argv)
 #endif
 		{"config-id",		required_argument,	NULL, 'i'},
 		{"signum",		required_argument,	NULL,  4 },
+		{"config-test",		no_argument,		NULL, 't'},
 		{"version",		no_argument,		NULL, 'v'},
 		{"help",		no_argument,		NULL, 'h'},
 
@@ -945,7 +982,7 @@ parse_cmdline(int argc, char **argv)
 	 * of longindex, so we need to ensure that before calling getopt_long(), longindex
 	 * is set to a know invalid value */
 	curind = optind;
-	while (longindex = -1, (c = getopt_long(argc, argv, ":vhlndDRS:f:p:i:mM::g::G"
+	while (longindex = -1, (c = getopt_long(argc, argv, ":vhlndDRS:f:p:i:mM::g::Gt"
 #if defined _WITH_VRRP_ && defined _WITH_LVS_
 					    "PC"
 #endif
@@ -1047,6 +1084,9 @@ parse_cmdline(int argc, char **argv)
 			__set_bit(NO_SYSLOG_BIT, &debug);
 			reopen_log = true;
 			break;
+		case 't':
+			__set_bit(CONFIG_TEST_BIT, &debug);
+			break;
 		case 'f':
 			conf_file = optarg;
 			break;
@@ -1194,6 +1234,9 @@ keepalived_main(int argc, char **argv)
 	/* Initialise pointer to child finding function */
 	set_child_finder_name(find_keepalived_child_name);
 
+	/* If one of our children terminates, we want to clear it out */
+	set_child_remover(parent_child_remover);
+
 	/* Initialise daemon_mode */
 #ifdef _WITH_VRRP_
 	__set_bit(DAEMON_VRRP, &daemon_mode);
diff --git a/keepalived/vrrp/vrrp_daemon.c b/keepalived/vrrp/vrrp_daemon.c
@@ -322,6 +322,12 @@ start_vrrp(void)
 	if (reload)
 		init_global_data(global_data);
 
+	/* If we are just testing the configuration, then we terminate now */
+	if (__test_bit(CONFIG_TEST_BIT, &debug)) {
+		stop_vrrp(KEEPALIVED_EXIT_CONFIG_TEST);
+		return;
+	}
+
 	/* Set the process priority and non swappable if configured */
 	set_process_priorities(
 #ifdef _HAVE_SCHED_RT_
@@ -681,7 +687,9 @@ vrrp_respawn_thread(thread_t * thread)
 	}
 
 	/* We catch a SIGCHLD, handle it */
-	if (!__test_bit(DONT_RESPAWN_BIT, &debug)) {
+	if (__test_bit(CONFIG_TEST_BIT, &debug))
+		raise(SIGTERM);
+	else if (!__test_bit(DONT_RESPAWN_BIT, &debug)) {
 		log_message(LOG_ALERT, "VRRP child process(%d) died: Respawning", pid);
 		start_vrrp_child();
 	} else {
@@ -771,7 +779,7 @@ start_vrrp_child(void)
 
 	/* Clear any child finder functions set in parent */
 	set_child_finder_name(NULL);
-	set_child_finder(NULL, NULL, NULL, NULL, NULL, 0);	/* Currently these won't be set */
+	destroy_child_finder();
 
 	/* Child process part, write pidfile */
 	if (!pidfile_write(vrrp_pidfile, getpid())) {
diff --git a/lib/bitops.h b/lib/bitops.h
@@ -74,6 +74,7 @@ enum global_bits {
 #ifdef _WITH_LVS_
 	LOG_ADDRESS_CHANGES,
 #endif
+	CONFIG_TEST_BIT,
 };
 
 #endif
diff --git a/lib/scheduler.c b/lib/scheduler.c
@@ -48,9 +48,7 @@
 #include "utils.h"
 #include "signals.h"
 #include "logger.h"
-#ifdef _DEBUG_
 #include "bitops.h"
-#endif
 #include "git-commit.h"
 #include <sys/utsname.h>
 #include <linux/version.h>
@@ -195,7 +193,13 @@ set_child_finder(void (*adder_func)(thread_t *),
 	}
 }
 
-static void
+void
+set_child_remover(void (*remover_func)(thread_t *))
+{
+	child_remover = remover_func;
+}
+
+void
 destroy_child_finder(void)
 {
 	set_child_finder(NULL, NULL, NULL, NULL, NULL, 0);
@@ -1201,13 +1205,15 @@ process_child_termination(pid_t pid, int status)
 	if (child_remover)
 		child_remover(thread);
 
-	if (permanent_vrrp_checker_error)
+	if (permanent_vrrp_checker_error || __test_bit(CONFIG_TEST_BIT, &debug))
 	{
-		/* The child had a permanant error, so no point in respawning */
+		/* The child had a permanant error, or we were just testing the config,
+		 * so no point in respawning */
 		thread->type = THREAD_UNUSED;
 		thread_list_add(&m->unuse, thread);
 
-		raise(SIGTERM);
+		if (!__test_bit(CONFIG_TEST_BIT, &debug))
+			raise(SIGTERM);
 	}
 	else
 	{
diff --git a/lib/scheduler.h b/lib/scheduler.h
@@ -121,8 +121,9 @@ typedef enum {
 #define THREAD_CHILD_STATUS(X) ((X)->u.c.status)
 
 /* Exit codes */
-#define KEEPALIVED_EXIT_FATAL	(EXIT_FAILURE+1)
-#define KEEPALIVED_EXIT_CONFIG	(EXIT_FAILURE+2)
+#define KEEPALIVED_EXIT_FATAL		(EXIT_FAILURE+1)
+#define KEEPALIVED_EXIT_CONFIG		(EXIT_FAILURE+2)
+#define KEEPALIVED_EXIT_CONFIG_TEST	(EXIT_FAILURE+3)
 
 #define DEFAULT_CHILD_FINDER ((void *)1)
 
@@ -138,6 +139,8 @@ extern bool snmp_running;
 /* Prototypes. */
 extern void set_child_finder_name(char const * (*)(pid_t));
 extern void set_child_finder(void (*)(thread_t *), thread_t *(*)(pid_t), void (*)(thread_t *), bool (*)(size_t), void(*)(void), size_t);
+extern void destroy_child_finder(void);
+extern void set_child_remover(void (*)(thread_t *));
 #ifndef _DEBUG_
 extern bool report_child_status(int, pid_t, const char *);
 #endif

-Original file line number
+Diff line change
 	* Allow dynamic definitions, e.g. $_VI_NAME
 	    Also $*_INSTANCE net_namspace $_INSTANCE
 	    so $* means only do if $_INSTANCE not blank. */
 +	* ng-scheduler - would libev help, or use select with epoll to get
 +	    micro-second timer
 +Collapse commentComment on line R39
acassen commented on Jun 17, 2018 
acassen
on Jun 17, 2018
Owner
acassen/scheduler-ng present high perf extension of current select scheduler. Personnally I dont like libev, we already have enough dependencies on extra libs. acassen/scheduler-ng introduce rbtree design to reduce timer handling latency (just the way linux kernel operate), and other extensions providing high perf I/O MUX around epoll(). timer is caclulated with nano precision and registered to epoll using a timerfd. The only missing part right now is to backport operations around signals in current scheduler to signalfd for epoll.
Code has comments. Press enter to view.
 Other issues awaiting resolution: