From: Tony Finch <dot@dotat.at>
Date: Fri, 14 Jun 2019 10:14:00 +0000 (+0100)
Subject: When a server reload fails, print a note in `rndc status`.
X-Git-Tag: v9.15.2~37^2~1
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=8e05e2e9d53c287ecc30d427a6570f1631143eef;p=thirdparty%2Fbind9.git

When a server reload fails, print a note in `rndc status`.

After a failed reload I noticed two problems:

* There was a missing newline in the output of `rndc status` so it
  finished "reload/reconfig in progressserver is up and running"
* The "reconfig in progress" note should have said "reconfig failed"
---

diff --git a/bin/named/include/named/server.h b/bin/named/include/named/server.h
index 399a14e6beb..25d0e4b755e 100644
--- a/bin/named/include/named/server.h
+++ b/bin/named/include/named/server.h
@@ -56,9 +56,9 @@ struct named_server {
 	char *			secrootsfile;	/*%< Secroots file name */
 	char *			bindkeysfile;	/*%< bind.keys file name */
 	char *			recfile;	/*%< Recursive file name */
-	bool		version_set;	/*%< User has set version */
+	bool			version_set;	/*%< User has set version */
 	char *			version;	/*%< User-specified version */
-	bool		hostname_set;	/*%< User has set hostname */
+	bool			hostname_set;	/*%< User has set hostname */
 	char *			hostname;	/*%< User-specified hostname */
 
 	/* Server data structures. */
@@ -78,9 +78,9 @@ struct named_server {
 
 	isc_mutex_t		reload_event_lock;
 	isc_event_t *		reload_event;
-	bool			reload_in_progress;
+	named_reload_t		reload_status;
 
-	bool		flushonshutdown;
+	bool			flushonshutdown;
 
 	named_cachelist_t	cachelist;	/*%< Possibly shared caches */
 	isc_stats_t *		zonestats;	/*% Zone management stats */
@@ -98,7 +98,7 @@ struct named_server {
 	dns_name_t		*session_keyname;
 	unsigned int		session_keyalg;
 	uint16_t		session_keybits;
-	bool		interface_auto;
+	bool			interface_auto;
 	unsigned char		secret[32];	/*%< Server Cookie Secret */
 	ns_cookiealg_t		cookiealg;
 
diff --git a/bin/named/include/named/types.h b/bin/named/include/named/types.h
index 0701091f9f0..234724a25c8 100644
--- a/bin/named/include/named/types.h
+++ b/bin/named/include/named/types.h
@@ -27,4 +27,13 @@ typedef ISC_LIST(named_dispatch_t)	named_dispatchlist_t;
 typedef struct named_statschannel	named_statschannel_t;
 typedef ISC_LIST(named_statschannel_t)	named_statschannellist_t;
 
+/*%
+ * Used for server->reload_status as printed by `rndc status`
+ */
+typedef enum {
+	NAMED_RELOAD_DONE,
+	NAMED_RELOAD_IN_PROGRESS,
+	NAMED_RELOAD_FAILED,
+} named_reload_t;
+
 #endif /* NAMED_TYPES_H */
diff --git a/bin/named/server.c b/bin/named/server.c
index 4f3c471b25b..b0019af6b89 100644
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -9397,7 +9397,7 @@ view_loaded(void *arg) {
 			      "FIPS mode is %s",
 			      FIPS_mode() ? "enabled" : "disabled");
 #endif
-		server->reload_in_progress = false;
+		server->reload_status = NAMED_RELOAD_DONE;
 
 		isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL,
 			      NAMED_LOGMODULE_SERVER, ISC_LOG_NOTICE,
@@ -9733,7 +9733,7 @@ named_server_create(isc_mem_t *mctx, named_server_t **serverp) {
 	CHECKFATAL(server->reload_event == NULL ?
 		   ISC_R_NOMEMORY : ISC_R_SUCCESS,
 		   "allocating reload event");
-	server->reload_in_progress = true;
+	server->reload_status = NAMED_RELOAD_IN_PROGRESS;
 
 	/*
 	 * Setup the server task, which is responsible for coordinating
@@ -10037,6 +10037,7 @@ loadconfig(named_server_t *server) {
 			      NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR,
 			      "reloading configuration failed: %s",
 			      isc_result_totext(result));
+		server->reload_status = NAMED_RELOAD_FAILED;
 	}
 
 	return (result);
@@ -10045,20 +10046,21 @@ loadconfig(named_server_t *server) {
 static isc_result_t
 reload(named_server_t *server) {
 	isc_result_t result;
-	server->reload_in_progress = true;
+	server->reload_status = NAMED_RELOAD_IN_PROGRESS;
 	CHECK(loadconfig(server));
 
 	result = load_zones(server, false, false);
-	if (result == ISC_R_SUCCESS)
+	if (result == ISC_R_SUCCESS) {
 		isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL,
 			      NAMED_LOGMODULE_SERVER, ISC_LOG_INFO,
 			      "reloading zones succeeded");
-	else
+	} else {
 		isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL,
 			      NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR,
 			      "reloading zones failed: %s",
 			      isc_result_totext(result));
-
+		server->reload_status = NAMED_RELOAD_FAILED;
+	}
  cleanup:
 	return (result);
 }
@@ -10392,20 +10394,22 @@ named_server_reloadcommand(named_server_t *server, isc_lex_t *lex,
 isc_result_t
 named_server_reconfigcommand(named_server_t *server) {
 	isc_result_t result;
-	server->reload_in_progress = true;
+	server->reload_status = NAMED_RELOAD_IN_PROGRESS;
 
 	CHECK(loadconfig(server));
 
 	result = load_zones(server, false, true);
-	if (result == ISC_R_SUCCESS)
+	if (result == ISC_R_SUCCESS) {
 		isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL,
 			      NAMED_LOGMODULE_SERVER, ISC_LOG_INFO,
 			      "scheduled loading new zones");
-	else
+	} else {
 		isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL,
 			      NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR,
 			      "loading new zones failed: %s",
 			      isc_result_totext(result));
+		server->reload_status = NAMED_RELOAD_FAILED;
+	}
 cleanup:
 	return (result);
 }
@@ -11539,8 +11543,11 @@ named_server_status(named_server_t *server, isc_buffer_t **text) {
 		     isc_quota_getmax(&server->sctx->tcpquota));
 	CHECK(putstr(text, line));
 
-	if (server->reload_in_progress) {
-		CHECK(putstr(text, "reload/reconfig in progress"));
+	if (server->reload_status != NAMED_RELOAD_DONE) {
+		snprintf(line, sizeof(line), "reload/reconfig %s\n",
+			 server->reload_status == NAMED_RELOAD_FAILED
+			 ? "failed" : "in progress");
+		CHECK(putstr(text, line));
 	}
 
 	CHECK(putstr(text, "server is up and running"));
diff --git a/bin/tests/system/rndc/tests.sh b/bin/tests/system/rndc/tests.sh
index 7cbe2c7338a..3718e143182 100644
--- a/bin/tests/system/rndc/tests.sh
+++ b/bin/tests/system/rndc/tests.sh
@@ -545,6 +545,20 @@ mv ns4/named.conf ns4/named.conf.save
 echo "error error error" >> ns4/named.conf
 $RNDC -s 10.53.0.4 -p ${EXTRAPORT6} -c ns4/key6.conf reconfig > rndc.out.1.test$n 2>&1 && ret=1
 grep "rndc: 'reconfig' failed: unexpected token" rndc.out.1.test$n > /dev/null || ret=1
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=`expr $status + $ret`
+
+n=`expr $n + 1`
+echo_i "check rndc status reports failure ($n)"
+ret=0
+$RNDC -s 10.53.0.4 -p ${EXTRAPORT6} -c ns4/key6.conf status > rndc.out.1.test$n 2>&1 || ret=1
+grep "reload/reconfig failed" rndc.out.1.test$n > /dev/null || ret=1
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=`expr $status + $ret`
+
+n=`expr $n + 1`
+echo_i "restore working config ($n)"
+ret=0
 mv ns4/named.conf.save ns4/named.conf
 sleep 1
 $RNDC -s 10.53.0.4 -p ${EXTRAPORT6} -c ns4/key6.conf reconfig > /dev/null || ret=1
@@ -552,6 +566,14 @@ sleep 1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
+n=`expr $n + 1`
+echo_i "check 'rndc status' 'reload/reconfig failure' is cleared after successful reload/reconfig ($n)"
+ret=0
+$RNDC -s 10.53.0.4 -p ${EXTRAPORT6} -c ns4/key6.conf status > rndc.out.1.test$n 2>&1 || ret=1
+grep "reload/reconfig failed" rndc.out.1.test$n > /dev/null && ret=1
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=`expr $status + $ret`
+
 n=`expr $n + 1`
 echo_i "test read-only control channel access ($n)"
 ret=0