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

Author: themilchenko

CHANGELOG.md

@@ -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).

roles/metrics-export.lua

@@ -329,8 +329,8 @@ local function apply_http(conf)
             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
@@ -339,23 +339,38 @@ 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
+            -- the same, we create a unique string concatenating the key (which is listen or server)
+            -- and its value.
+            enabled[target.value] = true
+
+            if http_servers[target.value] == nil then
+                if target.httpd_name == nil then
+                    if host == '0.0.0.0' then
+                        for k in pairs(http_servers) do
+                            if string.find(k, ':' .. port) ~= nil then
+                                http_servers[k].httpd:stop()
+                                enabled[k] = false
+                            end
+                        end
+                    elseif http_servers['listen_0.0.0.0:' .. port] ~= nil then
+                        local key = 'listen_0.0.0.0:' .. port
+                        http_servers[key].httpd:stop()
+                        enabled[key] = false
+                    end
+                end
 
-            if http_servers[tostring(target.value) .. tostring(target.is_httpd_role)] == nil then
                 local httpd
                 if node.listen ~= nil then
                     httpd = http_server.new(host, port, {
@@ -368,21 +383,21 @@ local function apply_http(conf)
                     })
                     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,
                 }
             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
 
@@ -422,14 +437,16 @@ local function apply_http(conf)
         end
     end
 
-    for target, server in pairs(http_servers) do
+    for target, server in pairs(http_servers or {}) do
         if not enabled[target] then
-            if server.is_httpd_role then
+            if server.httpd_name ~= nil then
                 for path, _ in pairs(server.routes) do
                     server.httpd:delete(path)
                 end
             else
-                server.httpd:stop()
+                if server.httpd.is_run == true then
+                    server.httpd:stop()
+                end
             end
             http_servers[target] = nil
         end
@@ -438,12 +455,14 @@ end
 
 local function stop_http()
     for _, server in pairs(http_servers or {}) do
-        if server.is_httpd_role then
+        if server.httpd_name ~= nil then
             for path, _ in pairs(server.routes) do
                 server.httpd:delete(path)
             end
         else
-            server.httpd:stop()
+            if server.httpd.is_run == true then
+                server.httpd:stop()
+            end
         end
     end
     http_servers = nil

test/integration/reload_config_test.lua

@@ -0,0 +1,88 @@
+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 = 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})
+
+    local file = fio.open(fio.pathjoin(cg.workdir, 'config.yaml'), {'O_RDONLY'})
+    t.assert(file ~= nil)
+
+    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