Skip to content

Commit

Permalink
Refactored fqdn tests by changing expectedIPs to the same type as dns…
Browse files Browse the repository at this point in the history 
…ResponseIPs.

Added 2 more tests.
- existingDNSCache is empty, the new response matches a selector.
- existingDNSCache is empty, the new response doesn't match any selector.
Added nit changes for fqdn.go
Signed-off-by: Hemant <hkbiet@gmail.com>
  • Loading branch information
@hkiiita
hkiiita committed Oct 29, 2024
1 parent a341aa6 commit eb3f4ba
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 51 deletions.
22 changes: 11 additions & 11 deletions pkg/agent/controller/networkpolicy/fqdn.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -423,13 +423,13 @@ func (f *fqdnController) onDNSResponse(
currentTime := time.Now()
ipWithExpirationMap := make(map[string]ipWithExpiration)

// timeToRequery establishes a maximum reference time for tracking the minimum re-query time to DNS, as IPs expire.
timeToRequery := currentTime.Add(24 * time.Hour)
// timeToReQuery establishes a maximum reference time for tracking the minimum re-query time to DNS, as IPs expire.
var timeToReQuery *time.Time

updateMapWithIP := func(ip string, ipMeta ipWithExpiration) {
updateIPWithExpiration := func(ip string, ipMeta ipWithExpiration) {
ipWithExpirationMap[ip] = ipMeta
if ipMeta.expirationTime.Before(timeToRequery) {
timeToRequery = ipMeta.expirationTime
if timeToReQuery == nil || ipMeta.expirationTime.Before(*timeToReQuery) {
timeToReQuery = &ipMeta.expirationTime
}
}

Expand All @@ -440,26 +440,26 @@ func (f *fqdnController) onDNSResponse(
// check for new IPs.
for newIPStr, newIPMeta := range newIPWithExpiration {
if _, exist := cachedDNSMeta.responseIPs[newIPStr]; !exist {
updateMapWithIP(newIPStr, newIPMeta)
updateIPWithExpiration(newIPStr, newIPMeta)
addressUpdate = true
}
}

// check for presence of already cached IPs in the new response.
for cachedIPStr, cachedIPMeta := range cachedDNSMeta.responseIPs {
if newIPMeta, exist := newIPWithExpiration[cachedIPStr]; !exist {
// An already cached IP has been found in new response.
// The IP was not found in current response.
if cachedIPMeta.expirationTime.Before(currentTime) {
// this IP is expired and stale, remove it by not including it but also signal an update to syncRules.
addressUpdate = true
} else {
// It hasn't expired yet, so just retain it with its existing expirationTime.
updateMapWithIP(cachedIPStr, cachedIPMeta)
updateIPWithExpiration(cachedIPStr, cachedIPMeta)
}
} else {
// This already cached IP is part of the current response, so update it with max time between received time and its old cached time.
expTime := laterOf(newIPMeta.expirationTime, cachedIPMeta.expirationTime)
updateMapWithIP(cachedIPStr, ipWithExpiration{
updateIPWithExpiration(cachedIPStr, ipWithExpiration{
ip: cachedIPMeta.ip,
expirationTime: expTime,
})
Expand All @@ -483,7 +483,7 @@ func (f *fqdnController) onDNSResponse(
}
if addToCache {
for ipStr, ipMeta := range newIPWithExpiration {
updateMapWithIP(ipStr, ipMeta)
updateIPWithExpiration(ipStr, ipMeta)
}
addressUpdate = true
}
Expand All @@ -493,7 +493,7 @@ func (f *fqdnController) onDNSResponse(
f.dnsEntryCache[fqdn] = dnsMeta{
responseIPs: ipWithExpirationMap,
}
f.dnsQueryQueue.AddAfter(fqdn, timeToRequery.Sub(currentTime))
f.dnsQueryQueue.AddAfter(fqdn, timeToReQuery.Sub(currentTime))
}

f.syncDirtyRules(fqdn, waitCh, addressUpdate)
Expand Down
114 changes: 74 additions & 40 deletions pkg/agent/controller/networkpolicy/fqdn_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"go.uber.org/mock/gomock"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/util/workqueue"
clocktesting "k8s.io/utils/clock/testing"

"antrea.io/antrea/pkg/agent/config"
openflowtest "antrea.io/antrea/pkg/agent/openflow/testing"
Expand All @@ -39,12 +40,6 @@ func newMockFQDNController(t *testing.T, controller *gomock.Controller, dnsServe
if dnsServer != nil {
dnsServerAddr = *dnsServer
}
mockDnsQueryQueue := workqueue.NewTypedRateLimitingQueueWithConfig(
workqueue.NewTypedItemExponentialFailureRateLimiter[string](1*time.Millisecond, 100*time.Millisecond),
workqueue.TypedRateLimitingQueueConfig[string]{
Name: "fqdn",
},
)
f, err := newFQDNController(
mockOFClient,
newIDAllocator(testAsyncDeleteInterval),
Expand All @@ -54,7 +49,6 @@ func newMockFQDNController(t *testing.T, controller *gomock.Controller, dnsServe
false,
config.DefaultHostGatewayOFPort,
)
f.dnsQueryQueue = mockDnsQueryQueue
require.NoError(t, err)
return f, mockOFClient
}
Expand Down Expand Up @@ -595,18 +589,27 @@ func TestSyncDirtyRules(t *testing.T) {

func TestOnDNSResponse(t *testing.T) {
testFQDN := "fqdn-test-pod.lfx.test"
currentTime := time.Now()
selectorItem1 := fqdnSelectorItem{
matchName: testFQDN,
}
selectorItem2 := fqdnSelectorItem{
matchName: "random-domain.com",
}
fakeClock := clocktesting.NewFakeClock(time.Now())
currentTime := fakeClock.Now()

tests := []struct {
name string
existingDNSCache map[string]dnsMeta
dnsResponseIPs map[string]ipWithExpiration
expectedIPs map[string]time.Time
expectedItem string
name string
existingDNSCache map[string]dnsMeta
dnsResponseIPs map[string]ipWithExpiration
expectedIPs map[string]ipWithExpiration
expectedItem string
mockSelectorToRuleIDs map[fqdnSelectorItem]sets.Set[string]
}{
{
name: "new IP added",
existingDNSCache: map[string]dnsMeta{
"fqdn-test-pod.lfx.test": {
testFQDN: {
responseIPs: map[string]ipWithExpiration{
"192.1.1.1": {ip: net.ParseIP("192.1.1.1"), expirationTime: currentTime.Add(5 * time.Second)},
"192.1.1.2": {ip: net.ParseIP("192.1.1.2"), expirationTime: currentTime.Add(6 * time.Second)},
Expand All @@ -616,10 +619,10 @@ func TestOnDNSResponse(t *testing.T) {
dnsResponseIPs: map[string]ipWithExpiration{
"192.1.1.3": {ip: net.ParseIP("192.1.1.3"), expirationTime: currentTime.Add(10 * time.Second)},
},
expectedIPs: map[string]time.Time{
"192.1.1.1": currentTime.Add(5 * time.Second),
"192.1.1.2": currentTime.Add(6 * time.Second),
"192.1.1.3": currentTime.Add(10 * time.Second),
expectedIPs: map[string]ipWithExpiration{
"192.1.1.1": {ip: net.ParseIP("192.1.1.1"), expirationTime: currentTime.Add(5 * time.Second)},
"192.1.1.2": {ip: net.ParseIP("192.1.1.2"), expirationTime: currentTime.Add(6 * time.Second)},
"192.1.1.3": {ip: net.ParseIP("192.1.1.3"), expirationTime: currentTime.Add(10 * time.Second)},
},
expectedItem: testFQDN,
},
Expand All @@ -634,9 +637,9 @@ func TestOnDNSResponse(t *testing.T) {
},
},
dnsResponseIPs: map[string]ipWithExpiration{},
expectedIPs: map[string]time.Time{
"192.1.1.1": currentTime.Add(5 * time.Second),
"192.1.1.2": currentTime.Add(6 * time.Second),
expectedIPs: map[string]ipWithExpiration{
"192.1.1.1": {ip: net.ParseIP("192.1.1.1"), expirationTime: currentTime.Add(5 * time.Second)},
"192.1.1.2": {ip: net.ParseIP("192.1.1.2"), expirationTime: currentTime.Add(6 * time.Second)},
},
},
{
Expand All @@ -651,8 +654,8 @@ func TestOnDNSResponse(t *testing.T) {
dnsResponseIPs: map[string]ipWithExpiration{
"192.1.1.1": {ip: net.ParseIP("192.1.1.1"), expirationTime: currentTime.Add(10 * time.Second)},
},
expectedIPs: map[string]time.Time{
"192.1.1.1": currentTime.Add(10 * time.Second),
expectedIPs: map[string]ipWithExpiration{
"192.1.1.1": {ip: net.ParseIP("192.1.1.1"), expirationTime: currentTime.Add(10 * time.Second)},
},
},
{
Expand All @@ -665,10 +668,10 @@ func TestOnDNSResponse(t *testing.T) {
},
},
dnsResponseIPs: map[string]ipWithExpiration{
"192.1.1.3": {ip: net.ParseIP("192.1.1.3"), expirationTime: currentTime.Add(10 * time.Second)},
"192.1.1.3": {ip: net.ParseIP("192.1.1.3"), expirationTime: currentTime.Add(5 * time.Second)},
},
expectedIPs: map[string]time.Time{
"192.1.1.3": currentTime.Add(10 * time.Second),
expectedIPs: map[string]ipWithExpiration{
"192.1.1.3": {ip: net.ParseIP("192.1.1.3"), expirationTime: currentTime.Add(5 * time.Second)},
},
expectedItem: testFQDN,
},
Expand All @@ -681,38 +684,69 @@ func TestOnDNSResponse(t *testing.T) {
},
},
},
dnsResponseIPs: map[string]ipWithExpiration{},
expectedIPs: map[string]ipWithExpiration{
"192.1.1.2": {ip: net.ParseIP("192.1.1.2"), expirationTime: currentTime.Add(5 * time.Second)},
},
},
{
name: "existingDNSCache is empty, the new response matches a selector.",
existingDNSCache: map[string]dnsMeta{},
dnsResponseIPs: map[string]ipWithExpiration{
"192.1.1.3": {ip: net.ParseIP("192.1.1.3"), expirationTime: currentTime.Add(10 * time.Second)},
"192.1.1.1": {ip: net.ParseIP("192.1.1.1"), expirationTime: currentTime.Add(5 * time.Second)},
},
expectedIPs: map[string]time.Time{
"192.1.1.2": currentTime.Add(5 * time.Second),
"192.1.1.3": currentTime.Add(10 * time.Second),
expectedIPs: map[string]ipWithExpiration{
"192.1.1.1": {ip: net.ParseIP("192.1.1.1"), expirationTime: currentTime.Add(5 * time.Second)},
},
expectedItem: testFQDN,
mockSelectorToRuleIDs: map[fqdnSelectorItem]sets.Set[string]{
selectorItem1: sets.New[string]("mockRule1"),
},
},
{
name: "existingDNSCache is empty, the new response doesn't match any selector",
existingDNSCache: map[string]dnsMeta{},
dnsResponseIPs: map[string]ipWithExpiration{
"192.1.1.1": {ip: net.ParseIP("192.1.1.1"), expirationTime: currentTime.Add(5 * time.Second)},
},
expectedIPs: map[string]ipWithExpiration{
"192.1.1.1": {ip: net.ParseIP("192.1.1.1"), expirationTime: currentTime.Add(5 * time.Second)},
},
mockSelectorToRuleIDs: map[fqdnSelectorItem]sets.Set[string]{
selectorItem2: sets.New[string]("mockRule2"),
},
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
controller := gomock.NewController(t)
f, _ := newMockFQDNController(t, controller, nil)
mockDNSQueryQueue := workqueue.NewTypedRateLimitingQueueWithConfig(
workqueue.NewTypedItemExponentialFailureRateLimiter[string](minRetryDelay, maxRetryDelay),
workqueue.TypedRateLimitingQueueConfig[string]{
Name: "fqdn-test",
Clock: fakeClock,
},
)
f.dnsQueryQueue = mockDNSQueryQueue
f.dnsEntryCache = tc.existingDNSCache
if tc.mockSelectorToRuleIDs != nil {
f.selectorItemToRuleIDs = tc.mockSelectorToRuleIDs
}

f.onDNSResponse(testFQDN, tc.dnsResponseIPs, nil)

cachedDnsMetaData := f.dnsEntryCache[testFQDN]
assert.Equal(t, len(cachedDnsMetaData.responseIPs), len(tc.expectedIPs), "Expected %d IPs in cache, got %d", len(tc.expectedIPs), len(cachedDnsMetaData.responseIPs))

for ipStr, expectedTTL := range tc.expectedIPs {
cachedIPMeta, exists := cachedDnsMetaData.responseIPs[ipStr]

assert.True(t, exists, "Expected %s to be found in dns cache.", ipStr)
assert.False(t, cachedIPMeta.expirationTime.Before(currentTime), "Expected %s to have a valid TTL, but it has expired.", ipStr)
assert.Equal(t, expectedTTL, cachedIPMeta.expirationTime, "Expected TTL for %s to be %v, got %v", ipStr, expectedTTL, cachedIPMeta.expirationTime)
cachedDnsMetaData, _ := f.dnsEntryCache[testFQDN]

if _, exists := f.selectorItemToRuleIDs[selectorItem2]; !exists {
assert.Equal(t, tc.expectedIPs, cachedDnsMetaData.responseIPs, "Expected %+v in cache, got %+v", tc.expectedIPs, cachedDnsMetaData.responseIPs)
} else {
assert.NotEqual(t, tc.expectedIPs, cachedDnsMetaData.responseIPs, "Did not Expect %+v in cache, got %+v", tc.expectedIPs, cachedDnsMetaData.responseIPs)
}

if tc.expectedItem == testFQDN {
if tc.expectedItem != "" {
fakeClock.Step(5 * time.Second)
actualItem, _ := f.dnsQueryQueue.Get()
f.dnsQueryQueue.Done(actualItem)
assert.Equal(t, tc.expectedItem, actualItem)
Expand Down

0 comments on commit eb3f4ba

Please sign in to comment.