Skip to content

Commit

Permalink
hotfix(upstream) display active targets based on proper definition
Browse files Browse the repository at this point in the history
Previously we defined active upstream targets as any target with
a nonzero weight. This definition was properly implemented, but
caused confusion. As such, we redfine 'active targets' to be the most
recent entry of each nonzero target. This presents a more accurate
picture of the targets currently in use by the balancer.
  • Loading branch information
p0pr0ck5 committed Mar 31, 2017
1 parent 476d6ab commit c3c6eba
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 6 deletions.
12 changes: 12 additions & 0 deletions kong/api/routes/upstreams.lua
Original file line number Diff line number Diff line change
Expand Up @@ -148,15 +148,27 @@ return {
if not found[entry.target] and not ignored[entry.target] then
if entry.weight ~= 0 then
entry.order = nil -- dont show our order key to the client

-- add what we want to send to the client in our array
found_n = found_n + 1
found[found_n] = entry

-- track that we found this host:port so we only show
-- the most recent one (kinda)
found[entry.target] = true

else
ignored[entry.target] = true
end
end
end

-- get rid of the hash elements we tracked
-- so we can send this to the client
for i = 1, found_n do
found[found[i].target] = nil
end

-- for now lets not worry about rolling our own pagination
-- we also end up returning a "backwards" list of targets because
-- of how we sorted- do we care?
Expand Down
26 changes: 20 additions & 6 deletions spec/02-integration/03-admin_api/09-targets_routes_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,9 @@ describe("Admin API", function()
end)

describe("/upstreams/{upstream}/targets/active/", function()
describe("only shows active targets", function()
describe("GET", function()
local upstream_name3 = "example.com"
local api4

before_each(function()
local upstream3 = assert(helpers.dao.upstreams:insert {
Expand Down Expand Up @@ -298,12 +299,24 @@ describe("Admin API", function()
upstream_id = upstream3.id,
})

-- and an insert of a separate active target
-- an insert of a separate active target
assert(helpers.dao.targets:insert {
target = "api-3:80",
weight = 10,
upstream_id = upstream3.id,
})

-- two target inserts (we should only see the last one)
assert(helpers.dao.targets:insert {
target = "api-4:80",
weight = 10,
upstream_id = upstream3.id,
})
api4 = assert(helpers.dao.targets:insert {
target = "api-4:80",
weight = 10,
upstream_id = upstream3.id,
})
end)

it("only shows active targets", function()
Expand All @@ -313,10 +326,11 @@ describe("Admin API", function()
})
assert.response(res).has.status(200)
local json = assert.response(res).has.jsonbody()
assert.equal(2, #json.data)
assert.equal(2, json.total)
assert.equal("api-3:80", json.data[1].target)
assert.equal("api-2:80", json.data[2].target)
assert.equal(3, #json.data)
assert.equal(3, json.total)
assert.equal(api4.id, json.data[1].id)
assert.equal("api-3:80", json.data[2].target)
assert.equal("api-2:80", json.data[3].target)
end)
end)
end)
Expand Down

0 comments on commit c3c6eba

Please sign in to comment.