diff --git a/doc/configuration.txt b/doc/configuration.txt index ef79c9be7..0d1476333 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -647,8 +647,8 @@ which must be placed before other sections, but it may be repeated if needed. In addition, some automatic identifiers may automatically be assigned to some of the created objects (e.g. proxies), and by reordering sections, their identifiers will change. These ones appear in the statistics for example. As -such, the configuration below will assign "foo" ID number 1 and "bar" ID number -2, which will be swapped if the two sections are reversed: +such, the configuration below will assign "foo" an ID number smaller than its +"bar" counterpart. This will be swapped if the two sections are reversed: listen foo bind :80 @@ -8692,9 +8692,11 @@ id Arguments : none - Set a persistent ID for the proxy. This ID must be unique and positive. - An unused ID will automatically be assigned if unset. The first assigned - value will be 1. This ID is currently only returned in statistics. + Set a persistent ID for the proxy. This ID must be unique and positive. An + unused ID will automatically be assigned if unset. Due to an historical + behavior, value 1 is not used unless explicitely set. Thus, the lowest value + automatically assigned will be 2. This ID is currently only returned in + statistics. ignore-persist { if | unless } diff --git a/reg-tests/proxy/proxy_id.vtc b/reg-tests/proxy/proxy_id.vtc new file mode 100644 index 000000000..4a2b851a8 --- /dev/null +++ b/reg-tests/proxy/proxy_id.vtc @@ -0,0 +1,58 @@ +varnishtest "Ensure that proxies automatic numbering remains consistent across versions" + +feature ignore_unknown_macro + +# No ID explicitely set. First automatically assigned value must be set to '2'. +# Value '1' is skipped due to an historical bug. +haproxy h1 -conf { + defaults + timeout connect "${HAPROXY_TEST_TIMEOUT-5s}" + timeout client "${HAPROXY_TEST_TIMEOUT-5s}" + timeout server "${HAPROXY_TEST_TIMEOUT-5s}" + + listen fe1 + bind "fd@${fe1}" + + listen fe2 + bind "fd@${fe2}" +} -start + +haproxy h1 -cli { + send "show stat 1 -1 -1" + expect !~ "fe[12]," + + send "show stat 2 -1 -1" + expect ~ "fe1," + + send "show stat 3 -1 -1" + expect ~ "fe2," +} + +# Explicitely uses ID 1 and 2. First automatically assigned value must be +# set to '3'. +haproxy h2 -conf { + defaults + timeout connect "${HAPROXY_TEST_TIMEOUT-5s}" + timeout client "${HAPROXY_TEST_TIMEOUT-5s}" + timeout server "${HAPROXY_TEST_TIMEOUT-5s}" + + listen fe1 + bind "fd@${fe1}" + listen fe2 + id 1 # 1 set as automatic value + bind "fd@${fe1}" + listen fe3 + id 2 + bind "fd@${fe3}" +} -start + +haproxy h2 -cli { + send "show stat 1 -1 -1" + expect ~ "fe2," + + send "show stat 2 -1 -1" + expect ~ "fe3," + + send "show stat 3 -1 -1" + expect ~ "fe1," +} diff --git a/src/cfgparse.c b/src/cfgparse.c index 3065319ad..bcb53d00b 100644 --- a/src/cfgparse.c +++ b/src/cfgparse.c @@ -2786,7 +2786,8 @@ int check_config_validity() struct server *newsrv = NULL; struct mt_list back; int err_code = 0; - unsigned int next_pxid = 1; + /* Value forced to skip '1' due to an historical bug, see below for more details. */ + unsigned int next_pxid = 2; struct bind_conf *bind_conf; char *err; struct cfg_postparser *postparser; @@ -2874,16 +2875,19 @@ init_proxies_list_stage1: unsigned int next_id; proxy_init_per_thr(curproxy); + + /* Assign automatic UUID if unset except for internal proxies. + * + * WARNING proxy UUID initialization is buggy as value '1' is + * skipped if not explicitely used. This is an historical bug + * and should not be corrected to prevent breakage on future + * versions. + */ if (!(curproxy->cap & PR_CAP_INT) && curproxy->uuid < 0) { - /* proxy ID not set, use automatic numbering with first - * spare entry starting with next_pxid. We don't assign - * numbers for internal proxies as they may depend on - * build or config options and we don't want them to - * possibly reuse existing IDs. - */ next_pxid = proxy_get_next_id(next_pxid); curproxy->uuid = next_pxid; proxy_index_id(curproxy); + next_pxid++; } if (curproxy->mode == PR_MODE_HTTP && global.tune.bufsize >= (256 << 20) && ONLY_ONCE()) { @@ -2892,17 +2896,6 @@ init_proxies_list_stage1: cfgerr++; } - /* next IDs are shifted even if the proxy is disabled, this - * guarantees that a proxy that is temporarily disabled in the - * configuration doesn't cause a renumbering. Internal proxies - * that are not assigned a static ID must never shift the IDs - * either since they may appear in any order (Lua, logs, etc). - * The GLOBAL proxy that carries the stats socket has its ID - * forced to zero. - */ - if (curproxy->uuid >= 0) - next_pxid++; - if (curproxy->flags & PR_FL_DISABLED) { /* ensure we don't keep listeners uselessly bound. We * can't disable their listeners yet (fdtab not