-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
feat(core) upstreams #1735
feat(core) upstreams #1735
Changes from 71 commits
2b9486d
42c73b0
6c3ab98
67431fd
03693df
5f60bf0
2927eda
9916ef4
1a4d03b
ee654dd
42277d1
cc1c446
3fc057f
19ba990
ddc7230
e089725
04d1489
4771514
39e646c
c5f51e0
50d1636
b014ab3
3c59cb7
5d38d4c
913665d
b745277
f633cac
153a028
f06576b
6e3e3d7
85c0c43
f2469c7
4d93ede
6565d3f
b138dc2
c085643
ece41c7
773afac
6d9194f
e6ebd98
fa11a9c
4d9921e
2ce14ae
03f6f36
1750a4e
aa2012d
cefdd66
cd44bb7
6ae91a5
85f42d0
6641dde
5fd062a
67892b8
b4c622d
b5e2c92
e3161ce
bd6eda9
5ef2106
5c7c730
c71123a
3aced2d
8364662
8f75266
c62aa12
603ba29
b2e3fbc
8861044
c0434bd
850e1fd
14525e7
cc8d6f4
27b87d2
47c640c
aa0139d
d6a8664
0d642fd
7363532
32ffda4
fa0d43e
25ef72b
4f19d2a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,5 +11,5 @@ dir='doc' | |
--readme='readme.md' | ||
sort=true | ||
sort_modules=true | ||
not_luadoc=true | ||
--not_luadoc=true | ||
all=false |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
local crud = require "kong.api.crud_helpers" | ||
|
||
return { | ||
["/upstreams/"] = { | ||
GET = function(self, dao_factory) | ||
crud.paginated_set(self, dao_factory.upstreams) | ||
end, | ||
|
||
PUT = function(self, dao_factory) | ||
crud.put(self.params, dao_factory.upstreams) | ||
end, | ||
|
||
POST = function(self, dao_factory, helpers) | ||
crud.post(self.params, dao_factory.upstreams) | ||
end | ||
}, | ||
|
||
["/upstreams/:name_or_id"] = { | ||
before = function(self, dao_factory, helpers) | ||
crud.find_upstream_by_name_or_id(self, dao_factory, helpers) | ||
end, | ||
|
||
GET = function(self, dao_factory, helpers) | ||
return helpers.responses.send_HTTP_OK(self.upstream) | ||
end, | ||
|
||
PATCH = function(self, dao_factory) | ||
crud.patch(self.params, dao_factory.upstreams, self.upstream) | ||
end, | ||
|
||
DELETE = function(self, dao_factory) | ||
crud.delete(self.upstream, dao_factory.upstreams) | ||
end | ||
}, | ||
|
||
["/upstreams/:name_or_id/targets/"] = { | ||
before = function(self, dao_factory, helpers) | ||
crud.find_upstream_by_name_or_id(self, dao_factory, helpers) | ||
self.params.upstream_id = self.upstream.id | ||
end, | ||
|
||
GET = function(self, dao_factory) | ||
crud.paginated_set(self, dao_factory.targets) | ||
end, | ||
|
||
POST = function(self, dao_factory, helpers) | ||
local cleanup_factor = 10 -- when to cleanup; invalid-entries > (valid-ones * cleanup_factor) | ||
|
||
--cleaning up history, check if it's necessary... | ||
local target_history = dao_factory.targets:find_all( | ||
{ upstream_id = self.params.upstream_id }) | ||
if target_history then --ignoring errors here, will be caught when posting below | ||
-- sort the targets | ||
for _,target in ipairs(target_history) do | ||
target.order = target.created_at..":"..target.id | ||
end | ||
-- sort table in reverse order | ||
table.sort(target_history, function(a,b) return a.order>b.order end) | ||
-- do clean up | ||
local cleaned = {} | ||
local delete = {} | ||
for _, entry in ipairs(target_history) do | ||
if cleaned[entry.target] then | ||
-- we got a newer entry for this target than this, so this one can go | ||
delete[#delete+1] = entry | ||
else | ||
-- haven't got this one, so this is the last one for this target | ||
cleaned[entry.target] = true | ||
cleaned[#cleaned+1] = entry | ||
if entry.weight == 0 then | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: so setting an upstream's wight to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes. On the balancer object it gets deleted. But because we need individual Kong nodes to retain the same order of slots in the balancer, we must still record setting it to 0. This way we can replay changes on each node and end up with the same balancer config. |
||
delete[#delete+1] = entry | ||
end | ||
end | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style/nitpick: feel free to use more space to make the code more readable. Line jumps, for example, could be inserted at many places in this snippet. A good practice I have recently found myself to be kind of is inserting a blank lines before There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At some point, I believe we should formalize the code-style used in this project. |
||
|
||
-- do we need to cleanup? | ||
-- either nothing left, or when 10x more outdated than active entries | ||
if (#cleaned == 0 and #delete > 0) or | ||
(#delete >= (math.max(#cleaned,1)*cleanup_factor)) then | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: could use some spacing as well... |
||
ngx.log(ngx.WARN, "Starting cleanup of target table for upstream "..tostring(self.params.upstream_id)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
local cnt = 0 | ||
for _, entry in ipairs(delete) do | ||
-- not sending update events, one event at the end, based on the | ||
-- post of the new entry should suffice to reload only once | ||
dao_factory.targets:delete( | ||
{ id = entry.id }, | ||
{ quiet = true } | ||
) | ||
-- ignoring errors here, deleted by id, so should not matter | ||
-- in case another kong-node does the same cleanup simultaneously | ||
cnt = cnt + 1 | ||
end | ||
ngx.log(ngx.WARN, "Finished cleanup of target table for upstream ".. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto:
|
||
tostring(self.params.upstream_id).." removed "..tostring(cnt).." target entries") | ||
end | ||
end | ||
|
||
crud.post(self.params, dao_factory.targets) | ||
end, | ||
}, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: in your comments, it is very difficult to understand what you mean at the first glance at it because you use semi-colons
;
instead of colons:
.Semi-colons are meant to separate two independent clauses in a sentence. Colons, on the other side, are used to present an explanation after what could stand as an independent clause. That is, the later should be used when you make a statement, and present the explanation for that statement. Not the former.
I know this can sound like a nitpick but believe me: many times did I found myself re-reading your comments because I did not understand the second clause was the explanation for the first one.