From 9bd39b449005724b7ab2eb6ed9838ad251e82d96 Mon Sep 17 00:00:00 2001 From: Francesco Torta <62566275+fra98@users.noreply.github.com> Date: Mon, 11 Nov 2024 17:26:52 +0100 Subject: [PATCH] fixup! test: ipam sync routine unit tests --- pkg/ipam/sync.go | 34 ++++++++++++++++++++++++---------- pkg/ipam/sync_test.go | 36 ++++++++++++++++++------------------ 2 files changed, 42 insertions(+), 28 deletions(-) diff --git a/pkg/ipam/sync.go b/pkg/ipam/sync.go index 0e953df4e5..a8d061629b 100644 --- a/pkg/ipam/sync.go +++ b/pkg/ipam/sync.go @@ -55,20 +55,27 @@ func (lipam *LiqoIPAM) sync(ctx context.Context, syncFrequency time.Duration) { func (lipam *LiqoIPAM) syncNetworks(ctx context.Context, expiredThreshold time.Time) error { // List all networks present in the cluster. - nets, err := listNetworksOnCluster(ctx, lipam.Client) + clusterNetworks, err := listNetworksOnCluster(ctx, lipam.Client) if err != nil { return err } - // Create a set for faster lookup. - nwSet := make(map[string]struct{}) - for _, net := range nets { - nwSet[net] = struct{}{} + // Create the set of networks present in the cluster (for faster lookup later). + setClusterNetworks := make(map[string]struct{}) + + // Add networks that are present in the cluster but not in the cache. + for _, net := range clusterNetworks { + if _, inCache := lipam.cacheNetworks[net]; !inCache { + if err := lipam.reserveNetwork(net); err != nil { + return err + } + } + setClusterNetworks[net] = struct{}{} // add network to the set } // Remove networks that are present in the cache but not in the cluster, and were added before the threshold. for key := range lipam.cacheNetworks { - if _, ok := nwSet[key]; !ok && lipam.cacheNetworks[key].creationTimestamp.Before(expiredThreshold) { + if _, inCluster := setClusterNetworks[key]; !inCluster && lipam.cacheNetworks[key].creationTimestamp.Before(expiredThreshold) { lipam.freeNetwork(lipam.cacheNetworks[key].cidr) } } @@ -83,15 +90,22 @@ func (lipam *LiqoIPAM) syncIPs(ctx context.Context, expiredThreshold time.Time) return err } - // Create a set for faster lookup. - ipSet := make(map[string]struct{}) + // Create the set of IPs present in the cluster (for faster lookup later). + setClusterIPs := make(map[string]struct{}) + + // Add IPs that are present in the cluster but not in the cache. for _, ip := range ips { - ipSet[ip.String()] = struct{}{} + if _, inCache := lipam.cacheIPs[ip.String()]; !inCache { + if err := lipam.reserveIP(ip); err != nil { + return err + } + } + setClusterIPs[ip.String()] = struct{}{} // add IP to the set } // Remove IPs that are present in the cache but not in the cluster, and were added before the threshold. for key := range lipam.cacheIPs { - if _, ok := ipSet[key]; !ok && lipam.cacheIPs[key].creationTimestamp.Before(expiredThreshold) { + if _, inCluster := setClusterIPs[key]; !inCluster && lipam.cacheIPs[key].creationTimestamp.Before(expiredThreshold) { lipam.freeIP(lipam.cacheIPs[key].ipCidr) } } diff --git a/pkg/ipam/sync_test.go b/pkg/ipam/sync_test.go index 607c98626b..8d9c54e794 100644 --- a/pkg/ipam/sync_test.go +++ b/pkg/ipam/sync_test.go @@ -39,8 +39,8 @@ var _ = Describe("Sync routine tests", func() { ctx context.Context fakeClientBuilder *fake.ClientBuilder now time.Time - expiredThreshold time.Time - expired time.Time + newEntryThreshold time.Time + notNewTimestamp time.Time fakeIpamServer *LiqoIPAM @@ -96,8 +96,8 @@ var _ = Describe("Sync routine tests", func() { ctx = context.Background() fakeClientBuilder = fake.NewClientBuilder().WithScheme(scheme.Scheme) now = time.Now() - expiredThreshold = now.Add(-syncFrequency) - expired = now.Add(-2 * syncFrequency) + newEntryThreshold = now.Add(-syncFrequency) + notNewTimestamp = newEntryThreshold.Add(-time.Minute) }) Describe("Testing the sync routine", func() { @@ -116,21 +116,21 @@ var _ = Describe("Sync routine tests", func() { cacheNetworks: make(map[string]networkInfo), } addNetowrkToCache(fakeIpamServer, "10.0.0.0/16", now) - addNetowrkToCache(fakeIpamServer, "10.1.0.0/16", expired) - addNetowrkToCache(fakeIpamServer, "10.3.0.0/16", expired) + addNetowrkToCache(fakeIpamServer, "10.1.0.0/16", notNewTimestamp) + addNetowrkToCache(fakeIpamServer, "10.3.0.0/16", notNewTimestamp) addNetowrkToCache(fakeIpamServer, "10.4.0.0/16", now) }) It("should remove networks from cache if they are not present in the cluster", func() { // Run sync - Expect(fakeIpamServer.syncNetworks(ctx, expiredThreshold)).To(Succeed()) + Expect(fakeIpamServer.syncNetworks(ctx, newEntryThreshold)).To(Succeed()) // Check the cache Expect(fakeIpamServer.cacheNetworks).To(HaveKey("10.0.0.0/16")) // network in cluster and cache - Expect(fakeIpamServer.cacheNetworks).To(HaveKey("10.1.0.0/16")) // network in cluster and cache before expired threshold - Expect(fakeIpamServer.cacheNetworks).NotTo(HaveKey("10.2.0.0/16")) // network in cluster but not in cache - Expect(fakeIpamServer.cacheNetworks).NotTo(HaveKey("10.3.0.0/16")) // network not in cluster but in cache before expired threshold - Expect(fakeIpamServer.cacheNetworks).To(HaveKey("10.4.0.0/16")) // network not in cluster but in cache after expired threshold + Expect(fakeIpamServer.cacheNetworks).To(HaveKey("10.1.0.0/16")) // network in cluster and cache before new entry threshold + Expect(fakeIpamServer.cacheNetworks).To(HaveKey("10.2.0.0/16")) // network in cluster but not in cache + Expect(fakeIpamServer.cacheNetworks).NotTo(HaveKey("10.3.0.0/16")) // network not in cluster but in cache before new entry threshold + Expect(fakeIpamServer.cacheNetworks).To(HaveKey("10.4.0.0/16")) // network not in cluster but in cache after new entry threshold }) }) @@ -149,26 +149,26 @@ var _ = Describe("Sync routine tests", func() { cacheIPs: make(map[string]ipInfo), } addIPToCache(fakeIpamServer, "10.0.0.0", "10.0.0.0/24", now) - addIPToCache(fakeIpamServer, "10.0.0.1", "10.0.0.0/24", expired) - addIPToCache(fakeIpamServer, "10.0.0.3", "10.0.0.0/24", expired) + addIPToCache(fakeIpamServer, "10.0.0.1", "10.0.0.0/24", notNewTimestamp) + addIPToCache(fakeIpamServer, "10.0.0.3", "10.0.0.0/24", notNewTimestamp) addIPToCache(fakeIpamServer, "10.0.0.4", "10.0.0.0/24", now) }) It("should remove IPs from cache if they are not present in the cluster", func() { // Run sync - Expect(fakeIpamServer.syncIPs(ctx, expiredThreshold)).To(Succeed()) + Expect(fakeIpamServer.syncIPs(ctx, newEntryThreshold)).To(Succeed()) // Check the cache Expect(fakeIpamServer.cacheIPs).To(HaveKey( ipCidr{ip: "10.0.0.0", cidr: "10.0.0.0/24"}.String())) // IP in cluster and cache Expect(fakeIpamServer.cacheIPs).To(HaveKey( - ipCidr{ip: "10.0.0.1", cidr: "10.0.0.0/24"}.String())) // IP in cluster and cache before expired threshold - Expect(fakeIpamServer.cacheIPs).NotTo(HaveKey( + ipCidr{ip: "10.0.0.1", cidr: "10.0.0.0/24"}.String())) // IP in cluster and cache before new entry threshold + Expect(fakeIpamServer.cacheIPs).To(HaveKey( ipCidr{ip: "10.0.0.2", cidr: "10.0.0.0/24"}.String())) // IP in cluster but not in cache Expect(fakeIpamServer.cacheIPs).NotTo(HaveKey( - ipCidr{ip: "10.0.0.3", cidr: "10.0.0.0/24"}.String())) // IP not in cluster but in cache before expired threshold + ipCidr{ip: "10.0.0.3", cidr: "10.0.0.0/24"}.String())) // IP not in cluster but in cache before new entry threshold Expect(fakeIpamServer.cacheIPs).To(HaveKey( - ipCidr{ip: "10.0.0.4", cidr: "10.0.0.0/24"}.String())) // IP not in cluster but in cache after expired threshold + ipCidr{ip: "10.0.0.4", cidr: "10.0.0.0/24"}.String())) // IP not in cluster but in cache after new entry threshold }) }) })