From 187165aa3ed5121cf158ac05237f132058de5175 Mon Sep 17 00:00:00 2001 From: Sergey Morozov Date: Fri, 24 Oct 2025 11:10:02 +0300 Subject: [PATCH 1/5] roles: fix redunant server recreation Changing the address during a configuration reload resulted in incorrect assertions, which always recreated the server. After this patch server will recreate itself only in changing a listen parameter. Closes #219 --- CHANGELOG.md | 2 ++ roles/httpd.lua | 2 +- test/integration/httpd_role_test.lua | 16 ++++++++++++++++ test/mocks/mock_role.lua | 10 ++++++++++ 4 files changed, 29 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f81f272..fe9cbe9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## Fixed +- Do not recreate server if it's address and port were not changed (#219). + ## [1.8.0] - 2025-07-07 The release introduces a new log level configuration option in `httpd` role diff --git a/roles/httpd.lua b/roles/httpd.lua index 49ce049..c9b9b88 100644 --- a/roles/httpd.lua +++ b/roles/httpd.lua @@ -121,7 +121,7 @@ local function apply_http(name, node) httpd = httpd, routes = {}, } - elseif servers[name].host ~= host or servers[name].port ~= port then + elseif servers[name].httpd.host ~= host or servers[name].httpd.port ~= port then servers[name].httpd:stop() servers[name].httpd = http_server.new(host, port, parse_params(node)) servers[name].httpd:start() diff --git a/test/integration/httpd_role_test.lua b/test/integration/httpd_role_test.lua index 9875445..d0bb5bd 100644 --- a/test/integration/httpd_role_test.lua +++ b/test/integration/httpd_role_test.lua @@ -172,6 +172,22 @@ g.test_change_server_addr_on_the_run = function(cg) t.assert_equals(resp.body, 'pong') end +g.test_keep_existing_server_routes_on_config_reload = function(cg) + local resp = http_client:get('http://0.0.0.0:13001/ping_once') + t.assert_equals(resp.status, 200, 'response not 200') + t.assert_equals(resp.body, 'pong once') + + local cfg = table.deepcopy(config) + cfg.credentials.users.testguest = { roles = {'super'} } + treegen.write_file(cg.server.chdir, 'config.yaml', yaml.encode(cfg)) + local _, err = cg.server:eval("require('config'):reload()") + t.assert_not(err) + + resp = http_client:get('http://0.0.0.0:13001/ping_once') + t.assert_equals(resp.status, 200, 'response not 200') + t.assert_equals(resp.body, 'pong once') +end + g.test_log_requests = function(cg) t.skip_if(cg.params.use_tls) diff --git a/test/mocks/mock_role.lua b/test/mocks/mock_role.lua index 29409a9..1af004d 100644 --- a/test/mocks/mock_role.lua +++ b/test/mocks/mock_role.lua @@ -1,6 +1,7 @@ local M = {dependencies = { 'roles.httpd' }} local servers = {} +local applied = {} M.validate = function() end @@ -14,6 +15,15 @@ M.apply = function(conf) }, function(tx) return tx:render({text = 'pong'}) end) + + if applied[server.id] == nil then + servers[server.id]:route({ + path = '/ping_once', + }, function(tx) + return tx:render({text = 'pong once'}) + end) + applied[server.id] = {} + end end end end From 0f6156dc7415b78deae946d597a8ccb0d8ef0a97 Mon Sep 17 00:00:00 2001 From: themilchenko Date: Wed, 22 Oct 2025 19:06:06 +0300 Subject: [PATCH 2/5] tests: refactor log test Global log level settings were declared that were only used in one test, but all tests in group were re-run because of it. This patch fixes it and update config with its log level only for this test. --- test/integration/httpd_role_test.lua | 61 +++++++++++++++------------- 1 file changed, 32 insertions(+), 29 deletions(-) diff --git a/test/integration/httpd_role_test.lua b/test/integration/httpd_role_test.lua index d0bb5bd..406122f 100644 --- a/test/integration/httpd_role_test.lua +++ b/test/integration/httpd_role_test.lua @@ -9,7 +9,7 @@ local http_client = require('http.client').new() local helpers = require('test.helpers') -local LOG_LEVEL = { +local LOG_LEVELS = { INFO = 5, VERBOSE = 6, DEBUG = 7, @@ -17,11 +17,6 @@ local LOG_LEVEL = { local g = t.group(nil, t.helpers.matrix({ use_tls = {true, false}, - log_level = { - LOG_LEVEL.INFO, - LOG_LEVEL.VERBOSE, - LOG_LEVEL.DEBUG, - }, })) local ssl_data_dir = fio.abspath(fio.pathjoin(helpers.get_testdir_path(), "ssl_data")) @@ -103,8 +98,6 @@ g.before_each(function(cg) cfg = tls_config end - cfg.log = {level = cg.params.log_level} - local config_file = treegen.write_file(dir, 'config.yaml', yaml.encode(cfg)) local opts = {config_file = config_file, chdir = dir} @@ -188,31 +181,41 @@ g.test_keep_existing_server_routes_on_config_reload = function(cg) t.assert_equals(resp.body, 'pong once') end -g.test_log_requests = function(cg) - t.skip_if(cg.params.use_tls) - - local function make_request(address) - local resp = http_client:get(string.format('http://%s/ping', address)) - t.assert_equals(resp.status, 200, 'response not 200') - end +for log_name, log_lvl in pairs(LOG_LEVELS) do + g.before_test('test_log_requests_' .. string.lower(log_name), function(cg) + local cfg = table.copy(config) + cfg.log = {level = log_lvl} + treegen.write_file(cg.server.chdir, 'config.yaml', yaml.encode(cfg)) + local _, err = cg.server:eval("require('config'):reload()") + t.assert_not(err) + end) + + g['test_log_requests_' .. string.lower(log_name)] = function(cg) + t.skip_if(cg.params.use_tls) + + local function make_request(address) + local resp = http_client:get(string.format('http://%s/ping', address)) + t.assert_equals(resp.status, 200, 'response not 200') + end - local function assert_should_log(expected) - local grep_res = cg.server:grep_log('GET /ping', math.huge) - if expected then - t.assert(grep_res) - else - t.assert_not(grep_res) + local function assert_should_log(expected) + local grep_res = cg.server:grep_log('GET /ping', math.huge) + if expected then + t.assert(grep_res) + else + t.assert_not(grep_res) + end end - end - local log_level = tonumber(cg.params.log_level) + local log_level = tonumber(log_lvl) - make_request('localhost:13002') - assert_should_log(log_level >= LOG_LEVEL.DEBUG) + make_request('localhost:13002') + assert_should_log(log_level >= LOG_LEVELS.DEBUG) - make_request('localhost:13001') - assert_should_log(log_level >= LOG_LEVEL.VERBOSE) + make_request('localhost:13001') + assert_should_log(log_level >= LOG_LEVELS.VERBOSE) - make_request('localhost:13000') - assert_should_log(log_level >= LOG_LEVEL.INFO) + make_request('localhost:13000') + assert_should_log(log_level >= LOG_LEVELS.INFO) + end end From 2479fd334b88a3428c539514704eef591d95af39 Mon Sep 17 00:00:00 2001 From: themilchenko Date: Thu, 23 Oct 2025 12:40:49 +0300 Subject: [PATCH 3/5] tests: refactor tls cases --- test/integration/httpd_role_test.lua | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/test/integration/httpd_role_test.lua b/test/integration/httpd_role_test.lua index 406122f..c0246f2 100644 --- a/test/integration/httpd_role_test.lua +++ b/test/integration/httpd_role_test.lua @@ -135,20 +135,27 @@ g.test_httpd_role_usage = function(cg) end g.test_stop_server_after_remove = function(cg) - local resp = http_client:get('http://localhost:13001/ping') + local protocol = cg.params.use_tls and 'https' or 'http' + + t.assert(helpers.tcp_connection_exists('localhost', 13000)) + local resp = http_client:get(protocol .. '://localhost:13000/ping', { + ca_file = cg.params.use_tls and fio.pathjoin(ssl_data_dir, 'ca.crt'), + }) t.assert_equals(resp.status, 200, 'response not 200') t.assert_equals(resp.body, 'pong') local cfg = table.deepcopy(config) - cfg.groups['group-001'].replicasets['replicaset-001'].roles_cfg['roles.httpd'].additional = nil + cfg.groups['group-001'].replicasets['replicaset-001'].roles_cfg['roles.httpd'].default = nil treegen.write_file(cg.server.chdir, 'config.yaml', yaml.encode(cfg)) local _, err = cg.server:eval("require('config'):reload()") t.assert_not(err) - t.assert_not(helpers.tcp_connection_exists('localhost', 13001)) + t.assert_not(helpers.tcp_connection_exists('localhost', 13000)) end g.test_change_server_addr_on_the_run = function(cg) + t.skip_if(cg.params.use_tls, 'no certs for testing addr') + local resp = http_client:get('http://0.0.0.0:13001/ping') t.assert_equals(resp.status, 200, 'response not 200') t.assert_equals(resp.body, 'pong') @@ -166,7 +173,11 @@ g.test_change_server_addr_on_the_run = function(cg) end g.test_keep_existing_server_routes_on_config_reload = function(cg) - local resp = http_client:get('http://0.0.0.0:13001/ping_once') + local protocol = cg.params.use_tls and 'https' or 'http' + + local resp = http_client:get(protocol .. '://localhost:13000/ping_once', { + ca_file = cg.params.use_tls and fio.pathjoin(ssl_data_dir, 'ca.crt'), + }) t.assert_equals(resp.status, 200, 'response not 200') t.assert_equals(resp.body, 'pong once') @@ -176,7 +187,10 @@ g.test_keep_existing_server_routes_on_config_reload = function(cg) local _, err = cg.server:eval("require('config'):reload()") t.assert_not(err) - resp = http_client:get('http://0.0.0.0:13001/ping_once') + t.assert(helpers.tcp_connection_exists('localhost', 13000)) + resp = http_client:get(protocol .. '://localhost:13000/ping_once', { + ca_file = cg.params.use_tls and fio.pathjoin(ssl_data_dir, 'ca.crt'), + }) t.assert_equals(resp.status, 200, 'response not 200') t.assert_equals(resp.body, 'pong once') end From da357472c55e179d371c60f016a5d0750662927c Mon Sep 17 00:00:00 2001 From: themilchenko Date: Wed, 22 Oct 2025 19:10:13 +0300 Subject: [PATCH 4/5] roles: update opts on config reload There were no way to update config on the run with method `config:reload()` instead of listen parameters. This patch fixes it, all options support config reload. Closes #216 --- CHANGELOG.md | 1 + roles/httpd.lua | 20 +++++++++++++++++--- test/integration/httpd_role_test.lua | 22 ++++++++++++++++++++++ 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe9cbe9..9bd5034 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## Fixed - Do not recreate server if it's address and port were not changed (#219). +- Server doesn't change after updating parameters on config reload (#216). ## [1.8.0] - 2025-07-07 diff --git a/roles/httpd.lua b/roles/httpd.lua index c9b9b88..0fae3e0 100644 --- a/roles/httpd.lua +++ b/roles/httpd.lua @@ -107,23 +107,37 @@ local function parse_params(node) } end +local function server_has_changed(name, node_params, host, port) + if servers[name].httpd.host ~= host or servers[name].httpd.port ~= port then + return true + end + for k in pairs(node_params) do + if k ~= 'listen' and servers[name].httpd.options[k] ~= node_params[k] then + return true + end + end + return false +end + local function apply_http(name, node) local host, port, err = parse_listen(node.listen) if err ~= nil then error("failed to parse URI: " .. err) end + local params = parse_params(node) + if servers[name] == nil then - local httpd = http_server.new(host, port, parse_params(node)) + local httpd = http_server.new(host, port, params) httpd:start() servers[name] = { httpd = httpd, routes = {}, } - elseif servers[name].httpd.host ~= host or servers[name].httpd.port ~= port then + elseif server_has_changed(name, params, host, port) then servers[name].httpd:stop() - servers[name].httpd = http_server.new(host, port, parse_params(node)) + servers[name].httpd = http_server.new(host, port, params) servers[name].httpd:start() end end diff --git a/test/integration/httpd_role_test.lua b/test/integration/httpd_role_test.lua index c0246f2..12f5739 100644 --- a/test/integration/httpd_role_test.lua +++ b/test/integration/httpd_role_test.lua @@ -233,3 +233,25 @@ for log_name, log_lvl in pairs(LOG_LEVELS) do assert_should_log(log_level >= LOG_LEVELS.INFO) end end + +g.test_enable_tls_on_config_reload = function(cg) + -- We should start with no tls firstly. + t.skip_if(cg.params.use_tls) + + local resp = http_client:get('http://localhost:13000/ping') + t.assert_equals(resp.status, 200, 'response not 200') + t.assert_equals(resp.body, 'pong') + + treegen.write_file(cg.server.chdir, 'config.yaml', yaml.encode(tls_config)) + local _, err = cg.server:eval("require('config'):reload()") + t.assert_not(err) + + resp = http_client:get('https://localhost:13000/ping', { + ca_file = fio.pathjoin(ssl_data_dir, 'ca.crt') + }) + t.assert_equals(resp.status, 200, 'response not 200') + t.assert_equals(resp.body, 'pong') + + local resp = http_client:get('http://localhost:13000/ping') + t.assert_equals(resp.status, 444, 'response not 444') +end From b1f18ba9827611251fceebe6b111ca9a5d2d0996 Mon Sep 17 00:00:00 2001 From: themilchenko Date: Thu, 23 Oct 2025 12:01:25 +0300 Subject: [PATCH 5/5] ci: remove packaging for debian-buster Since Debain buster distribution has reached its end-of-life building on it is no longer supported. --- .github/workflows/packaging.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/packaging.yml b/.github/workflows/packaging.yml index 394e184..455e24c 100644 --- a/.github/workflows/packaging.yml +++ b/.github/workflows/packaging.yml @@ -30,7 +30,6 @@ jobs: matrix: platform: - { os: 'debian', dist: 'stretch' } - - { os: 'debian', dist: 'buster' } - { os: 'debian', dist: 'bullseye' } - { os: 'el', dist: '7' } - { os: 'el', dist: '8' }