Skip to content
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

refactor(clustering) dpcp config clean #8880

Closed
wants to merge 68 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
68 commits
Select commit Hold shift + click to select a range
31152d1
update_config.lua
chronolaw May 26, 2022
283a966
use new update_config
chronolaw May 26, 2022
2595d8e
add update_config to rockspec
chronolaw May 26, 2022
3179b83
remove duplicated code
chronolaw May 26, 2022
dd506af
fix calculate_config_hash test
chronolaw May 26, 2022
cb924a0
calculate_config_hash in cp
chronolaw May 26, 2022
a1f148d
clean extract_major_minor
chronolaw May 26, 2022
8fa4a6a
to_sort_string style clean
chronolaw May 26, 2022
63b3edd
style clean
chronolaw May 26, 2022
0f1e9f0
remove oop derived
chronolaw May 26, 2022
65a3f9a
add parent fields
chronolaw May 26, 2022
c122969
add plugins_list field
chronolaw May 26, 2022
2284eb2
adjust plugins_list
chronolaw May 26, 2022
8c4fb09
local plugins_list
chronolaw May 26, 2022
6a48b69
renamge parent to initer
chronolaw May 26, 2022
68925f9
change MT to _MT
chronolaw May 26, 2022
3ec4be2
code clean
chronolaw May 26, 2022
8709d5d
use utils.validate_cert
chronolaw May 27, 2022
486c70f
luacheck clean
chronolaw May 27, 2022
b8ed36f
plugins_list_to_map clean
chronolaw May 27, 2022
61075ae
use utils.validate_connection_certs
chronolaw May 27, 2022
c060981
clean check_for_revocation_status
chronolaw May 27, 2022
66abd81
luacheck clean
chronolaw May 27, 2022
21f79b9
clean check_version_compatibility
chronolaw May 27, 2022
3b04364
clean check_configuration_compatibility
chronolaw May 27, 2022
16dee27
rename clustering_utils to _M
chronolaw May 27, 2022
4da6404
remove param initer
chronolaw May 27, 2022
0a8bf89
typo fix
chronolaw May 27, 2022
de66e7a
change to config_helper
chronolaw May 27, 2022
c5be4cf
add missing rockspec
chronolaw May 27, 2022
d5436d7
fix unit test
chronolaw May 27, 2022
e4b74d7
cp code clean
chronolaw May 27, 2022
9d4169a
add check_configuration_compatibility
chronolaw May 27, 2022
ea83d84
small style clean
chronolaw May 30, 2022
5eba885
add check protocol support
chronolaw May 30, 2022
bd244b6
try fix conflict
chronolaw May 30, 2022
329688e
try fix conflict
chronolaw May 30, 2022
4b8895d
merge chore/dpcp_config_clean
chronolaw May 30, 2022
69e1204
fix merge error
chronolaw May 30, 2022
66d10d7
move check_protocol_support to utils
chronolaw May 30, 2022
7affc3e
lua lint
chronolaw May 30, 2022
07fd555
code clean in init.lua
chronolaw May 31, 2022
79a275d
fix init cp/dp worker
chronolaw May 31, 2022
9a2bc91
clean init_dp_worker
chronolaw May 31, 2022
031c18e
change check_protocol logic
chronolaw May 31, 2022
310a3b5
fix init_dp_worker
chronolaw May 31, 2022
18f7fb5
move ws_client to utils
chronolaw May 31, 2022
36e6540
move wrpc ws_client to utils
chronolaw May 31, 2022
69a003d
clean control_plane
chronolaw May 31, 2022
7c98016
remove useless comments
chronolaw May 31, 2022
e4c3543
simplify usage of plugins_list
chronolaw May 31, 2022
4f0adb1
remove version_handshake
chronolaw May 31, 2022
a615cbb
fix plugins_list in cp
chronolaw May 31, 2022
0f8a2eb
remove version_negotiation code
chronolaw May 31, 2022
436cb82
localize extract_major_minor
chronolaw May 31, 2022
5a25d4d
localize check_kong_version_compatibility
chronolaw May 31, 2022
e9cb7d1
fix merge conflict
chronolaw May 31, 2022
0155ea8
clean ws_server
chronolaw May 31, 2022
fe2d337
clean cp ws server
chronolaw May 31, 2022
de945de
clean cp ws server
chronolaw May 31, 2022
288e667
local validate_connection_certs
chronolaw May 31, 2022
6d5aa75
clean connect_dp
chronolaw May 31, 2022
cddc8e2
fix table insert in cp
chronolaw May 31, 2022
4ea4949
clean init.lua
chronolaw Jun 1, 2022
092ad86
require style fix
chronolaw Jun 1, 2022
6ea52d0
fix merge conflict
chronolaw Jun 3, 2022
c5bb0cd
fix conflict
chronolaw Jun 3, 2022
7287710
yield in to_sorted_string
chronolaw Jun 3, 2022
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
4 changes: 1 addition & 3 deletions kong-2.8.0-0.rockspec
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,13 @@ build = {
["kong.conf_loader.listeners"] = "kong/conf_loader/listeners.lua",

["kong.clustering"] = "kong/clustering/init.lua",
["kong.clustering.version_negotiation"] = "kong/clustering/version_negotiation/init.lua",
["kong.clustering.version_negotiation.services_known"] = "kong/clustering/version_negotiation/services_known.lua",
["kong.clustering.version_negotiation.services_requested"] = "kong/clustering/version_negotiation/services_requested.lua",
Comment on lines -68 to -70
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when is this coming back?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without these, this PR is doing more than a refactor.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After this PR, we will try another method to implement similar feature.

["kong.clustering.data_plane"] = "kong/clustering/data_plane.lua",
["kong.clustering.control_plane"] = "kong/clustering/control_plane.lua",
["kong.clustering.wrpc_data_plane"] = "kong/clustering/wrpc_data_plane.lua",
["kong.clustering.wrpc_control_plane"] = "kong/clustering/wrpc_control_plane.lua",
["kong.clustering.utils"] = "kong/clustering/utils.lua",
["kong.clustering.compat.removed_fields"] = "kong/clustering/compat/removed_fields.lua",
["kong.clustering.config_helper"] = "kong/clustering/config_helper.lua",

["kong.cluster_events"] = "kong/cluster_events/init.lua",
["kong.cluster_events.strategies.cassandra"] = "kong/cluster_events/strategies/cassandra.lua",
Expand Down
234 changes: 234 additions & 0 deletions kong/clustering/config_helper.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,234 @@
local constants = require("kong.constants")
kikito marked this conversation as resolved.
Show resolved Hide resolved
local declarative = require("kong.db.declarative")

local tostring = tostring
local assert = assert
local type = type
local error = error
local pairs = pairs
local ipairs = ipairs
local concat = table.concat
local sort = table.sort

local isempty = require("table.isempty")
local isarray = require("table.isarray")
local nkeys = require("table.nkeys")
local new_tab = require("table.new")

local yield = require("kong.tools.utils").yield

local ngx_log = ngx.log
local ngx_null = ngx.null
local ngx_md5 = ngx.md5
local ngx_md5_bin = ngx.md5_bin

local ngx_DEBUG = ngx.DEBUG

local DECLARATIVE_EMPTY_CONFIG_HASH = constants.DECLARATIVE_EMPTY_CONFIG_HASH
local _log_prefix = "[clustering] "


local _M = {}


local function to_sorted_string(value)
yield(true)

if value == ngx_null then
return "/null/"
end

local t = type(value)
if t == "string" or t == "number" then
return value
end

if t == "boolean" then
return tostring(value)
end

if t == "table" then
if isempty(value) then
return "{}"
end

if isarray(value) then
local count = #value
if count == 1 then
return to_sorted_string(value[1])
end

if count == 2 then
return to_sorted_string(value[1]) .. ";" ..
to_sorted_string(value[2])
end

if count == 3 then
return to_sorted_string(value[1]) .. ";" ..
to_sorted_string(value[2]) .. ";" ..
to_sorted_string(value[3])
end

if count == 4 then
return to_sorted_string(value[1]) .. ";" ..
to_sorted_string(value[2]) .. ";" ..
to_sorted_string(value[3]) .. ";" ..
to_sorted_string(value[4])
end

if count == 5 then
return to_sorted_string(value[1]) .. ";" ..
to_sorted_string(value[2]) .. ";" ..
to_sorted_string(value[3]) .. ";" ..
to_sorted_string(value[4]) .. ";" ..
to_sorted_string(value[5])
end

local i = 0
local o = new_tab(count < 100 and count or 100, 0)
for j = 1, count do
i = i + 1
o[i] = to_sorted_string(value[j])

if j % 100 == 0 then
i = 1
o[i] = ngx_md5_bin(concat(o, ";", 1, 100))
end
Comment on lines +93 to +96
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are there plans to fix this off-by-one error?

on every j==n*100, replaces all values in o with a hash of the first 100 and sets i to 1, so the next value (number 101) will go on o[2]. So when j==200, there will be 101 values in o (the last hash plus 100 values more) but it will again use only 100 values for the next hash, so it drops the hashes values of items number 201, 301, 401.... etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems true. Shall we fix it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we can use local i = 1 and new_tab(count < 100 and count +1 or 101, 0)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we fix it?

I don't know. mentioned it just that it can be discussed. probably in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think perhaps we can not change it because of the compatibility of old version.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the hash method has changed several times already. there's no compatibility issue

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's fix the off-by-one error on this PR then then. Another PR will make everything take even longer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will force us to modify all the hashes on the test, which is a pain.

New directive: Do it only if it can be done quickly on this PR. We have lived with the problem so far.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can do it later, now we should finish refactoring first.

end

return ngx_md5_bin(concat(o, ";", 1, i))

else
local count = nkeys(value)
local keys = new_tab(count, 0)
local i = 0
for k in pairs(value) do
i = i + 1
keys[i] = k
end

sort(keys)

local o = new_tab(count, 0)
for i = 1, count do
o[i] = keys[i] .. ":" .. to_sorted_string(value[keys[i]])
end

value = concat(o, ";", 1, count)

return #value > 512 and ngx_md5_bin(value) or value
end -- isarray

end -- table

error("invalid type to be sorted (JSON types are supported)")
end

local function calculate_config_hash(config_table)
if type(config_table) ~= "table" then
local config_hash = ngx_md5(to_sorted_string(config_table))
return config_hash, { config = config_hash, }
end

local routes = config_table.routes
local services = config_table.services
local plugins = config_table.plugins
local upstreams = config_table.upstreams
local targets = config_table.targets

local routes_hash = routes and ngx_md5(to_sorted_string(routes)) or DECLARATIVE_EMPTY_CONFIG_HASH
local services_hash = services and ngx_md5(to_sorted_string(services)) or DECLARATIVE_EMPTY_CONFIG_HASH
local plugins_hash = plugins and ngx_md5(to_sorted_string(plugins)) or DECLARATIVE_EMPTY_CONFIG_HASH
local upstreams_hash = upstreams and ngx_md5(to_sorted_string(upstreams)) or DECLARATIVE_EMPTY_CONFIG_HASH
local targets_hash = targets and ngx_md5(to_sorted_string(targets)) or DECLARATIVE_EMPTY_CONFIG_HASH

config_table.routes = nil
config_table.services = nil
config_table.plugins = nil
config_table.upstreams = nil
config_table.targets = nil

local config_hash = ngx_md5(to_sorted_string(config_table) .. routes_hash
.. services_hash
.. plugins_hash
.. upstreams_hash
.. targets_hash)

config_table.routes = routes
config_table.services = services
config_table.plugins = plugins
config_table.upstreams = upstreams
config_table.targets = targets

return config_hash, {
config = config_hash,
routes = routes_hash,
services = services_hash,
plugins = plugins_hash,
upstreams = upstreams_hash,
targets = targets_hash,
}
end

local hash_fields = {
"config",
"routes",
"services",
"plugins",
"upstreams",
"targets",
}

local function fill_empty_hashes(hashes)
for _, field_name in ipairs(hash_fields) do
hashes[field_name] = hashes[field_name] or DECLARATIVE_EMPTY_CONFIG_HASH
end
end

function _M.update(declarative_config, config_table, config_hash, hashes)
assert(type(config_table) == "table")

if not config_hash then
config_hash, hashes = calculate_config_hash(config_table)
end

if hashes then
fill_empty_hashes(hashes)
end

local current_hash = declarative.get_current_hash()
if current_hash == config_hash then
ngx_log(ngx_DEBUG, _log_prefix, "same config received from control plane, ",
"no need to reload")
return true
end

local entities, err, _, meta, new_hash =
declarative_config:parse_table(config_table, config_hash)
if not entities then
return nil, "bad config received from control plane " .. err
end

if current_hash == new_hash then
ngx_log(ngx_DEBUG, _log_prefix, "same config received from control plane, ",
"no need to reload")
return true
end

-- NOTE: no worker mutex needed as this code can only be
-- executed by worker 0

local res
res, err = declarative.load_into_cache_with_events(entities, meta, new_hash, hashes)
if not res then
return nil, err
end

return true
end


_M.calculate_config_hash = calculate_config_hash


return _M
Loading