Skip to content

api: fix address already in use error #36

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- `Address already in use` error on change role's config on the fly (#34).

### Changed

- Unclear error message when `roles.httpd` config is not applied yet (#33).
Expand Down
85 changes: 49 additions & 36 deletions roles/metrics-export.lua
Original file line number Diff line number Diff line change
Expand Up @@ -322,15 +322,35 @@ local function routes_equal(old, new)
return true
end

local function disable_server(name)
local server = (http_servers or {})[name]

if server ~= nil then
if server.httpd_name ~= nil then
for path, _ in pairs(server.routes) do
server.httpd:delete(path)
end
else
if server.httpd.is_run == true then
server.httpd:stop()
end
end

http_servers[name] = nil
end
end

local function apply_http(conf)
local enabled = {}
local listen_servers_to_start = {}
local applied_servers = {}

for _, node in ipairs(conf) do
if #(node.endpoints or {}) > 0 then
local host, port, target
if node.server ~= nil then
target = {
value = node.server,
is_httpd_role = true,
value = 'httpd_' .. node.server,
httpd_name = node.server,
}
elseif node.listen ~= nil then
local err
Expand All @@ -339,23 +359,19 @@ local function apply_http(conf)
error("failed to parse URI: " .. err, 2)
end
target = {
value = node.listen,
is_httpd_role = false,
value = 'listen_' .. host .. ':' .. tostring(port),
}
else
target = {
value = httpd_role.DEFAULT_SERVER_NAME,
is_httpd_role = true,
value = 'httpd_' .. httpd_role.DEFAULT_SERVER_NAME,
httpd_name = httpd_role.DEFAULT_SERVER_NAME,
}
end

http_servers = http_servers or {}
-- Since the 'listen' and 'server' names of other servers in the config may be
-- the same, we create a unique string concatenating the key name and information
-- about whether it is an httpd key or not.
enabled[tostring(target.value) .. tostring(target.is_httpd_role)] = true
applied_servers[target.value] = {}

if http_servers[tostring(target.value) .. tostring(target.is_httpd_role)] == nil then
if http_servers[target.value] == nil then
local httpd
if node.listen ~= nil then
httpd = http_server.new(host, port, {
Expand All @@ -366,23 +382,29 @@ local function apply_http(conf)
ssl_password = node.ssl_password,
ssl_password_file = node.ssl_password_file
})
httpd:start()
else
httpd = httpd_role.get_server(target.value)
httpd = httpd_role.get_server(target.httpd_name)
if httpd == nil then
error(('failed to get server by name %q, check that roles.httpd was' ..
' already applied'):format(target.value))
' already applied'):format(target.httpd_name))
end
end

http_servers[tostring(target.value) .. tostring(target.is_httpd_role)] = {
http_servers[target.value] = {
httpd = httpd,
routes = {},
is_httpd_role = target.is_httpd_role,
httpd_name = target.httpd_name,
}

if node.listen ~= nil then
-- Defer starting a server with `listen` key to not trigger
-- "already in use" error of running servers that should be stopped
-- if it isn't in applying config.
table.insert(listen_servers_to_start, http_servers[target.value])
end
end

local server = http_servers[tostring(target.value) .. tostring(target.is_httpd_role)]
local server = http_servers[target.value]
local httpd = server.httpd
local old_routes = server.routes

Expand Down Expand Up @@ -422,29 +444,20 @@ local function apply_http(conf)
end
end

for target, server in pairs(http_servers) do
if not enabled[target] then
if server.is_httpd_role then
for path, _ in pairs(server.routes) do
server.httpd:delete(path)
end
else
server.httpd:stop()
end
http_servers[target] = nil
for name in pairs(http_servers or {}) do
if applied_servers[name] == nil then
disable_server(name)
end
end

for _, server in pairs(listen_servers_to_start) do
server.httpd:start()
end
end

local function stop_http()
for _, server in pairs(http_servers or {}) do
if server.is_httpd_role then
for path, _ in pairs(server.routes) do
server.httpd:delete(path)
end
else
server.httpd:stop()
end
for name in pairs(http_servers or {}) do
disable_server(name)
end
http_servers = nil
end
Expand Down
102 changes: 102 additions & 0 deletions test/integration/reload_config_test.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
local fio = require('fio')
local yaml = require('yaml')
local socket = require('socket')
local helpers = require('test.helpers')
local server = require('test.helpers.server')

local t = require('luatest')
local g = t.group()

g.before_all(function()
helpers.skip_if_unsupported()
end)

g.before_each(function(cg)
cg.workdir = fio.tempdir()
fio.mktree(cg.workdir)

fio.copytree(".rocks", fio.pathjoin(cg.workdir, ".rocks"))
fio.copytree("roles", fio.pathjoin(cg.workdir, "roles"))
fio.copytree(fio.pathjoin("test", "ssl_data"), fio.pathjoin(cg.workdir, "ssl_data"))
fio.copyfile(fio.pathjoin('test', 'entrypoint', 'config.yaml'), cg.workdir)
end)

g.after_each(function(cg)
cg.server:stop()
fio.rmtree(cg.workdir)
end)

local function is_tcp_connect(host, port)
local tcp = socket.tcp()
tcp:settimeout(0.3)
local ok, _ = tcp:connect(host, port)
tcp:close()

return ok
end

local function change_listen_target_in_config(cg, old_addr, new_addr)
local file = fio.open(fio.pathjoin(cg.workdir, 'config.yaml'), {'O_RDONLY'})
t.assert(file ~= nil)

local cfg = file:read()
file:close()

cfg = yaml.decode(cfg)
local export_instances = cfg.groups['group-001'].replicasets['replicaset-001'].
instances.master.roles_cfg['roles.metrics-export'].http

for i, v in pairs(export_instances) do
if v.listen ~= nil and v.listen == old_addr then
export_instances[i].listen = new_addr
end
end

file = fio.open(fio.pathjoin(cg.workdir, 'config.yaml'), {'O_CREAT', 'O_WRONLY', 'O_TRUNC'}, tonumber('644', 8))
file:write(yaml.encode(cfg))
file:close()
end

g.test_reload_config_update_addr = function(cg)
cg.server = server:new({
config_file = fio.pathjoin(cg.workdir, 'config.yaml'),
chdir = cg.workdir,
alias = 'master',
workdir = cg.workdir,
})

cg.server:start({wait_until_ready = true})

t.assert(is_tcp_connect('127.0.0.1', 8082))
t.assert_not(is_tcp_connect('127.0.0.2', 8082))

change_listen_target_in_config(cg, '127.0.0.1:8082', '0.0.0.0:8082')
cg.server:eval("require('config'):reload()")

t.assert(is_tcp_connect('127.0.0.1', 8082))
t.assert(is_tcp_connect('127.0.0.2', 8082))
t.assert(is_tcp_connect('127.1.2.3', 8082))

change_listen_target_in_config(cg, '0.0.0.0:8082', '127.0.0.1:8082')
cg.server:eval("require('config'):reload()")

t.assert_not(is_tcp_connect('127.0.0.2', 8082))
t.assert(is_tcp_connect('127.0.0.1', 8082))
end

g.test_reload_config_global_addr_conflict = function(cg)
cg.server = server:new({
config_file = fio.pathjoin(cg.workdir, 'config.yaml'),
chdir = cg.workdir,
alias = 'master',
workdir = cg.workdir,
})

cg.server:start({wait_until_ready = true})

change_listen_target_in_config(cg, 8081, '0.0.0.0:8082')
t.assert_error_msg_content_equals(
"Can't create tcp_server: Address already in use",
function() cg.server:eval("require('config'):reload()") end
)
end
106 changes: 106 additions & 0 deletions test/unit/http_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,57 @@ local test_reapply_delete_cases = {
},
},
},
["listen_global_addr"] = {
apply_cases = {
{
cfg = {
http = {
{
listen = '0.0.0.0:8081',
endpoints = {
{
path = "/metrics1",
format = "prometheus",
},
},
},
},
},
expected_json_urls = {},
expected_prometheus_urls = {
"http://127.0.0.1:8081/metrics1",
"http://127.1.2.3:8081/metrics1"
},
expected_none_urls = {},
},
{
cfg = {
http = {
{
listen = '127.0.0.1:8081',
endpoints = {
{
path = "/metrics1",
format = "json",
},
{
path = "/metrics2",
format = "prometheus",
},
},
},
},
},
expected_json_urls = {
"http://127.0.0.1:8081/metrics1",
},
expected_prometheus_urls = {
"http://127.0.0.1:8081/metrics2",
},
expected_none_urls = {},
},
},
},
}

for name, case in pairs(test_reapply_delete_cases) do
Expand Down Expand Up @@ -858,6 +909,61 @@ local test_reapply_add_cases = {
},
},
},
["listen_global_addr"] = {
apply_cases = {
{
cfg = {
http = {
{
listen = '127.0.0.1:8081',
endpoints = {
{
path = "/metrics1",
format = "prometheus",
},
},
},
},
},
expected_json_urls = {},
expected_prometheus_urls = {
"http://127.0.0.1:8081/metrics1"
},
expected_none_urls = {
"http://127.0.0.1:8081/metrics2",
"http://127.0.0.1:8082/metrics1",
},
},
{
cfg = {
http = {
{
listen = '0.0.0.0:8081',
endpoints = {
{
path = "/metrics1",
format = "json",
},
{
path = "/metrics2",
format = "prometheus",
},
},
},
},
},
expected_json_urls = {
"http://127.0.0.1:8081/metrics1",
"http://127.1.2.3:8081/metrics1",
},
expected_prometheus_urls = {
"http://127.0.0.1:8081/metrics2",
"http://127.1.2.3:8081/metrics2",
},
expected_none_urls = {},
},
},
},
}

for name, case in pairs(test_reapply_add_cases) do
Expand Down
Loading