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(dns): flakiness due to hook conflict #13298

Merged
merged 3 commits into from
Jul 1, 2024
Merged

tests(dns): flakiness due to hook conflict #13298

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b632391
tests(dns): flakiness due to hook conflict
StarlightIbuki May 17, 2024
e581c8e
Update spec/01-unit/21-dns-client/02-client_spec.lua
StarlightIbuki Jun 26, 2024
2b6a280
Apply suggestions from code review
StarlightIbuki Jul 1, 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
42 changes: 23 additions & 19 deletions spec/01-unit/21-dns-client/02-client_spec.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,27 +36,32 @@ end

describe("[DNS client]", function()

local client, resolver, query_func
local client, resolver

before_each(function()
client = require("kong.resty.dns.client")
resolver = require("resty.dns.resolver")

-- you can replace this `query_func` upvalue to spy on resolver query calls.
-- `resolver.query_func` is hooked to inspect resolver query calls. New values can be assigned to it.
-- This default will just call the original resolver (hence is transparent)
query_func = function(self, original_query_func, name, options)
resolver.query_func = function(self, original_query_func, name, options)
return original_query_func(self, name, options)
end

-- patch the resolver lib, such that any new resolver created will query
-- using the `query_func` upvalue defined above
-- using the `resolver.query_func` defined above
local old_new = resolver.new
resolver.new = function(...)
local r, err = old_new(...)
if not r then
return nil, err
end
local original_query_func = r.query

-- remember the passed in query_func
-- so it won't be replaced by the next resolver.new call
-- and won't interfere with other tests
local query_func = resolver.query_func
r.query = function(self, ...)
return query_func(self, original_query_func, ...)
end
Expand All @@ -70,7 +75,6 @@ describe("[DNS client]", function()
package.loaded["resty.dns.resolver"] = nil
client = nil
resolver = nil
query_func = nil
end)

describe("initialization", function()
Expand Down Expand Up @@ -583,7 +587,7 @@ describe("[DNS client]", function()
"127.0.0.1 host"
}
}))
query_func = function(self, original_query_func, name, options)
resolver.query_func = function(self, original_query_func, name, options)
-- The first request uses syncQuery not waiting on the
-- aysncQuery timer, so the low-level r:query() could not sleep(5s),
-- it can only sleep(timeout).
Expand Down Expand Up @@ -613,7 +617,7 @@ describe("[DNS client]", function()
-- KAG-2300 - https://github.com/Kong/kong/issues/10182
-- If we encounter a timeout while talking to the DNS server, don't keep trying with other record types
assert(client.init({ timeout = 1000, retrans = 2 }))
query_func = function(self, original_query_func, name, options)
resolver.query_func = function(self, original_query_func, name, options)
if options.qtype == client.TYPE_SRV then
ngx.sleep(10)
else
Expand Down Expand Up @@ -713,7 +717,7 @@ describe("[DNS client]", function()
it("fetching names case insensitive", function()
assert(client.init())

query_func = function(self, original_query_func, name, options)
resolver.query_func = function(self, original_query_func, name, options)
return {
{
name = "some.UPPER.case",
Expand Down Expand Up @@ -901,7 +905,7 @@ describe("[DNS client]", function()
assert(client.init())

local callcount = 0
query_func = function(self, original_query_func, name, options)
resolver.query_func = function(self, original_query_func, name, options)
callcount = callcount + 1
return original_query_func(self, name, options)
end
Expand Down Expand Up @@ -949,7 +953,7 @@ describe("[DNS client]", function()
assert(client.init())

local callcount = 0
query_func = function(self, original_query_func, name, options)
resolver.query_func = function(self, original_query_func, name, options)
callcount = callcount + 1
return original_query_func(self, name, options)
end
Expand Down Expand Up @@ -996,7 +1000,7 @@ describe("[DNS client]", function()
},
}

query_func = function(self, original_query_func, name, options)
resolver.query_func = function(self, original_query_func, name, options)
if name == host and options.qtype == client.TYPE_SRV then
return entry
end
Expand All @@ -1019,7 +1023,7 @@ describe("[DNS client]", function()
"nameserver 198.51.100.0",
},
}))
query_func = function(self, original_query_func, name, opts)
resolver.query_func = function(self, original_query_func, name, opts)
if name ~= "hello.world" and (opts or {}).qtype ~= client.TYPE_CNAME then
return original_query_func(self, name, opts)
end
Expand Down Expand Up @@ -1509,7 +1513,7 @@ describe("[DNS client]", function()
}))

local callcount = 0
query_func = function(self, original_query_func, name, options)
resolver.query_func = function(self, original_query_func, name, options)
callcount = callcount + 1
-- Introducing a simulated network delay ensures individual_toip always
-- triggers a DNS query to avoid it triggering only once due to a cache
Expand Down Expand Up @@ -1575,7 +1579,7 @@ describe("[DNS client]", function()
}))

-- mock query function to return a default record
query_func = function(self, original_query_func, name, options)
resolver.query_func = function(self, original_query_func, name, options)
return {
{
type = client.TYPE_A,
Expand Down Expand Up @@ -1613,7 +1617,7 @@ describe("[DNS client]", function()

-- mock query function to count calls
local call_count = 0
query_func = function(self, original_query_func, name, options)
resolver.query_func = function(self, original_query_func, name, options)
call_count = call_count + 1
return original_query_func(self, name, options)
end
Expand Down Expand Up @@ -1688,7 +1692,7 @@ describe("[DNS client]", function()

-- mock query function to count calls, and return errors
local call_count = 0
query_func = function(self, original_query_func, name, options)
resolver.query_func = function(self, original_query_func, name, options)
call_count = call_count + 1
return { errcode = 5, errstr = "refused" }
end
Expand Down Expand Up @@ -1754,7 +1758,7 @@ describe("[DNS client]", function()
local results = {}

local call_count = 0
query_func = function(self, original_query_func, name, options)
resolver.query_func = function(self, original_query_func, name, options)
call_count = call_count + 1
sleep(0.5) -- make sure we take enough time so the other threads
-- will be waiting behind this one
Expand Down Expand Up @@ -1817,7 +1821,7 @@ describe("[DNS client]", function()

-- insert a stub thats waits and returns a fixed record
local name = TEST_DOMAIN
query_func = function()
resolver.query_func = function()
local ip = ip
local entry = {
{
Expand Down Expand Up @@ -1884,7 +1888,7 @@ describe("[DNS client]", function()
-- insert a stub thats waits and returns a fixed record
local call_count = 0
local name = TEST_DOMAIN
query_func = function()
resolver.query_func = function()
local ip = "1.4.2.3"
local entry = {
{
Expand Down
27 changes: 16 additions & 11 deletions spec/01-unit/21-dns-client/03-client_cache_spec.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,27 +19,32 @@ end

describe("[DNS client cache]", function()

local client, resolver, query_func
local client, resolver

before_each(function()
client = require("kong.resty.dns.client")
resolver = require("resty.dns.resolver")

-- you can replace this `query_func` upvalue to spy on resolver query calls.
-- `resolver.query_func` is hooked to inspect resolver query calls. New values can be assigned to it.
-- This default will just call the original resolver (hence is transparent)
query_func = function(self, original_query_func, name, options)
resolver.query_func = function(self, original_query_func, name, options)
return original_query_func(self, name, options)
end

-- patch the resolver lib, such that any new resolver created will query
-- using the `query_func` upvalue defined above
-- using the `resolver.query_func` defined above
local old_new = resolver.new
resolver.new = function(...)
local r = old_new(...)
local original_query_func = r.query

-- remember the passed in query_func
-- so it won't be replaced by the next resolver.new call
-- and won't interfere with other tests
local query_func = resolver.query_func
r.query = function(self, ...)
if not query_func then
print(debug.traceback("WARNING: query_func is not set"))
if not resolver.query_func then
print(debug.traceback("WARNING: resolver.query_func is not set"))
dump(self, ...)
return
end
Expand All @@ -53,8 +58,8 @@ describe("[DNS client cache]", function()
package.loaded["kong.resty.dns.client"] = nil
package.loaded["resty.dns.resolver"] = nil
client = nil
resolver.query_func = nil
resolver = nil
query_func = nil
end)


Expand All @@ -81,7 +86,7 @@ describe("[DNS client cache]", function()
assert(client.init(config))
lrucache = client.getcache()

query_func = function(self, original_query_func, qname, opts)
resolver.query_func = function(self, original_query_func, qname, opts)
return mock_records[qname..":"..opts.qtype] or { errcode = 3, errstr = "name error" }
end
end)
Expand Down Expand Up @@ -218,7 +223,7 @@ describe("[DNS client cache]", function()
-- the 'result3' resolve call above will also trigger a new background query
-- (because the sleep of 0.1 equals the records ttl of 0.1)
-- so let's yield to activate that background thread now. If not done so,
-- the `after_each` will clear `query_func` and an error will appear on the
-- the `after_each` will clear `resolver.query_func` and an error will appear on the
-- next test after this one that will yield.
sleep(0.1)
end)
Expand Down Expand Up @@ -281,7 +286,7 @@ describe("[DNS client cache]", function()
assert(client.init(config))
lrucache = client.getcache()

query_func = function(self, original_query_func, qname, opts)
resolver.query_func = function(self, original_query_func, qname, opts)
return mock_records[qname..":"..opts.qtype] or { errcode = 3, errstr = "name error" }
end
end)
Expand Down Expand Up @@ -461,7 +466,7 @@ describe("[DNS client cache]", function()
assert(client.init(config))
lrucache = client.getcache()

query_func = function(self, original_query_func, qname, opts)
resolver.query_func = function(self, original_query_func, qname, opts)
return mock_records[qname..":"..opts.qtype] or { errcode = 3, errstr = "name error" }
end
end)
Expand Down
Loading