Skip to content

Commit

Permalink
Remove time-sensitive setup and assertions from TestDNSResolverCache
Browse files Browse the repository at this point in the history
Instead of relying on log messages, this hooks into the DNS resolver,
and tracks how many times a resolution attempt was made after that.
This should be more reliable, and avoid the recent flakiness we've seen
with this test on Windows[1], even after #1974:

scheduler_ext_test.go:1130:
        Error Trace:    D:/a/k6/k6/execution/scheduler_ext_test.go:1130
        Error:          Should be true
        Test:           TestDNSResolver/cache/3s
        Messages:       expected error to contain one of the list of messages

[1]: https://github.com/grafana/k6/actions/runs/5047828671/jobs/9071063454
  • Loading branch information
Ivan Mirić committed May 24, 2023
1 parent a2f311d commit 1bd7109
Showing 1 changed file with 36 additions and 33 deletions.
69 changes: 36 additions & 33 deletions execution/scheduler_ext_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"net"
"net/url"
"reflect"
"strings"
"sync/atomic"
"testing"
"time"
Expand Down Expand Up @@ -1046,32 +1045,43 @@ func TestDNSResolverCache(t *testing.T) {
}`)

testCases := map[string]struct {
opts lib.Options
expLogEntries int
opts lib.Options
// Amount of expected resolutions after the IP change.
// This will demonstrate how the cache is being used, depending on the
// configured TTL.
expNewResolutions uint32
}{
"default": { // IPs are cached for 5m
lib.Options{DNS: types.DefaultDNSConfig()}, 0,
"default": {
// IPs are cached for 5m, so no new resolution attempts will be made.
opts: lib.Options{DNS: types.DefaultDNSConfig()},
expNewResolutions: 0,
},
"0": { // cache is disabled, every request does a DNS lookup
lib.Options{DNS: types.DNSConfig{
"0ok": {
// The cache is disabled, so every request does a DNS lookup.
opts: lib.Options{DNS: types.DNSConfig{
TTL: null.StringFrom("0"),
Select: types.NullDNSSelect{DNSSelect: types.DNSfirst, Valid: true},
Policy: types.NullDNSPolicy{DNSPolicy: types.DNSpreferIPv4, Valid: false},
}}, 5,
}},
expNewResolutions: 7,
},
"1000": { // cache IPs for 1s, check that unitless values are interpreted as ms
lib.Options{DNS: types.DNSConfig{
TTL: null.StringFrom("1000"),
"1700": {
// IPs are cached for 1.7s.
// This also checks that unitless values are interpreted as ms.
opts: lib.Options{DNS: types.DNSConfig{
TTL: null.StringFrom("1700"),
Select: types.NullDNSSelect{DNSSelect: types.DNSfirst, Valid: true},
Policy: types.NullDNSPolicy{DNSPolicy: types.DNSpreferIPv4, Valid: false},
}}, 4,
}},
expNewResolutions: 2,
},
"3s": {
lib.Options{DNS: types.DNSConfig{
opts: lib.Options{DNS: types.DNSConfig{
TTL: null.StringFrom("3s"),
Select: types.NullDNSSelect{DNSSelect: types.DNSfirst, Valid: true},
Policy: types.NullDNSPolicy{DNSPolicy: types.DNSpreferIPv4, Valid: false},
}}, 3,
}},
expNewResolutions: 1,
},
}

Expand All @@ -1097,36 +1107,29 @@ func TestDNSResolverCache(t *testing.T) {
}, nil)
require.NoError(t, err)

mr := mockresolver.New(nil)
mr := mockresolver.New(map[string][]net.IP{"myhost": {net.ParseIP(sr("HTTPBIN_IP"))}})
var newResolutions uint32
resolveHook := func(host string, result []net.IP) {
require.Len(t, result, 1)
if result[0].String() == "127.0.0.1" {
mr.Set("myhost", "127.0.0.254")
} else {
atomic.AddUint32(&newResolutions, 1)
}
}
mr.ResolveHook = resolveHook
runner.ActualResolver = mr.LookupIPAll

ctx, cancel, execScheduler, samples := newTestScheduler(t, runner, logger, tc.opts)
defer cancel()

mr.Set("myhost", sr("HTTPBIN_IP"))
time.AfterFunc(1700*time.Millisecond, func() {
mr.Set("myhost", "127.0.0.254")
})
defer mr.Unset("myhost")

errCh := make(chan error, 1)
go func() { errCh <- execScheduler.Run(ctx, ctx, samples) }()

select {
case err := <-errCh:
require.NoError(t, err)
entries := logHook.Drain()
require.Len(t, entries, tc.expLogEntries)
for _, entry := range entries {
require.IsType(t, &url.Error{}, entry.Data["error"])
err, ok := entry.Data["error"].(*url.Error)
assert.True(t, ok)
errMsg := err.Error()
contains := strings.Contains(errMsg, "connection refused") ||
strings.Contains(errMsg, "request timeout") ||
strings.Contains(errMsg, "dial: i/o timeout")
assert.Truef(t, contains, "expected error to contain one of the list of messages")
}
assert.Equal(t, tc.expNewResolutions, atomic.LoadUint32(&newResolutions))
case <-time.After(10 * time.Second):
t.Fatal("timed out")
}
Expand Down

0 comments on commit 1bd7109

Please sign in to comment.