From: Willy Tarreau <w@1wt.eu>
Date: Fri, 11 Jun 2021 14:27:10 +0000 (+0200)
Subject: MEDIUM: proxy: remove the deprecated "grace" keyword
X-Git-Tag: v2.5-dev1~142
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=eb77824;p=thirdparty%2Fhaproxy.git

MEDIUM: proxy: remove the deprecated "grace" keyword

Commit ab0a5192a ("MEDIUM: config: mark "grace" as deprecated") marked
the "grace" keyword as deprecated in 2.3, tentative removal for 2.4
with a hard deadline in 2.5, so let's remove it and return an error now.
This old and outdated feature was incompatible with soft-stop, reload
and socket transfers, and keeping it forced ugly hacks in the lower
layers of the protocol stack.
---

diff --git a/admin/syntax-highlight/haproxy.vim b/admin/syntax-highlight/haproxy.vim
index 02c3232551..e50a76cf5e 100644
--- a/admin/syntax-highlight/haproxy.vim
+++ b/admin/syntax-highlight/haproxy.vim
@@ -44,7 +44,7 @@ syn keyword hapParam     chroot cliexp
 syn keyword hapParam     daemon debug disabled
 syn keyword hapParam     enabled
 syn keyword hapParam     fullconn
-syn keyword hapParam     gid grace group
+syn keyword hapParam     gid group
 syn keyword hapParam     maxconn monitor-uri
 syn keyword hapParam     nbproc noepoll nopoll
 syn keyword hapParam     pidfile
diff --git a/doc/configuration.txt b/doc/configuration.txt
index 55469de38d..c5d85c674e 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -3562,7 +3562,6 @@ errorloc303                               X          X         X         X
 force-persist                             -          -         X         X
 filter                                    -          X         X         X
 fullconn                                  X          -         X         X
-grace                                     X          X         X         X
 hash-type                                 X          -         X         X
 http-after-response                       -          X         X         X
 http-check comment                        X          -         X         X
@@ -5097,25 +5096,6 @@ fullconn <conns>
   See also : "maxconn", "server"
 
 
-grace <time> (deprecated)
-  Maintain a proxy operational for some time after a soft stop
-  May be used in sections :   defaults | frontend | listen | backend
-                                 yes   |    yes   |   yes  |   yes
-  Arguments :
-    <time>    is the time (by default in milliseconds) for which the instance
-              will remain operational with the frontend sockets still listening
-              when a soft-stop is received via the SIGUSR1 signal.
-
-  This may be used to ensure that the services disappear in a certain order.
-  This was designed so that frontends which are dedicated to monitoring by an
-  external equipment fail immediately while other ones remain up for the time
-  needed by the equipment to detect the failure.
-
-  Note that currently, there is very little benefit in using this parameter,
-  and it may in fact complicate the soft-reconfiguration process more than
-  simplify it.
-
-
 hash-balance-factor <factor>
   Specify the balancing factor for bounded-load consistent hashing
   May be used in sections :   defaults | frontend | listen | backend
diff --git a/include/haproxy/protocol.h b/include/haproxy/protocol.h
index 81f4d5bdbb..eba28e5ade 100644
--- a/include/haproxy/protocol.h
+++ b/include/haproxy/protocol.h
@@ -50,11 +50,9 @@ int protocol_bind_all(int verbose);
  */
 int protocol_unbind_all(void);
 
-/* stops all listeners of all registered protocols, except when the belong to a
- * proxy configured with a grace time. This will normally catch every single
- * listener, all protocols included, and the grace ones will have to be handled
- * by the proxy stopping loop. This is to be used during soft_stop() only. It
- * does not return any error.
+/* stops all listeners of all registered protocols. This will normally catch
+ * every single listener, all protocols included. This is to be used during
+ * soft_stop() only. It does not return any error.
  */
 void protocol_stop_now(void);
 
diff --git a/include/haproxy/proxy-t.h b/include/haproxy/proxy-t.h
index 95d3bc978f..93b5bd420c 100644
--- a/include/haproxy/proxy-t.h
+++ b/include/haproxy/proxy-t.h
@@ -389,7 +389,6 @@ struct proxy {
 	char *check_path;			/* PATH environment to use for external agent checks */
 	struct http_reply *replies[HTTP_ERR_SIZE]; /* HTTP replies for known errors */
 	unsigned int log_count;			/* number of logs produced by the frontend */
-	int grace;				/* grace time after stop request */
 	int uuid;				/* universally unique proxy ID, used for SNMP */
 	unsigned int backlog;			/* force the frontend's listen backlog */
 	unsigned long bind_proc;		/* bitmask of processes using this proxy */
diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c
index 29778a18c1..ea9dae6641 100644
--- a/src/cfgparse-listen.c
+++ b/src/cfgparse-listen.c
@@ -49,7 +49,7 @@ static const char *common_kw_list[] = {
 	"use-server", "force-persist", "ignore-persist", "force-persist",
 	"stick-table", "stick", "stats", "option", "default_backend",
 	"http-reuse", "monitor", "transparent", "maxconn", "backlog",
-	"fullconn", "grace", "dispatch", "balance", "hash-type",
+	"fullconn", "dispatch", "balance", "hash-type",
 	"hash-balance-factor", "unique-id-format", "unique-id-header",
 	"log-format", "log-format-sd", "log-tag", "log", "source", "usesrc",
 	NULL /* must be last */
@@ -204,7 +204,6 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm)
 	static struct proxy *last_defproxy = NULL;
 	const char *err;
 	int rc;
-	unsigned val;
 	int err_code = 0;
 	struct acl_cond *cond = NULL;
 	char *errmsg = NULL;
@@ -2464,36 +2463,10 @@ stats_error_parsing:
 			goto out;
 	}
 	else if (strcmp(args[0], "grace") == 0) {  /* grace time (ms) */
-		if (*(args[1]) == 0) {
-			ha_alert("parsing [%s:%d] : '%s' expects a time in milliseconds.\n", file, linenum, args[0]);
-			err_code |= ERR_ALERT | ERR_FATAL;
-			goto out;
-		}
-		err = parse_time_err(args[1], &val, TIME_UNIT_MS);
-		if (err == PARSE_TIME_OVER) {
-			ha_alert("parsing [%s:%d]: timer overflow in argument <%s> to grace time, maximum value is 2147483647 ms (~24.8 days).\n",
-			         file, linenum, args[1]);
-			err_code |= ERR_ALERT | ERR_FATAL;
-			goto out;
-		}
-		else if (err == PARSE_TIME_UNDER) {
-			ha_alert("parsing [%s:%d]: timer underflow in argument <%s> to grace time, minimum non-null value is 1 ms.\n",
-			         file, linenum, args[1]);
-			err_code |= ERR_ALERT | ERR_FATAL;
-			goto out;
-		}
-		else if (err) {
-			ha_alert("parsing [%s:%d] : unexpected character '%c' in grace time.\n",
-				 file, linenum, *err);
-			err_code |= ERR_ALERT | ERR_FATAL;
-			goto out;
-		}
-		curproxy->grace = val;
-		if (alertif_too_many_args(1, file, linenum, args, &err_code))
-			goto out;
-
-		ha_warning("parsing [%s:%d]: the '%s' is deprecated and will be removed in a future version.\n",
+		ha_alert("parsing [%s:%d]: the '%s' keyword is not supported any more since HAProxy version 2.5.\n",
 			   file, linenum, args[0]);
+		err_code |= ERR_ALERT | ERR_FATAL;
+		goto out;
 	}
 	else if (strcmp(args[0], "dispatch") == 0) {  /* dispatch address */
 		struct sockaddr_storage *sk;
diff --git a/src/protocol.c b/src/protocol.c
index 767e03a240..ecae17a608 100644
--- a/src/protocol.c
+++ b/src/protocol.c
@@ -140,11 +140,9 @@ int protocol_unbind_all(void)
 	return err;
 }
 
-/* stops all listeners of all registered protocols, except when the belong to a
- * proxy configured with a grace time. This will normally catch every single
- * listener, all protocols included, and the grace ones will have to be handled
- * by the proxy stopping loop. This is to be used during soft_stop() only. It
- * does not return any error.
+/* stops all listeners of all registered protocols. This will normally catch
+ * every single listener, all protocols included. This is to be used during
+ * soft_stop() only. It does not return any error.
  */
 void protocol_stop_now(void)
 {
@@ -154,8 +152,7 @@ void protocol_stop_now(void)
 	HA_SPIN_LOCK(PROTO_LOCK, &proto_lock);
 	list_for_each_entry(proto, &protocols, list) {
 		list_for_each_entry_safe(listener, lback, &proto->receivers, rx.proto_list)
-			if (!listener->bind_conf->frontend->grace)
-				stop_listener(listener, 0, 1, 0);
+			stop_listener(listener, 0, 1, 0);
 	}
 	HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock);
 }
diff --git a/src/proxy.c b/src/proxy.c
index 13d2cc8a6f..6d0219359f 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -1724,8 +1724,6 @@ static int proxy_defproxy_cpy(struct proxy *curproxy, const struct proxy *defpro
 		curproxy->comp->types = defproxy->comp->types;
 	}
 
-	curproxy->grace  = defproxy->grace;
-
 	if (defproxy->check_path)
 		curproxy->check_path = strdup(defproxy->check_path);
 	if (defproxy->check_command)
@@ -1978,12 +1976,10 @@ struct task *hard_stop(struct task *t, void *context, unsigned int state)
 
 /*
  * this function disables health-check servers so that the process will quickly be ignored
- * by load balancers. Note that if a proxy was already in the PAUSED state, then its grace
- * time will not be used since it would already not listen anymore to the socket.
+ * by load balancers.
  */
 void soft_stop(void)
 {
-	struct proxy *p;
 	struct task *task;
 
 	stopping = 1;
@@ -2000,31 +1996,9 @@ void soft_stop(void)
 		}
 	}
 
-	/* stop all stoppable listeners, resulting in disabling all proxies
-	 * that don't use a grace period.
-	 */
+	/* stop all stoppable listeners */
 	protocol_stop_now();
 
-	p = proxies_list;
-	tv_update_date(0,1); /* else, the old time before select will be used */
-	while (p) {
-		if (!p->disabled) {
-			ha_warning("Stopping %s %s in %d ms.\n", proxy_cap_str(p->cap), p->id, p->grace);
-			send_log(p, LOG_WARNING, "Stopping %s %s in %d ms.\n", proxy_cap_str(p->cap), p->id, p->grace);
-			p->stop_time = tick_add(now_ms, p->grace);
-
-			/* Note: do not wake up stopped proxies' task nor their tables'
-			 * tasks as these ones might point to already released entries.
-			 */
-			if (p->table && p->table->size && p->table->sync_task)
-				task_wakeup(p->table->sync_task, TASK_WOKEN_MSG);
-
-			if (p->task)
-				task_wakeup(p->task, TASK_WOKEN_MSG);
-		}
-		p = p->next;
-	}
-
 	/* signal zero is used to broadcast the "stopping" event */
 	signal_handler(0);
 }