From 14c3c5c1216cc8cb7b421135dc44d658a748bc11 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Mon, 23 Aug 2021 14:10:51 +0200 Subject: [PATCH] MEDIUM: server: allow to remove servers at runtime except non purgeable Relax the condition on "delete server" CLI handler to be able to remove all servers, even non dynamic, except if they are flagged as non purgeable. This change is necessary to extend the use cases for dynamic servers with reload. It's expected that each dynamic server created via the CLI is manually commited in the haproxy configuration by the user. Dynamic servers will be present on reload only if they are present in the configuration file. This means that non-dynamic servers must be allowed to be removable at runtime. The dynamic servers removal reg-test has been updated and renamed to reflect its purpose. A new test is present to check that non-purgeable servers cannot be removed. --- doc/management.txt | 14 +++- reg-tests/server/cli_delete_dynamic_server.vtc | 98 ++++++++++++++++++++++++ reg-tests/server/cli_delete_server.vtc | 81 ++++++++------------ reg-tests/server/get_srv_stats.lua | 11 +++ src/server.c | 7 +- 5 files changed, 155 insertions(+), 56 deletions(-) create mode 100644 reg-tests/server/cli_delete_dynamic_server.vtc create mode 100644 reg-tests/server/get_srv_stats.lua diff --git a/doc/management.txt b/doc/management.txt index a6baf8e..58d06e4 100644 --- a/doc/management.txt +++ b/doc/management.txt @@ -1470,6 +1470,11 @@ add server / [args]* init-addr method. This means that no resolution will be undertaken if a FQDN is specified as an address, even if the server creation will be validated. + To support the reload operations, it is expected that the server created via + the CLI is also manually inserted in the relevant haproxy configuration file. + A dynamic server not present in the configuration won't be restored after a + reload operation. + A dynamic server may use the "track" keyword to follow the check status of another server from the configuration. However, it is not possible to track another dynamic server. This is to ensure that the tracking chain is kept @@ -1786,10 +1791,11 @@ del ssl crt-list numbers, use "show ssl crt-list -n ". del server / - Remove a server attached to the backend . Only valid on a server - added at runtime. The server must be put in maintenance mode prior to its - deletion. The operation is cancelled if the serveur still has active - or idle connection or its connection queue is not empty. + Remove a server attached to the backend . All servers are eligible, + except servers which are referenced by other configuration elements. The + server must be put in maintenance mode prior to its deletion. The operation + is cancelled if the serveur still has active or idle connection or its + connection queue is not empty. disable agent / Mark the auxiliary agent check as temporarily stopped. diff --git a/reg-tests/server/cli_delete_dynamic_server.vtc b/reg-tests/server/cli_delete_dynamic_server.vtc new file mode 100644 index 0000000..b1ddc4f --- /dev/null +++ b/reg-tests/server/cli_delete_dynamic_server.vtc @@ -0,0 +1,98 @@ +# This script is to test the proper behavior with dynamic servers insertion and +# deletion, in particular with the load-balancing of requests. +# +varnishtest "Delete server via cli" + +feature ignore_unknown_macro + +#REQUIRE_VERSION=2.4 + +# static server +server s1 -repeat 3 { + rxreq + txresp \ + -body "resp from s1" +} -start + +# use as a dynamic server, added then deleted via CLI +server s2 -repeat 3 { + rxreq + txresp \ + -body "resp from s2" +} -start + +haproxy h1 -conf { + defaults + mode http + timeout connect 1s + timeout client 1s + timeout server 1s + + frontend fe + bind "fd@${feS}" + default_backend test + + backend test + server s1 ${s1_addr}:${s1_port} +} -start + +# add a new dynamic server to be able to delete it then +haproxy h1 -cli { + # add a dynamic server and enable it + send "experimental-mode on; add server test/s2 ${s2_addr}:${s2_port}" + expect ~ "New server registered." + + send "enable server test/s2" + expect ~ ".*" +} + +haproxy h1 -cli { + # experimental mode disabled + send "del server test/s1" + expect ~ "This command is restricted to experimental mode only." + + # non existent backend + send "experimental-mode on; del server foo/s1" + expect ~ "No such backend." + + # non existent server + send "experimental-mode on; del server test/other" + expect ~ "No such server." +} + +# first check that both servers are active +client c1 -connect ${h1_feS_sock} { + txreq + rxresp + expect resp.body == "resp from s1" + + txreq + rxresp + expect resp.body == "resp from s2" +} -run + +# delete the dynamic server +haproxy h1 -cli { + # server not in maintenance mode + send "experimental-mode on; del server test/s2" + expect ~ "Only servers in maintenance mode can be deleted." + + send "disable server test/s2" + expect ~ ".*" + + # valid command + send "experimental-mode on; del server test/s2" + expect ~ "Server deleted." +} + +# now check that only the first server is used +client c2 -connect ${h1_feS_sock} { + txreq + rxresp + expect resp.body == "resp from s1" + + txreq + rxresp + expect resp.body == "resp from s1" +} -run + diff --git a/reg-tests/server/cli_delete_server.vtc b/reg-tests/server/cli_delete_server.vtc index 7b92d71..d752d6f 100644 --- a/reg-tests/server/cli_delete_server.vtc +++ b/reg-tests/server/cli_delete_server.vtc @@ -1,24 +1,21 @@ +# This script is to test the ability to remove servers, unless they are +# referenced by some elements from the configuration. +# varnishtest "Delete server via cli" feature ignore_unknown_macro #REQUIRE_VERSION=2.4 -# static server -server s1 -repeat 3 { +server s1 { rxreq - txresp \ - -body "resp from s1" -} -start - -# use as a dynamic server, added then deleted via CLI -server s2 -repeat 3 { - rxreq - txresp \ - -body "resp from s2" + txresp } -start haproxy h1 -conf { + global + lua-load ${testdir}/get_srv_stats.lua + defaults mode http timeout connect 1s @@ -27,22 +24,19 @@ haproxy h1 -conf { frontend fe bind "fd@${feS}" + acl s1_full srv_sess_rate(test/s1) gt 50 default_backend test backend test - server s1 ${s1_addr}:${s1_port} + http-request add-header s4-stats %[lua.get_srv_stats(s4)] + use-server s3 unless { always_false } + server s1 ${s1_addr}:${s1_port} # referenced in ACL + server s2 ${s1_addr}:${s1_port} check # referenced in track + server s3 ${s1_addr}:${s1_port} track s2 # referenced in use-server + server s4 ${s1_addr}:${s1_port} # referenced in lua script + server s5 ${s1_addr}:${s1_port} # removable server } -start -# add a new dynamic server to be able to delete it then -haproxy h1 -cli { - # add a dynamic server and enable it - send "experimental-mode on; add server test/s2 ${s2_addr}:${s2_port}" - expect ~ "New server registered." - - send "enable server test/s2" - expect ~ ".*" -} - haproxy h1 -cli { # experimental mode disabled send "del server test/s1" @@ -56,44 +50,37 @@ haproxy h1 -cli { send "experimental-mode on; del server test/other" expect ~ "No such server." - # static server + # server referenced in ACL send "experimental-mode on; del server test/s1" - expect ~ "Only servers added at runtime via CLI cmd can be deleted." + expect ~ "This server cannot be removed at runtime due to other configuration elements pointing to it." + + # tracked server + send "experimental-mode on; del server test/s2" + expect ~ "This server cannot be removed at runtime due to other configuration elements pointing to it." + + # tracked server + send "experimental-mode on; del server test/s3" + expect ~ "This server cannot be removed at runtime due to other configuration elements pointing to it." } -# first check that both servers are active +# make a request to force the execution of the lua script which references a +# server client c1 -connect ${h1_feS_sock} { txreq rxresp - expect resp.body == "resp from s1" - - txreq - rxresp - expect resp.body == "resp from s2" } -run -# delete the dynamic server haproxy h1 -cli { - # server not in maintenance mode - send "experimental-mode on; del server test/s2" + send "experimental-mode on; del server test/s4" + expect ~ "This server cannot be removed at runtime due to other configuration elements pointing to it." + + send "experimental-mode on; del server test/s5" expect ~ "Only servers in maintenance mode can be deleted." - send "disable server test/s2" + send "disable server test/s5" expect ~ ".*" # valid command - send "experimental-mode on; del server test/s2" + send "experimental-mode on; del server test/s5" expect ~ "Server deleted." } - -# now check that only the first server is used -client c2 -connect ${h1_feS_sock} { - txreq - rxresp - expect resp.body == "resp from s1" - - txreq - rxresp - expect resp.body == "resp from s1" -} -run - diff --git a/reg-tests/server/get_srv_stats.lua b/reg-tests/server/get_srv_stats.lua new file mode 100644 index 0000000..105b954 --- /dev/null +++ b/reg-tests/server/get_srv_stats.lua @@ -0,0 +1,11 @@ +local function lua_get_srv_stats(txn, name) + for _, backend in pairs(core.backends) do + for _, server in pairs(backend.servers) do + if server.name == name then + return server:get_stats() + end + end + end +end + +core.register_fetches('get_srv_stats', lua_get_srv_stats) diff --git a/src/server.c b/src/server.c index e055fc2..f0228be 100644 --- a/src/server.c +++ b/src/server.c @@ -4741,14 +4741,11 @@ static int cli_parse_delete_server(char **args, char *payload, struct appctx *ap goto out; } - if (!(srv->flags & SRV_F_DYNAMIC)) { - cli_err(appctx, "Only servers added at runtime via CLI cmd can be deleted."); + if (srv->flags & SRV_F_NON_PURGEABLE) { + cli_err(appctx, "This server cannot be removed at runtime due to other configuration elements pointing to it."); goto out; } - /* A dynamic server cannot be tracked. */ - BUG_ON(srv->trackers); - /* Only servers in maintenance can be deleted. This ensures that the * server is not present anymore in the lb structures (through * lbprm.set_server_status_down). -- 1.7.10.4