Skip to content

Commit 66281f8

Browse files
themilchenkothemilchenko
committed
api: fix address already in use error
There was a bug when user wants to change server's `listen` address and reload config on the fly - error `address already in use` occures. This patch reworks server's address handling by applying roles' config. Closes #34
1 parent 3396bc0 commit 66281f8

File tree

4 files changed

+260
-20
lines changed

4 files changed

+260
-20
lines changed

‎CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1111

1212
### Fixed
1313

14+
- `Address already in use` error on change role's config on the fly (#34).
15+
1416
### Changed
1517

1618
- Unclear error message when `roles.httpd` config is not applied yet (#33).

‎roles/metrics-export.lua

Lines changed: 50 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -329,8 +329,8 @@ local function apply_http(conf)
329329
local host, port, target
330330
if node.server ~= nil then
331331
target = {
332-
value = node.server,
333-
is_httpd_role = true,
332+
value = 'httpd_' .. node.server,
333+
httpd_name = node.server,
334334
}
335335
elseif node.listen ~= nil then
336336
local err
@@ -339,23 +339,49 @@ local function apply_http(conf)
339339
error("failed to parse URI: " .. err, 2)
340340
end
341341
target = {
342-
value = node.listen,
343-
is_httpd_role = false,
342+
value = 'listen_' .. host .. ':' .. tostring(port),
344343
}
345344
else
346345
target = {
347-
value = httpd_role.DEFAULT_SERVER_NAME,
348-
is_httpd_role = true,
346+
value = 'httpd_' .. httpd_role.DEFAULT_SERVER_NAME,
347+
httpd_name = httpd_role.DEFAULT_SERVER_NAME,
349348
}
350349
end
351350

352351
http_servers = http_servers or {}
352+
353353
-- Since the 'listen' and 'server' names of other servers in the config may be
354-
-- the same, we create a unique string concatenating the key name and information
355-
-- about whether it is an httpd key or not.
356-
enabled[tostring(target.value) .. tostring(target.is_httpd_role)] = true
354+
-- the same, we create a unique string concatenating the key (which is listen or server)
355+
-- and its value.
356+
enabled[target.value] = true
357+
358+
if http_servers[target.value] == nil then
359+
if target.httpd_name == nil then
360+
local count_global_addrs = 0
361+
362+
for _, n in pairs(conf) do
363+
-- Count if it is more than one server that on gloabl address 0.0.0.0.
364+
if n.listen ~= nil and string.find(tostring(n.listen), tostring(port)) ~= nil then
365+
count_global_addrs = count_global_addrs + 1
366+
end
367+
end
368+
369+
if host == '0.0.0.0' then
370+
if count_global_addrs == 1 then
371+
for k in pairs(http_servers) do
372+
if http_servers[k].httpd_name == nil and string.find(k, ':' .. port) ~= nil then
373+
http_servers[k].httpd:stop()
374+
enabled[k] = false
375+
end
376+
end
377+
end
378+
elseif http_servers['listen_0.0.0.0:' .. port] ~= nil and count_global_addrs == 1 then
379+
local key = 'listen_0.0.0.0:' .. port
380+
http_servers[key].httpd:stop()
381+
enabled[key] = false
382+
end
383+
end
357384

358-
if http_servers[tostring(target.value) .. tostring(target.is_httpd_role)] == nil then
359385
local httpd
360386
if node.listen ~= nil then
361387
httpd = http_server.new(host, port, {
@@ -368,21 +394,21 @@ local function apply_http(conf)
368394
})
369395
httpd:start()
370396
else
371-
httpd = httpd_role.get_server(target.value)
397+
httpd = httpd_role.get_server(target.httpd_name)
372398
if httpd == nil then
373399
error(('failed to get server by name %q, check that roles.httpd was' ..
374-
' already applied'):format(target.value))
400+
' already applied'):format(target.httpd_name))
375401
end
376402
end
377403

378-
http_servers[tostring(target.value) .. tostring(target.is_httpd_role)] = {
404+
http_servers[target.value] = {
379405
httpd = httpd,
380406
routes = {},
381-
is_httpd_role = target.is_httpd_role,
407+
httpd_name = target.httpd_name,
382408
}
383409
end
384410

385-
local server = http_servers[tostring(target.value) .. tostring(target.is_httpd_role)]
411+
local server = http_servers[target.value]
386412
local httpd = server.httpd
387413
local old_routes = server.routes
388414

@@ -422,14 +448,16 @@ local function apply_http(conf)
422448
end
423449
end
424450

425-
for target, server in pairs(http_servers) do
451+
for target, server in pairs(http_servers or {}) do
426452
if not enabled[target] then
427-
if server.is_httpd_role then
453+
if server.httpd_name ~= nil then
428454
for path, _ in pairs(server.routes) do
429455
server.httpd:delete(path)
430456
end
431457
else
432-
server.httpd:stop()
458+
if server.httpd.is_run == true then
459+
server.httpd:stop()
460+
end
433461
end
434462
http_servers[target] = nil
435463
end
@@ -438,12 +466,14 @@ end
438466

439467
local function stop_http()
440468
for _, server in pairs(http_servers or {}) do
441-
if server.is_httpd_role then
469+
if server.httpd_name ~= nil then
442470
for path, _ in pairs(server.routes) do
443471
server.httpd:delete(path)
444472
end
445473
else
446-
server.httpd:stop()
474+
if server.httpd.is_run == true then
475+
server.httpd:stop()
476+
end
447477
end
448478
end
449479
http_servers = nil

‎test/integration/reload_config_test.lua

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
local fio = require('fio')
2+
local yaml = require('yaml')
3+
local socket = require('socket')
4+
local helpers = require('test.helpers')
5+
local server = require('test.helpers.server')
6+
7+
local t = require('luatest')
8+
local g = t.group()
9+
10+
g.before_all(function()
11+
helpers.skip_if_unsupported()
12+
end)
13+
14+
g.before_each(function(cg)
15+
cg.workdir = fio.tempdir()
16+
fio.mktree(cg.workdir)
17+
18+
fio.copytree(".rocks", fio.pathjoin(cg.workdir, ".rocks"))
19+
fio.copytree("roles", fio.pathjoin(cg.workdir, "roles"))
20+
fio.copytree(fio.pathjoin("test", "ssl_data"), fio.pathjoin(cg.workdir, "ssl_data"))
21+
fio.copyfile(fio.pathjoin('test', 'entrypoint', 'config.yaml'), cg.workdir)
22+
end)
23+
24+
g.after_each(function(cg)
25+
cg.server:stop()
26+
fio.rmtree(cg.workdir)
27+
end)
28+
29+
local function is_tcp_connect(host, port)
30+
local tcp = socket.tcp()
31+
tcp:settimeout(0.3)
32+
local ok, _ = tcp:connect(host, port)
33+
tcp:close()
34+
35+
return ok
36+
end
37+
38+
local function change_listen_target_in_config(cg, old_addr, new_addr)
39+
local file = fio.open(fio.pathjoin(cg.workdir, 'config.yaml'), {'O_RDONLY'})
40+
t.assert(file ~= nil)
41+
42+
local cfg = file:read()
43+
file:close()
44+
45+
cfg = yaml.decode(cfg)
46+
local export_instances = cfg.groups['group-001'].replicasets['replicaset-001'].
47+
instances.master.roles_cfg['roles.metrics-export'].http
48+
49+
for i, v in pairs(export_instances) do
50+
if v.listen ~= nil and v.listen == old_addr then
51+
export_instances[i].listen = new_addr
52+
end
53+
end
54+
55+
file = fio.open(fio.pathjoin(cg.workdir, 'config.yaml'), {'O_CREAT', 'O_WRONLY', 'O_TRUNC'}, tonumber('644', 8))
56+
file:write(yaml.encode(cfg))
57+
file:close()
58+
end
59+
60+
g.test_reload_config_update_addr = function(cg)
61+
cg.server = server:new({
62+
config_file = fio.pathjoin(cg.workdir, 'config.yaml'),
63+
chdir = cg.workdir,
64+
alias = 'master',
65+
workdir = cg.workdir,
66+
})
67+
68+
cg.server:start({wait_until_ready = true})
69+
70+
t.assert(is_tcp_connect('127.0.0.1', 8082))
71+
t.assert_not(is_tcp_connect('127.0.0.2', 8082))
72+
73+
change_listen_target_in_config(cg, '127.0.0.1:8082', '0.0.0.0:8082')
74+
cg.server:eval("require('config'):reload()")
75+
76+
t.assert(is_tcp_connect('127.0.0.1', 8082))
77+
t.assert(is_tcp_connect('127.0.0.2', 8082))
78+
t.assert(is_tcp_connect('127.1.2.3', 8082))
79+
80+
change_listen_target_in_config(cg, '0.0.0.0:8082', '127.0.0.1:8082')
81+
cg.server:eval("require('config'):reload()")
82+
83+
t.assert_not(is_tcp_connect('127.0.0.2', 8082))
84+
t.assert(is_tcp_connect('127.0.0.1', 8082))
85+
end
86+
87+
g.test_reload_config_global_addr_conflict = function(cg)
88+
cg.server = server:new({
89+
config_file = fio.pathjoin(cg.workdir, 'config.yaml'),
90+
chdir = cg.workdir,
91+
alias = 'master',
92+
workdir = cg.workdir,
93+
})
94+
95+
cg.server:start({wait_until_ready = true})
96+
97+
change_listen_target_in_config(cg, 8081, '0.0.0.0:8082')
98+
t.assert_error_msg_content_equals(
99+
"Can't create tcp_server: Address already in use",
100+
function() cg.server:eval("require('config'):reload()") end
101+
)
102+
end

‎test/unit/http_test.lua

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -699,6 +699,57 @@ local test_reapply_delete_cases = {
699699
},
700700
},
701701
},
702+
["listen_global_addr"] = {
703+
apply_cases = {
704+
{
705+
cfg = {
706+
http = {
707+
{
708+
listen = '0.0.0.0:8081',
709+
endpoints = {
710+
{
711+
path = "/metrics1",
712+
format = "prometheus",
713+
},
714+
},
715+
},
716+
},
717+
},
718+
expected_json_urls = {},
719+
expected_prometheus_urls = {
720+
"http://127.0.0.1:8081/metrics1",
721+
"http://127.1.2.3:8081/metrics1"
722+
},
723+
expected_none_urls = {},
724+
},
725+
{
726+
cfg = {
727+
http = {
728+
{
729+
listen = '127.0.0.1:8081',
730+
endpoints = {
731+
{
732+
path = "/metrics1",
733+
format = "json",
734+
},
735+
{
736+
path = "/metrics2",
737+
format = "prometheus",
738+
},
739+
},
740+
},
741+
},
742+
},
743+
expected_json_urls = {
744+
"http://127.0.0.1:8081/metrics1",
745+
},
746+
expected_prometheus_urls = {
747+
"http://127.0.0.1:8081/metrics2",
748+
},
749+
expected_none_urls = {},
750+
},
751+
},
752+
},
702753
}
703754

704755
for name, case in pairs(test_reapply_delete_cases) do
@@ -858,6 +909,61 @@ local test_reapply_add_cases = {
858909
},
859910
},
860911
},
912+
["listen_global_addr"] = {
913+
apply_cases = {
914+
{
915+
cfg = {
916+
http = {
917+
{
918+
listen = '127.0.0.1:8081',
919+
endpoints = {
920+
{
921+
path = "/metrics1",
922+
format = "prometheus",
923+
},
924+
},
925+
},
926+
},
927+
},
928+
expected_json_urls = {},
929+
expected_prometheus_urls = {
930+
"http://127.0.0.1:8081/metrics1"
931+
},
932+
expected_none_urls = {
933+
"http://127.0.0.1:8081/metrics2",
934+
"http://127.0.0.1:8082/metrics/1",
935+
},
936+
},
937+
{
938+
cfg = {
939+
http = {
940+
{
941+
listen = '0.0.0.0:8081',
942+
endpoints = {
943+
{
944+
path = "/metrics1",
945+
format = "json",
946+
},
947+
{
948+
path = "/metrics2",
949+
format = "prometheus",
950+
},
951+
},
952+
},
953+
},
954+
},
955+
expected_json_urls = {
956+
"http://127.0.0.1:8081/metrics1",
957+
"http://127.1.2.3:8081/metrics1",
958+
},
959+
expected_prometheus_urls = {
960+
"http://127.0.0.1:8081/metrics2",
961+
"http://127.1.2.3:8081/metrics2",
962+
},
963+
expected_none_urls = {},
964+
},
965+
},
966+
},
861967
}
862968

863969
for name, case in pairs(test_reapply_add_cases) do

0 commit comments

Comments
 (0)