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.

Sign up for GitHub

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

tests(clustering): dp status ready when use RPC Sync #14035

Merged
merged 13 commits into from
Dec 23, 2024
Merged

tests(clustering): dp status ready when use RPC Sync #14035

Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
b199657
test
lhanjian Dec 18, 2024
afb2218
update
lhanjian Dec 19, 2024
8484c23
update
lhanjian Dec 19, 2024
fea9118
update
lhanjian Dec 19, 2024
03bbae7
update
lhanjian Dec 19, 2024
a5fb458
update
lhanjian Dec 19, 2024
0fa56f8
update
lhanjian Dec 19, 2024
3aed793
update
lhanjian Dec 19, 2024
5e24567
update
lhanjian Dec 20, 2024
ef07a4a
Update spec/02-integration/09-hybrid_mode/11-status_spec.lua
lhanjian Dec 20, 2024
3aa41c2
update
lhanjian Dec 20, 2024
26dc2a4
update
lhanjian Dec 20, 2024
b03b074
update
lhanjian Dec 20, 2024
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
58 changes: 52 additions & 6 deletions spec/02-integration/09-hybrid_mode/11-status_spec.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,6 @@ for _, strategy in helpers.each_strategy() do
end)

describe("dp status ready endpoint for no config", function()
chronolaw marked this conversation as resolved.
Show resolved Hide resolved
-- XXX FIXME
local skip_rpc_sync = rpc_sync == "on" and pending or it

lazy_setup(function()
assert(start_kong_cp())
assert(start_kong_dp())
Expand Down Expand Up @@ -107,8 +104,8 @@ for _, strategy in helpers.each_strategy() do
end)

-- now dp receive config from cp, so dp should be ready

skip_rpc_sync("should return 200 on data plane after configuring", function()
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that you misunderstand my words, we still need this case for rpc_sync = off but not use skip_rpc_sync, it should be a normal 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.

fixed

Copy link
Contributor

Choose a reason for hiding this comment

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

It should also support {"off", "off"}

Copy link
Contributor

@chobits chobits Dec 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

note that "off" "off" has been supported by original test case, see Line 107,
I think we should write a completely new on on case to test incremental sync mode, like

if off off
-- using original cases
end

if on on then
-- write new cases in this pr
describe / it ()
....
end
end

So the new case does not need to support on

Copy link
Contributor

@chobits chobits Dec 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

because the original case (off off) will restart CP, if we implement new case in this scenario, current CP/DP incremental sync is hard to make the case stable, it is easy to get flakiness.

Why?

because restarting CP will cause DP connecting CP and introduce some delay, and currently in incremental sync feature, we could not have a good way to ensure the established connection between CP and DP. I think if we want to test CP/DP restart , we could file a new kag to track.

local it_rpc_sync_off= rpc_sync == "off" and it or pending
it_rpc_sync_off("should return 200 on data plane after configuring", function()
helpers.wait_until(function()
local http_client = helpers.http_client('127.0.0.1', dp_status_port)

Expand Down Expand Up @@ -160,10 +157,59 @@ for _, strategy in helpers.each_strategy() do
return true
end
end, 10)

end)
end)

local describe_rpc_sync_on = rpc == "on" and rpc_sync == "on" and describe or pending
describe_rpc_sync_on("dp status ready when rpc_sync == on", function()
lazy_setup(function()
assert(start_kong_cp())
assert(start_kong_dp())
end)

lazy_teardown(function()
assert(helpers.stop_kong("serve_cp"))
assert(helpers.stop_kong("serve_dp"))
end)

it("should return 200 on data plane after configuring when rpc_sync == on", function()
-- insert one entity to make dp ready for incremental sync
lhanjian marked this conversation as resolved.
Show resolved Hide resolved

local http_client = helpers.http_client('127.0.0.1', dp_status_port)

local res = http_client:send({
method = "GET",
path = "/status/ready",
})
http_client:close()
assert.equal(503, res.status)

local admin_client = helpers.admin_client(10000)
lhanjian marked this conversation as resolved.
Show resolved Hide resolved
local res = assert(admin_client:post("/services", {
body = { name = "service-001", url = "https://127.0.0.1:15556/request", },
headers = {["Content-Type"] = "application/json"}
}))
assert.res_status(201, res)

admin_client:close()

helpers.wait_until(function()
local http_client = helpers.http_client('127.0.0.1', dp_status_port)

local res = http_client:send({
method = "GET",
path = "/status/ready",
})

local status = res and res.status
http_client:close()

if status == 200 then
return true
end
end, 10)
end)
end)
end)
end -- for _, strategy
end -- for rpc_sync
Loading