Skip to content

Commit 9bd39b4

Browse files
fra98fra98
committed
fixup! test: ipam sync routine unit tests
1 parent a016151 commit 9bd39b4

File tree

2 files changed

+42
-28
lines changed

2 files changed

+42
-28
lines changed

‎pkg/ipam/sync.go‎

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,20 +55,27 @@ func (lipam *LiqoIPAM) sync(ctx context.Context, syncFrequency time.Duration) {
5555

5656
func (lipam *LiqoIPAM) syncNetworks(ctx context.Context, expiredThreshold time.Time) error {
5757
// List all networks present in the cluster.
58-
nets, err := listNetworksOnCluster(ctx, lipam.Client)
58+
clusterNetworks, err := listNetworksOnCluster(ctx, lipam.Client)
5959
if err != nil {
6060
return err
6161
}
6262

63-
// Create a set for faster lookup.
64-
nwSet := make(map[string]struct{})
65-
for _, net := range nets {
66-
nwSet[net] = struct{}{}
63+
// Create the set of networks present in the cluster (for faster lookup later).
64+
setClusterNetworks := make(map[string]struct{})
65+
66+
// Add networks that are present in the cluster but not in the cache.
67+
for _, net := range clusterNetworks {
68+
if _, inCache := lipam.cacheNetworks[net]; !inCache {
69+
if err := lipam.reserveNetwork(net); err != nil {
70+
return err
71+
}
72+
}
73+
setClusterNetworks[net] = struct{}{} // add network to the set
6774
}
6875

6976
// Remove networks that are present in the cache but not in the cluster, and were added before the threshold.
7077
for key := range lipam.cacheNetworks {
71-
if _, ok := nwSet[key]; !ok && lipam.cacheNetworks[key].creationTimestamp.Before(expiredThreshold) {
78+
if _, inCluster := setClusterNetworks[key]; !inCluster && lipam.cacheNetworks[key].creationTimestamp.Before(expiredThreshold) {
7279
lipam.freeNetwork(lipam.cacheNetworks[key].cidr)
7380
}
7481
}
@@ -83,15 +90,22 @@ func (lipam *LiqoIPAM) syncIPs(ctx context.Context, expiredThreshold time.Time)
8390
return err
8491
}
8592

86-
// Create a set for faster lookup.
87-
ipSet := make(map[string]struct{})
93+
// Create the set of IPs present in the cluster (for faster lookup later).
94+
setClusterIPs := make(map[string]struct{})
95+
96+
// Add IPs that are present in the cluster but not in the cache.
8897
for _, ip := range ips {
89-
ipSet[ip.String()] = struct{}{}
98+
if _, inCache := lipam.cacheIPs[ip.String()]; !inCache {
99+
if err := lipam.reserveIP(ip); err != nil {
100+
return err
101+
}
102+
}
103+
setClusterIPs[ip.String()] = struct{}{} // add IP to the set
90104
}
91105

92106
// Remove IPs that are present in the cache but not in the cluster, and were added before the threshold.
93107
for key := range lipam.cacheIPs {
94-
if _, ok := ipSet[key]; !ok && lipam.cacheIPs[key].creationTimestamp.Before(expiredThreshold) {
108+
if _, inCluster := setClusterIPs[key]; !inCluster && lipam.cacheIPs[key].creationTimestamp.Before(expiredThreshold) {
95109
lipam.freeIP(lipam.cacheIPs[key].ipCidr)
96110
}
97111
}

‎pkg/ipam/sync_test.go‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ var _ = Describe("Sync routine tests", func() {
3939
ctx context.Context
4040
fakeClientBuilder *fake.ClientBuilder
4141
now time.Time
42-
expiredThreshold time.Time
43-
expired time.Time
42+
newEntryThreshold time.Time
43+
notNewTimestamp time.Time
4444

4545
fakeIpamServer *LiqoIPAM
4646

@@ -96,8 +96,8 @@ var _ = Describe("Sync routine tests", func() {
9696
ctx = context.Background()
9797
fakeClientBuilder = fake.NewClientBuilder().WithScheme(scheme.Scheme)
9898
now = time.Now()
99-
expiredThreshold = now.Add(-syncFrequency)
100-
expired = now.Add(-2 * syncFrequency)
99+
newEntryThreshold = now.Add(-syncFrequency)
100+
notNewTimestamp = newEntryThreshold.Add(-time.Minute)
101101
})
102102

103103
Describe("Testing the sync routine", func() {
@@ -116,21 +116,21 @@ var _ = Describe("Sync routine tests", func() {
116116
cacheNetworks: make(map[string]networkInfo),
117117
}
118118
addNetowrkToCache(fakeIpamServer, "10.0.0.0/16", now)
119-
addNetowrkToCache(fakeIpamServer, "10.1.0.0/16", expired)
120-
addNetowrkToCache(fakeIpamServer, "10.3.0.0/16", expired)
119+
addNetowrkToCache(fakeIpamServer, "10.1.0.0/16", notNewTimestamp)
120+
addNetowrkToCache(fakeIpamServer, "10.3.0.0/16", notNewTimestamp)
121121
addNetowrkToCache(fakeIpamServer, "10.4.0.0/16", now)
122122
})
123123

124124
It("should remove networks from cache if they are not present in the cluster", func() {
125125
// Run sync
126-
Expect(fakeIpamServer.syncNetworks(ctx, expiredThreshold)).To(Succeed())
126+
Expect(fakeIpamServer.syncNetworks(ctx, newEntryThreshold)).To(Succeed())
127127

128128
// Check the cache
129129
Expect(fakeIpamServer.cacheNetworks).To(HaveKey("10.0.0.0/16")) // network in cluster and cache
130-
Expect(fakeIpamServer.cacheNetworks).To(HaveKey("10.1.0.0/16")) // network in cluster and cache before expired threshold
131-
Expect(fakeIpamServer.cacheNetworks).NotTo(HaveKey("10.2.0.0/16")) // network in cluster but not in cache
132-
Expect(fakeIpamServer.cacheNetworks).NotTo(HaveKey("10.3.0.0/16")) // network not in cluster but in cache before expired threshold
133-
Expect(fakeIpamServer.cacheNetworks).To(HaveKey("10.4.0.0/16")) // network not in cluster but in cache after expired threshold
130+
Expect(fakeIpamServer.cacheNetworks).To(HaveKey("10.1.0.0/16")) // network in cluster and cache before new entry threshold
131+
Expect(fakeIpamServer.cacheNetworks).To(HaveKey("10.2.0.0/16")) // network in cluster but not in cache
132+
Expect(fakeIpamServer.cacheNetworks).NotTo(HaveKey("10.3.0.0/16")) // network not in cluster but in cache before new entry threshold
133+
Expect(fakeIpamServer.cacheNetworks).To(HaveKey("10.4.0.0/16")) // network not in cluster but in cache after new entry threshold
134134
})
135135
})
136136

@@ -149,26 +149,26 @@ var _ = Describe("Sync routine tests", func() {
149149
cacheIPs: make(map[string]ipInfo),
150150
}
151151
addIPToCache(fakeIpamServer, "10.0.0.0", "10.0.0.0/24", now)
152-
addIPToCache(fakeIpamServer, "10.0.0.1", "10.0.0.0/24", expired)
153-
addIPToCache(fakeIpamServer, "10.0.0.3", "10.0.0.0/24", expired)
152+
addIPToCache(fakeIpamServer, "10.0.0.1", "10.0.0.0/24", notNewTimestamp)
153+
addIPToCache(fakeIpamServer, "10.0.0.3", "10.0.0.0/24", notNewTimestamp)
154154
addIPToCache(fakeIpamServer, "10.0.0.4", "10.0.0.0/24", now)
155155
})
156156

157157
It("should remove IPs from cache if they are not present in the cluster", func() {
158158
// Run sync
159-
Expect(fakeIpamServer.syncIPs(ctx, expiredThreshold)).To(Succeed())
159+
Expect(fakeIpamServer.syncIPs(ctx, newEntryThreshold)).To(Succeed())
160160

161161
// Check the cache
162162
Expect(fakeIpamServer.cacheIPs).To(HaveKey(
163163
ipCidr{ip: "10.0.0.0", cidr: "10.0.0.0/24"}.String())) // IP in cluster and cache
164164
Expect(fakeIpamServer.cacheIPs).To(HaveKey(
165-
ipCidr{ip: "10.0.0.1", cidr: "10.0.0.0/24"}.String())) // IP in cluster and cache before expired threshold
166-
Expect(fakeIpamServer.cacheIPs).NotTo(HaveKey(
165+
ipCidr{ip: "10.0.0.1", cidr: "10.0.0.0/24"}.String())) // IP in cluster and cache before new entry threshold
166+
Expect(fakeIpamServer.cacheIPs).To(HaveKey(
167167
ipCidr{ip: "10.0.0.2", cidr: "10.0.0.0/24"}.String())) // IP in cluster but not in cache
168168
Expect(fakeIpamServer.cacheIPs).NotTo(HaveKey(
169-
ipCidr{ip: "10.0.0.3", cidr: "10.0.0.0/24"}.String())) // IP not in cluster but in cache before expired threshold
169+
ipCidr{ip: "10.0.0.3", cidr: "10.0.0.0/24"}.String())) // IP not in cluster but in cache before new entry threshold
170170
Expect(fakeIpamServer.cacheIPs).To(HaveKey(
171-
ipCidr{ip: "10.0.0.4", cidr: "10.0.0.0/24"}.String())) // IP not in cluster but in cache after expired threshold
171+
ipCidr{ip: "10.0.0.4", cidr: "10.0.0.0/24"}.String())) // IP not in cluster but in cache after new entry threshold
172172
})
173173
})
174174
})

0 commit comments

Comments
 (0)