From ff1d03e8d24b94ff4cf0fdf74aa393b39e7cdb27 Mon Sep 17 00:00:00 2001 From: Francesco Cheinasso Date: Mon, 2 Dec 2024 10:15:19 +0100 Subject: [PATCH] feat: ipam timestamp --- cmd/ipam/main.go | 4 +- deployments/liqo/README.md | 1 + .../liqo/templates/liqo-ipam-deployment.yaml | 1 + deployments/liqo/values.yaml | 2 + pkg/consts/ipam.go | 3 +- pkg/ipam/core/ipam.go | 54 ++++++- pkg/ipam/core/node.go | 87 ++++++---- pkg/ipam/initialize.go | 2 +- pkg/ipam/ipam.go | 11 +- pkg/ipam/ips.go | 9 +- pkg/ipam/networks.go | 7 +- pkg/ipam/sync.go | 9 +- pkg/ipam/sync_test.go | 148 ++++++++++++------ pkg/utils/testutil/liqo.go | 106 +++++-------- 14 files changed, 279 insertions(+), 165 deletions(-) diff --git a/cmd/ipam/main.go b/cmd/ipam/main.go index 29a75454fd..15feb530e3 100644 --- a/cmd/ipam/main.go +++ b/cmd/ipam/main.go @@ -72,8 +72,10 @@ func main() { // Server options. cmd.Flags().IntVar(&options.ServerOpts.Port, "port", consts.IpamPort, "The port on which to listen for incoming gRPC requests.") - cmd.Flags().DurationVar(&options.ServerOpts.SyncFrequency, "sync-interval", consts.SyncInterval, + cmd.Flags().DurationVar(&options.ServerOpts.SyncInterval, "sync-interval", consts.SyncInterval, "The interval at which the IPAM will synchronize the IPAM storage.") + cmd.Flags().DurationVar(&options.ServerOpts.SyncGracePeriod, "sync-graceperiod", consts.SyncGracePeriod, + "The grace period the sync routine wait before releasing an ip or a network.") cmd.Flags().BoolVar(&options.ServerOpts.GraphvizEnabled, "enable-graphviz", false, "Enable the graphviz output for the IPAM.") cmd.Flags().StringSliceVar(&options.ServerOpts.Pools, "pools", []string{"10.0.0.0/8", "192.168.0.0/16", "172.16.0.0/12"}, diff --git a/deployments/liqo/README.md b/deployments/liqo/README.md index df0e767eeb..72020fdf5f 100644 --- a/deployments/liqo/README.md +++ b/deployments/liqo/README.md @@ -59,6 +59,7 @@ | ipam.internal.pod.priorityClassName | string | `""` | PriorityClassName (https://kubernetes.io/docs/concepts/scheduling-eviction/pod-priority-preemption/#pod-priority) for the IPAM pod. | | ipam.internal.pod.resources | object | `{"limits":{},"requests":{}}` | Resource requests and limits (https://kubernetes.io/docs/user-guide/compute-resources/) for the IPAM pod. | | ipam.internal.replicas | int | `1` | The number of IPAM instances to run, which can be increased for active/passive high availability. | +| ipam.internal.syncGracePeriod | string | `"30s"` | | | ipam.internal.syncInterval | string | `"2m"` | Set the interval at which the IPAM pod will synchronize it's in-memory status with the local cluster. If you want to disable the synchronization, set the interval to 0. | | ipam.internalCIDR | string | `"10.80.0.0/16"` | The subnet used for the internal CIDR. These IPs are assigned to the Liqo internal-network interfaces. | | ipam.podCIDR | string | `""` | The subnet used by the pods in your cluster, in CIDR notation (e.g., 10.0.0.0/16). | diff --git a/deployments/liqo/templates/liqo-ipam-deployment.yaml b/deployments/liqo/templates/liqo-ipam-deployment.yaml index 39d23e5825..5cd8a80145 100644 --- a/deployments/liqo/templates/liqo-ipam-deployment.yaml +++ b/deployments/liqo/templates/liqo-ipam-deployment.yaml @@ -52,6 +52,7 @@ spec: - --pod-name=$(POD_NAME) - --port=6000 - --sync-interval={{ .Values.ipam.internal.syncInterval }} + - --sync-graceperiod={{ .Values.ipam.internal.syncGracePeriod }} {{- if $ha }} - --leader-election - --leader-election-namespace=$(POD_NAMESPACE) diff --git a/deployments/liqo/values.yaml b/deployments/liqo/values.yaml index caab14993f..041aa5f24b 100644 --- a/deployments/liqo/values.yaml +++ b/deployments/liqo/values.yaml @@ -449,6 +449,8 @@ ipam: # -- Set the interval at which the IPAM pod will synchronize it's in-memory status with the local cluster. # If you want to disable the synchronization, set the interval to 0. syncInterval: 2m + ## -- Set the grace period the sync routine will wait before deleting an ip or a network. + syncGracePeriod: 30s # -- The subnet used by the pods in your cluster, in CIDR notation (e.g., 10.0.0.0/16). podCIDR: "" # -- The subnet used by the services in you cluster, in CIDR notation (e.g., 172.16.0.0/16). diff --git a/pkg/consts/ipam.go b/pkg/consts/ipam.go index 3b74e9e19c..194d4bc4de 100644 --- a/pkg/consts/ipam.go +++ b/pkg/consts/ipam.go @@ -24,7 +24,8 @@ const ( IpamPort = 6000 // SyncInterval is the frequency at which the IPAM should periodically sync its status. SyncInterval = 2 * time.Minute - + // SyncGracePeriod is the time the IPAM sync routine should wait before performing a deletion. + SyncGracePeriod = 30 * time.Second // NetworkNotRemappedLabelKey is the label key used to mark a Network that does not need CIDR remapping. NetworkNotRemappedLabelKey = "ipam.liqo.io/network-not-remapped" // NetworkNotRemappedLabelValue is the label value used to mark a Network that does not need CIDR remapping. diff --git a/pkg/ipam/core/ipam.go b/pkg/ipam/core/ipam.go index 6456f0d5b1..e2adb94203 100644 --- a/pkg/ipam/core/ipam.go +++ b/pkg/ipam/core/ipam.go @@ -17,7 +17,7 @@ package ipamcore import ( "fmt" "net/netip" - "slices" + "time" ) // Ipam represents the IPAM core structure. @@ -74,10 +74,10 @@ func (ipam *Ipam) NetworkAcquireWithPrefix(prefix netip.Prefix) *netip.Prefix { // NetworkRelease frees the network with the given prefix. // It returns the freed network or nil if the network is not found. -func (ipam *Ipam) NetworkRelease(prefix netip.Prefix) *netip.Prefix { +func (ipam *Ipam) NetworkRelease(prefix netip.Prefix, gracePeriod time.Duration) *netip.Prefix { for i := range ipam.roots { if isPrefixChildOf(ipam.roots[i].prefix, prefix) { - if result := networkRelease(prefix, &ipam.roots[i]); result != nil { + if result := networkRelease(prefix, &ipam.roots[i], gracePeriod); result != nil { return result } } @@ -137,13 +137,13 @@ func (ipam *Ipam) IPAcquireWithAddr(prefix netip.Prefix, addr netip.Addr) (*neti // IPRelease frees the IP address from the given prefix. // It returns the freed IP address or nil if the IP address is not found. -func (ipam *Ipam) IPRelease(prefix netip.Prefix, addr netip.Addr) (*netip.Addr, error) { +func (ipam *Ipam) IPRelease(prefix netip.Prefix, addr netip.Addr, gracePeriod time.Duration) (*netip.Addr, error) { node, err := ipam.search(prefix) if err != nil { return nil, err } if node != nil { - return node.ipRelease(addr), nil + return node.ipRelease(addr, gracePeriod), nil } return nil, nil } @@ -155,14 +155,18 @@ func (ipam *Ipam) ListIPs(prefix netip.Prefix) ([]netip.Addr, error) { return nil, err } if node != nil { - return slices.Clone(node.ips), nil + addrs := make([]netip.Addr, len(node.ips)) + for i := range node.ips { + addrs[i] = node.ips[i].addr + } + return addrs, nil } return nil, nil } -// IsAllocatedIP checks if the IP address is allocated from the given prefix. +// IPIsAllocated checks if the IP address is allocated from the given prefix. // It returns true if the IP address is allocated, false otherwise. -func (ipam *Ipam) IsAllocatedIP(prefix netip.Prefix, addr netip.Addr) (bool, error) { +func (ipam *Ipam) IPIsAllocated(prefix netip.Prefix, addr netip.Addr) (bool, error) { node, err := ipam.search(prefix) if err != nil { return false, err @@ -216,3 +220,37 @@ func checkRoots(roots []netip.Prefix) error { } return nil } + +// NetworkSetLastUpdateTimestamp sets the last update time of the network with the given prefix. +// This function is for testing purposes only. +func (ipam *Ipam) NetworkSetLastUpdateTimestamp(prefix netip.Prefix, lastUpdateTimestamp time.Time) error { + node, err := ipam.search(prefix) + if err != nil { + return err + } + if node == nil { + return fmt.Errorf("prefix %s not found", prefix) + } + node.lastUpdateTimestamp = lastUpdateTimestamp + return nil +} + +// IPSetCreationTimestamp sets the creation timestamp of the IP address with the given address. +// This function is for testing purposes only. +func (ipam *Ipam) IPSetCreationTimestamp(addr netip.Addr, prefix netip.Prefix, creationTimestamp time.Time) error { + node, err := ipam.search(prefix) + if err != nil { + return err + } + if node == nil { + return fmt.Errorf("prefix %s not found", prefix) + } + + for i := range node.ips { + if node.ips[i].addr.Compare(addr) == 0 { + node.ips[i].creationTimestamp = creationTimestamp + return nil + } + } + return nil +} diff --git a/pkg/ipam/core/node.go b/pkg/ipam/core/node.go index 9219a0976b..4b1058eee9 100644 --- a/pkg/ipam/core/node.go +++ b/pkg/ipam/core/node.go @@ -21,16 +21,25 @@ import ( "os" "path/filepath" "strings" + "time" ) +// nodeIP represents an IP address acquired by a node. +type nodeIP struct { + addr netip.Addr + creationTimestamp time.Time +} + // node represents a node in the binary tree. type node struct { + lastUpdateTimestamp time.Time + prefix netip.Prefix acquired bool left *node right *node - ips []netip.Addr + ips []nodeIP lastip netip.Addr } @@ -39,10 +48,12 @@ type nodeDirection string const ( leftDirection nodeDirection = "left" rightDirection nodeDirection = "right" + + graphvizFolder = "./graphviz" ) func newNode(prefix netip.Prefix) node { - return node{prefix: prefix} + return node{prefix: prefix, lastUpdateTimestamp: time.Now()} } func allocateNetwork(size int, node *node) *netip.Prefix { @@ -52,6 +63,7 @@ func allocateNetwork(size int, node *node) *netip.Prefix { if node.prefix.Bits() == size { if !node.isSplitted() { node.acquired = true + node.lastUpdateTimestamp = time.Now() return &node.prefix } return nil @@ -76,6 +88,7 @@ func allocateNetworkWithPrefix(prefix netip.Prefix, node *node) *netip.Prefix { if node.prefix.Addr().Compare(prefix.Addr()) == 0 && node.prefix.Bits() == prefix.Bits() { if !node.acquired && node.left == nil && node.right == nil { node.acquired = true + node.lastUpdateTimestamp = time.Now() return &node.prefix } return nil @@ -95,29 +108,31 @@ func allocateNetworkWithPrefix(prefix netip.Prefix, node *node) *netip.Prefix { return nil } -func networkRelease(prefix netip.Prefix, node *node) *netip.Prefix { +func networkRelease(prefix netip.Prefix, node *node, gracePeriod time.Duration) *netip.Prefix { var result *netip.Prefix if node == nil { return nil } - if node.prefix.Addr().Compare(prefix.Addr()) == 0 && node.prefix.Bits() == prefix.Bits() { + if node.prefix.Addr().Compare(prefix.Addr()) == 0 && node.prefix.Bits() == prefix.Bits() && + node.lastUpdateTimestamp.Add(gracePeriod).Before(time.Now()) { if node.acquired { node.acquired = false + node.lastUpdateTimestamp = time.Now() return &node.prefix } return nil } if node.left != nil && node.left.prefix.Overlaps(prefix) { - result = networkRelease(prefix, node.left) + result = networkRelease(prefix, node.left, gracePeriod) } if node.right != nil && node.right.prefix.Overlaps(prefix) { - result = networkRelease(prefix, node.right) + result = networkRelease(prefix, node.right, gracePeriod) } - node.merge() + node.merge(gracePeriod) return result } @@ -170,7 +185,7 @@ func listNetworks(node *node) []netip.Prefix { func (n *node) isAllocatedIP(ip netip.Addr) bool { for i := range n.ips { - if n.ips[i].Compare(ip) == 0 { + if n.ips[i].addr.Compare(ip) == 0 { return true } } @@ -202,8 +217,9 @@ func (n *node) ipAcquire() *netip.Addr { addr = n.prefix.Addr() } if !n.isAllocatedIP(addr) { - n.ips = append(n.ips, addr) + n.ips = append(n.ips, nodeIP{addr: addr, creationTimestamp: time.Now()}) n.lastip = addr + n.lastUpdateTimestamp = time.Now() return &addr } addr = addr.Next() @@ -221,25 +237,30 @@ func (n *node) allocateIPWithAddr(addr netip.Addr) *netip.Addr { } for i := range n.ips { - if n.ips[i].Compare(addr) == 0 { + if n.ips[i].addr.Compare(addr) == 0 { return nil } } - n.ips = append(n.ips, addr) + n.ips = append(n.ips, nodeIP{addr: addr, creationTimestamp: time.Now()}) + n.lastUpdateTimestamp = time.Now() - return &n.ips[len(n.ips)-1] + return &n.ips[len(n.ips)-1].addr } -func (n *node) ipRelease(ip netip.Addr) *netip.Addr { +func (n *node) ipRelease(ip netip.Addr, gracePeriod time.Duration) *netip.Addr { if !n.acquired { return nil } - for i, addr := range n.ips { - if addr.Compare(ip) == 0 { + for i, nodeIP := range n.ips { + if !nodeIP.creationTimestamp.Add(gracePeriod).Before(time.Now()) { + continue + } + if nodeIP.addr.Compare(ip) == 0 { n.ips = append(n.ips[:i], n.ips[i+1:]...) - return &addr + n.lastUpdateTimestamp = time.Now() + return &nodeIP.addr } } return nil @@ -273,21 +294,33 @@ func (n *node) split() { n.insert(rightDirection, right) } -func (n *node) merge() { - if n.left.isLeaf() && n.right.isLeaf() && !n.left.acquired && !n.right.acquired { - n.left = nil - n.right = nil +func (n *node) merge(gracePeriod time.Duration) { + if n.left == nil || n.right == nil { + return + } + if !n.left.lastUpdateTimestamp.Add(gracePeriod).Before(time.Now()) || !n.right.lastUpdateTimestamp.Add(gracePeriod).Before(time.Now()) { + return // grace period not expired + } + if !n.left.isLeaf() || !n.right.isLeaf() { + return } + if n.left.acquired || n.right.acquired { + return + } + + n.left = nil + n.right = nil + n.lastUpdateTimestamp = time.Now() } func (n *node) insert(nd nodeDirection, prefix netip.Prefix) { - newNode := &node{prefix: prefix} + newNode := newNode(prefix) switch nd { case leftDirection: - n.left = newNode + n.left = &newNode return case rightDirection: - n.right = newNode + n.right = &newNode return default: return @@ -335,13 +368,13 @@ func (n *node) toGraphviz() error { n.toGraphvizRecursive(&sb) sb.WriteString("}\n") - if _, err := os.Stat("./graphviz"); os.IsNotExist(err) { - if err := os.Mkdir("./graphviz", 0o700); err != nil { + if _, err := os.Stat(graphvizFolder + ""); os.IsNotExist(err) { + if err := os.Mkdir(graphvizFolder+"", 0o700); err != nil { return err } } - filePath := filepath.Clean("./graphviz/" + strings.NewReplacer("/", "_", ".", "_").Replace(n.prefix.String()) + ".dot") + filePath := filepath.Clean(graphvizFolder + "/" + strings.NewReplacer("/", "_", ".", "_").Replace(n.prefix.String()) + ".dot") file, err := os.Create(filePath) if err != nil { return err @@ -360,7 +393,7 @@ func (n *node) toGraphvizRecursive(sb *strings.Builder) { if len(n.ips) > 0 { ipsString := []string{} for i := range n.ips { - ipsString = append(ipsString, n.ips[i].String()) + ipsString = append(ipsString, n.ips[i].addr.String()) } label += "\\n" + strings.Join(ipsString, "\\n") } diff --git a/pkg/ipam/initialize.go b/pkg/ipam/initialize.go index 35d669ee36..258bebe098 100644 --- a/pkg/ipam/initialize.go +++ b/pkg/ipam/initialize.go @@ -57,7 +57,7 @@ func (lipam *LiqoIPAM) initializeNetworks(ctx context.Context) error { } for i := 0; i < int(netdetails.preallocated); i++ { if _, err := lipam.ipAcquire(net); err != nil { - return errors.Join(err, lipam.networkRelease(net)) + return errors.Join(err, lipam.networkRelease(net, 0)) } } } diff --git a/pkg/ipam/ipam.go b/pkg/ipam/ipam.go index 3fffafaaf9..7c0e71f6d2 100644 --- a/pkg/ipam/ipam.go +++ b/pkg/ipam/ipam.go @@ -46,7 +46,8 @@ type LiqoIPAM struct { type ServerOptions struct { Pools []string Port int - SyncFrequency time.Duration + SyncInterval time.Duration + SyncGracePeriod time.Duration GraphvizEnabled bool } @@ -74,7 +75,7 @@ func New(ctx context.Context, cl client.Client, roots []string, opts *ServerOpti } // Launch sync routine - go lipam.sync(ctx, opts.SyncFrequency) + go lipam.sync(ctx, opts.SyncInterval) hs.SetServingStatus(IPAM_ServiceDesc.ServiceName, grpc_health_v1.HealthCheckResponse_SERVING) @@ -118,7 +119,7 @@ func (lipam *LiqoIPAM) IPRelease(_ context.Context, req *IPReleaseRequest) (*IPR return &IPReleaseResponse{}, fmt.Errorf("failed to parse prefix %q: %w", req.GetCidr(), err) } - if err := lipam.ipRelease(addr, prefix); err != nil { + if err := lipam.ipRelease(addr, prefix, 0); err != nil { return &IPReleaseResponse{}, err } @@ -157,7 +158,7 @@ func (lipam *LiqoIPAM) NetworkAcquire(_ context.Context, req *NetworkAcquireRequ for i := 0; i < int(req.GetPreAllocated()); i++ { _, err := lipam.ipAcquire(*remappedCidr) if err != nil { - return &NetworkAcquireResponse{}, errors.Join(err, lipam.networkRelease(*remappedCidr)) + return &NetworkAcquireResponse{}, errors.Join(err, lipam.networkRelease(*remappedCidr, 0)) } } @@ -174,7 +175,7 @@ func (lipam *LiqoIPAM) NetworkRelease(_ context.Context, req *NetworkReleaseRequ return &NetworkReleaseResponse{}, fmt.Errorf("failed to parse prefix %q: %w", req.GetCidr(), err) } - if err := lipam.networkRelease(prefix); err != nil { + if err := lipam.networkRelease(prefix, 0); err != nil { return &NetworkReleaseResponse{}, err } diff --git a/pkg/ipam/ips.go b/pkg/ipam/ips.go index 00ba7ad79f..51694c855c 100644 --- a/pkg/ipam/ips.go +++ b/pkg/ipam/ips.go @@ -18,6 +18,7 @@ import ( "context" "fmt" "net/netip" + "time" klog "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" @@ -61,13 +62,13 @@ func (lipam *LiqoIPAM) ipAcquireWithAddr(addr netip.Addr, prefix netip.Prefix) e } // ipRelease frees an IP, removing it from the cache. -func (lipam *LiqoIPAM) ipRelease(addr netip.Addr, prefix netip.Prefix) error { - result, err := lipam.IpamCore.IPRelease(prefix, addr) +func (lipam *LiqoIPAM) ipRelease(addr netip.Addr, prefix netip.Prefix, gracePeriod time.Duration) error { + result, err := lipam.IpamCore.IPRelease(prefix, addr, gracePeriod) if err != nil { return fmt.Errorf("error freeing IP %q (network %q): %w", addr.String(), prefix.String(), err) } if result == nil { - klog.Infof("IP %q (network %q) already freed", addr.String(), prefix.String()) + klog.Infof("IP %q (network %q) already freed or grace period not over", addr.String(), prefix.String()) return nil } klog.Infof("Freed IP %q (network %q)", addr.String(), prefix.String()) @@ -80,7 +81,7 @@ func (lipam *LiqoIPAM) ipRelease(addr netip.Addr, prefix netip.Prefix) error { // isIPAvailable checks if an IP is available. func (lipam *LiqoIPAM) isIPAvailable(addr netip.Addr, prefix netip.Prefix) (bool, error) { - allocated, err := lipam.IpamCore.IsAllocatedIP(prefix, addr) + allocated, err := lipam.IpamCore.IPIsAllocated(prefix, addr) return !allocated, err } diff --git a/pkg/ipam/networks.go b/pkg/ipam/networks.go index c56a9aba4a..3fd4cfc301 100644 --- a/pkg/ipam/networks.go +++ b/pkg/ipam/networks.go @@ -18,6 +18,7 @@ import ( "context" "fmt" "net/netip" + "time" klog "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" @@ -60,10 +61,10 @@ func (lipam *LiqoIPAM) networkAcquireSpecific(prefix netip.Prefix) (*netip.Prefi } // networkRelease frees a network, removing it from the cache. -func (lipam *LiqoIPAM) networkRelease(prefix netip.Prefix) error { - result := lipam.IpamCore.NetworkRelease(prefix) +func (lipam *LiqoIPAM) networkRelease(prefix netip.Prefix, gracePeriod time.Duration) error { + result := lipam.IpamCore.NetworkRelease(prefix, gracePeriod) if result == nil { - klog.Infof("Network %q already freed", prefix.String()) + klog.Infof("Network %q already freed or grace period not over", prefix.String()) return nil } klog.Infof("Freed network %q", prefix.String()) diff --git a/pkg/ipam/sync.go b/pkg/ipam/sync.go index eaa481c2de..7bae8528b5 100644 --- a/pkg/ipam/sync.go +++ b/pkg/ipam/sync.go @@ -41,7 +41,7 @@ func (lipam *LiqoIPAM) sync(ctx context.Context, syncFrequency time.Duration) { func(ctx context.Context) (done bool, err error) { lipam.mutex.Lock() defer lipam.mutex.Unlock() - klog.V(3).Info("Started IPAM cache sync routine") + klog.V(3).Infof("Started IPAM cache sync routine (grace period: %s)", lipam.opts.SyncGracePeriod) // Sync networks. if err := lipam.syncNetworks(ctx); err != nil { @@ -71,7 +71,7 @@ func syncNetworkAcquire(lipam *LiqoIPAM, clusterNetworks map[netip.Prefix]prefix } for i := 0; i < int(clusterNetworkDetails.preallocated); i++ { if _, err := lipam.ipAcquire(clusterNetwork); err != nil { - return errors.Join(err, lipam.networkRelease(clusterNetwork)) + return errors.Join(err, lipam.networkRelease(clusterNetwork, 0)) } } } @@ -88,7 +88,7 @@ func syncNetworkFree(lipam *LiqoIPAM, clusterNetworks map[netip.Prefix]prefixDet // Remove networks that are present in the cache but not in the cluster, and were added before the threshold. for cachedNetwork := range cachedNetworks { if !isNetworkInCluster(clusterNetworks, cachedNetwork) { - if err := lipam.networkRelease(cachedNetwork); err != nil { + if err := lipam.networkRelease(cachedNetwork, lipam.opts.SyncGracePeriod); err != nil { return fmt.Errorf("failed to free network %q: %w", cachedNetwork.String(), err) } } @@ -145,7 +145,7 @@ func iscachedIPPreallocated(cachedIP netip.Addr, cachedNetwork netip.Prefix, clu func syncIPsFree(lipam *LiqoIPAM, clusterIPs, cachedIPs map[netip.Addr]netip.Prefix, clusterNetworks map[netip.Prefix]prefixDetails) error { for cachedIP, cachedNetwork := range cachedIPs { if _, ok := clusterIPs[cachedIP]; !ok && !iscachedIPPreallocated(cachedIP, cachedNetwork, clusterNetworks) { - if err := lipam.ipRelease(cachedIP, cachedNetwork); err != nil { + if err := lipam.ipRelease(cachedIP, cachedNetwork, lipam.opts.SyncGracePeriod); err != nil { return fmt.Errorf("failed to free IP %q: %w", cachedIP.String(), err) } } @@ -159,6 +159,7 @@ func (lipam *LiqoIPAM) syncIPs(ctx context.Context) error { if err != nil { return err } + clusterNetworksMap, err := lipam.listNetworksOnCluster(ctx, lipam.Client) if err != nil { return err diff --git a/pkg/ipam/sync_test.go b/pkg/ipam/sync_test.go index 45c3612d2f..ab324cba7a 100644 --- a/pkg/ipam/sync_test.go +++ b/pkg/ipam/sync_test.go @@ -17,22 +17,21 @@ package ipam import ( "context" "net/netip" + "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client/fake" - ipamv1alpha1 "github.com/liqotech/liqo/apis/ipam/v1alpha1" - networkingv1beta1 "github.com/liqotech/liqo/apis/networking/v1beta1" ipamcore "github.com/liqotech/liqo/pkg/ipam/core" + "github.com/liqotech/liqo/pkg/utils/testutil" ) var _ = Describe("Sync routine tests", func() { const ( - syncFrequency = 0 - testNamespace = "test" + syncGracePeriod = time.Second * 5 + testNamespace = "test" ) var ( @@ -41,37 +40,6 @@ var _ = Describe("Sync routine tests", func() { fakeIpamServer *LiqoIPAM - newNetwork = func(name, cidr string) *ipamv1alpha1.Network { - return &ipamv1alpha1.Network{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: testNamespace, - }, - Spec: ipamv1alpha1.NetworkSpec{ - CIDR: networkingv1beta1.CIDR(cidr), - }, - Status: ipamv1alpha1.NetworkStatus{ - CIDR: networkingv1beta1.CIDR(cidr), - }, - } - } - - newIP = func(name, ip, cidr string) *ipamv1alpha1.IP { - return &ipamv1alpha1.IP{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: testNamespace, - }, - Spec: ipamv1alpha1.IPSpec{ - IP: networkingv1beta1.IP(ip), - }, - Status: ipamv1alpha1.IPStatus{ - IP: networkingv1beta1.IP(ip), - CIDR: networkingv1beta1.CIDR(cidr), - }, - } - } - addNetwork = func(server *LiqoIPAM, cidr string) { prefix, err := netip.ParsePrefix(cidr) Expect(err).ShouldNot(HaveOccurred()) @@ -98,9 +66,10 @@ var _ = Describe("Sync routine tests", func() { BeforeEach(func() { // Add in-cluster networks client := fakeClientBuilder.WithObjects( - newNetwork("net1", "10.0.0.0/16"), - newNetwork("net2", "10.1.0.0/16"), - newNetwork("net3", "10.2.0.0/16"), + testutil.FakeNetwork("net1", "10.0.0.0/16", map[string]string{}), + testutil.FakeNetwork("net2", "10.1.0.0/16", map[string]string{}), + testutil.FakeNetwork("net3", "10.2.0.0/16", map[string]string{}), + testutil.FakeNetwork("net4", "10.4.0.0/16", map[string]string{}), ).Build() ipamCore, err := ipamcore.NewIpam([]string{"10.0.0.0/8"}) @@ -111,17 +80,58 @@ var _ = Describe("Sync routine tests", func() { Client: client, IpamCore: ipamCore, opts: &ServerOptions{ - SyncFrequency: syncFrequency, + SyncGracePeriod: syncGracePeriod, GraphvizEnabled: false, }, } addNetwork(fakeIpamServer, "10.0.0.0/16") addNetwork(fakeIpamServer, "10.1.0.0/16") addNetwork(fakeIpamServer, "10.3.0.0/16") - addNetwork(fakeIpamServer, "10.4.0.0/16") + addNetwork(fakeIpamServer, "10.5.0.0/16") }) It("should remove networks from cache if they are not present in the cluster", func() { + newLastUpdate := time.Now().Add(-syncGracePeriod) + + // Check the cache + Expect(fakeIpamServer.networkIsAvailable(netip.MustParsePrefix("10.0.0.0/16"))).To(Equal(false)) + Expect(fakeIpamServer.networkIsAvailable(netip.MustParsePrefix("10.1.0.0/16"))).To(Equal(false)) + Expect(fakeIpamServer.networkIsAvailable(netip.MustParsePrefix("10.2.0.0/16"))).To(Equal(true)) + Expect(fakeIpamServer.networkIsAvailable(netip.MustParsePrefix("10.3.0.0/16"))).To(Equal(false)) + Expect(fakeIpamServer.networkIsAvailable(netip.MustParsePrefix("10.4.0.0/16"))).To(Equal(true)) + Expect(fakeIpamServer.networkIsAvailable(netip.MustParsePrefix("10.5.0.0/16"))).To(Equal(false)) + + // Run sync + Expect(fakeIpamServer.syncNetworks(ctx)).To(Succeed()) + + Expect(fakeIpamServer.networkIsAvailable(netip.MustParsePrefix("10.0.0.0/16"))).To(Equal(false)) + Expect(fakeIpamServer.networkIsAvailable(netip.MustParsePrefix("10.1.0.0/16"))).To(Equal(false)) + Expect(fakeIpamServer.networkIsAvailable(netip.MustParsePrefix("10.2.0.0/16"))).To(Equal(false)) + Expect(fakeIpamServer.networkIsAvailable(netip.MustParsePrefix("10.3.0.0/16"))).To(Equal(false)) + Expect(fakeIpamServer.networkIsAvailable(netip.MustParsePrefix("10.4.0.0/16"))).To(Equal(false)) + Expect(fakeIpamServer.networkIsAvailable(netip.MustParsePrefix("10.5.0.0/16"))).To(Equal(false)) + + // Update the last update timestamp of the networks + Expect(fakeIpamServer.IpamCore.NetworkSetLastUpdateTimestamp(netip.MustParsePrefix("10.0.0.0/16"), newLastUpdate)).Should(Succeed()) + Expect(fakeIpamServer.IpamCore.NetworkSetLastUpdateTimestamp(netip.MustParsePrefix("10.1.0.0/16"), newLastUpdate)).Should(Succeed()) + Expect(fakeIpamServer.IpamCore.NetworkSetLastUpdateTimestamp(netip.MustParsePrefix("10.2.0.0/16"), newLastUpdate)).Should(Succeed()) + Expect(fakeIpamServer.IpamCore.NetworkSetLastUpdateTimestamp(netip.MustParsePrefix("10.4.0.0/16"), newLastUpdate)).Should(Succeed()) + Expect(fakeIpamServer.IpamCore.NetworkSetLastUpdateTimestamp(netip.MustParsePrefix("10.5.0.0/16"), newLastUpdate)).Should(Succeed()) + + // Run sync + Expect(fakeIpamServer.syncNetworks(ctx)).To(Succeed()) + + // Check the cache + Expect(fakeIpamServer.networkIsAvailable(netip.MustParsePrefix("10.0.0.0/16"))).To(Equal(false)) + Expect(fakeIpamServer.networkIsAvailable(netip.MustParsePrefix("10.1.0.0/16"))).To(Equal(false)) + Expect(fakeIpamServer.networkIsAvailable(netip.MustParsePrefix("10.2.0.0/16"))).To(Equal(false)) + Expect(fakeIpamServer.networkIsAvailable(netip.MustParsePrefix("10.3.0.0/16"))).To(Equal(false)) + Expect(fakeIpamServer.networkIsAvailable(netip.MustParsePrefix("10.4.0.0/16"))).To(Equal(false)) + Expect(fakeIpamServer.networkIsAvailable(netip.MustParsePrefix("10.5.0.0/16"))).To(Equal(true)) + + // Update the last update timestamp of the networks + Expect(fakeIpamServer.IpamCore.NetworkSetLastUpdateTimestamp(netip.MustParsePrefix("10.3.0.0/16"), newLastUpdate)).Should(Succeed()) + // Run sync Expect(fakeIpamServer.syncNetworks(ctx)).To(Succeed()) @@ -130,7 +140,8 @@ var _ = Describe("Sync routine tests", func() { Expect(fakeIpamServer.networkIsAvailable(netip.MustParsePrefix("10.1.0.0/16"))).To(Equal(false)) Expect(fakeIpamServer.networkIsAvailable(netip.MustParsePrefix("10.2.0.0/16"))).To(Equal(false)) Expect(fakeIpamServer.networkIsAvailable(netip.MustParsePrefix("10.3.0.0/16"))).To(Equal(true)) - Expect(fakeIpamServer.networkIsAvailable(netip.MustParsePrefix("10.4.0.0/16"))).To(Equal(true)) + Expect(fakeIpamServer.networkIsAvailable(netip.MustParsePrefix("10.4.0.0/16"))).To(Equal(false)) + Expect(fakeIpamServer.networkIsAvailable(netip.MustParsePrefix("10.5.0.0/16"))).To(Equal(true)) }) }) @@ -138,11 +149,11 @@ var _ = Describe("Sync routine tests", func() { BeforeEach(func() { // Add in-cluster IPs client := fakeClientBuilder.WithObjects( - newNetwork("net1", "10.0.0.0/24"), + testutil.FakeNetwork("net1", "10.0.0.0/24", map[string]string{}), - newIP("ip1", "10.0.0.0", "10.0.0.0/24"), - newIP("ip2", "10.0.0.1", "10.0.0.0/24"), - newIP("ip3", "10.0.0.2", "10.0.0.0/24"), + testutil.FakeIP("ip1", "10.0.0.0", "10.0.0.0/24", nil, nil, false), + testutil.FakeIP("ip2", "10.0.0.1", "10.0.0.0/24", nil, nil, false), + testutil.FakeIP("ip3", "10.0.0.2", "10.0.0.0/24", nil, nil, false), ).Build() ipamCore, err := ipamcore.NewIpam([]string{"10.0.0.0/8"}) @@ -153,8 +164,8 @@ var _ = Describe("Sync routine tests", func() { Client: client, IpamCore: ipamCore, opts: &ServerOptions{ - SyncFrequency: syncFrequency, GraphvizEnabled: false, + SyncGracePeriod: syncGracePeriod, }, } @@ -167,10 +178,51 @@ var _ = Describe("Sync routine tests", func() { }) It("should remove IPs from cache if they are not present in the cluster", func() { + newCreationTimestamp := time.Now().Add(-syncGracePeriod) + + // Check the cache before grace period + Expect(fakeIpamServer.isIPAvailable(netip.MustParseAddr("10.0.0.0"), netip.MustParsePrefix("10.0.0.0/24"))).To(Equal(false)) + Expect(fakeIpamServer.isIPAvailable(netip.MustParseAddr("10.0.0.1"), netip.MustParsePrefix("10.0.0.0/24"))).To(Equal(false)) + Expect(fakeIpamServer.isIPAvailable(netip.MustParseAddr("10.0.0.2"), netip.MustParsePrefix("10.0.0.0/24"))).To(Equal(true)) + Expect(fakeIpamServer.isIPAvailable(netip.MustParseAddr("10.0.0.3"), netip.MustParsePrefix("10.0.0.0/24"))).To(Equal(false)) + Expect(fakeIpamServer.isIPAvailable(netip.MustParseAddr("10.0.0.4"), netip.MustParsePrefix("10.0.0.0/24"))).To(Equal(false)) + + Expect(fakeIpamServer.syncIPs(ctx)).To(Succeed()) + + Expect(fakeIpamServer.isIPAvailable(netip.MustParseAddr("10.0.0.0"), netip.MustParsePrefix("10.0.0.0/24"))).To(Equal(false)) + Expect(fakeIpamServer.isIPAvailable(netip.MustParseAddr("10.0.0.1"), netip.MustParsePrefix("10.0.0.0/24"))).To(Equal(false)) + Expect(fakeIpamServer.isIPAvailable(netip.MustParseAddr("10.0.0.2"), netip.MustParsePrefix("10.0.0.0/24"))).To(Equal(false)) + Expect(fakeIpamServer.isIPAvailable(netip.MustParseAddr("10.0.0.3"), netip.MustParsePrefix("10.0.0.0/24"))).To(Equal(false)) + Expect(fakeIpamServer.isIPAvailable(netip.MustParseAddr("10.0.0.4"), netip.MustParsePrefix("10.0.0.0/24"))).To(Equal(false)) + + // Update the creation timestamp of the IPs + Expect(fakeIpamServer.IpamCore.IPSetCreationTimestamp( + netip.MustParseAddr("10.0.0.0"), netip.MustParsePrefix("10.0.0.0/24"), newCreationTimestamp)).Should(Succeed()) + Expect(fakeIpamServer.IpamCore.IPSetCreationTimestamp( + netip.MustParseAddr("10.0.0.1"), netip.MustParsePrefix("10.0.0.0/24"), newCreationTimestamp)).Should(Succeed()) + Expect(fakeIpamServer.IpamCore.IPSetCreationTimestamp( + netip.MustParseAddr("10.0.0.2"), netip.MustParsePrefix("10.0.0.0/24"), newCreationTimestamp)).Should(Succeed()) + Expect(fakeIpamServer.IpamCore.IPSetCreationTimestamp( + netip.MustParseAddr("10.0.0.4"), netip.MustParsePrefix("10.0.0.0/24"), newCreationTimestamp)).Should(Succeed()) + // Run sync Expect(fakeIpamServer.syncIPs(ctx)).To(Succeed()) - // Check the cache + // Check the cache after grace period + Expect(fakeIpamServer.isIPAvailable(netip.MustParseAddr("10.0.0.0"), netip.MustParsePrefix("10.0.0.0/24"))).To(Equal(false)) + Expect(fakeIpamServer.isIPAvailable(netip.MustParseAddr("10.0.0.1"), netip.MustParsePrefix("10.0.0.0/24"))).To(Equal(false)) + Expect(fakeIpamServer.isIPAvailable(netip.MustParseAddr("10.0.0.2"), netip.MustParsePrefix("10.0.0.0/24"))).To(Equal(false)) + Expect(fakeIpamServer.isIPAvailable(netip.MustParseAddr("10.0.0.3"), netip.MustParsePrefix("10.0.0.0/24"))).To(Equal(false)) + Expect(fakeIpamServer.isIPAvailable(netip.MustParseAddr("10.0.0.4"), netip.MustParsePrefix("10.0.0.0/24"))).To(Equal(true)) + + // Update the creation timestamp of the IPs + Expect(fakeIpamServer.IpamCore.IPSetCreationTimestamp( + netip.MustParseAddr("10.0.0.3"), netip.MustParsePrefix("10.0.0.0/24"), newCreationTimestamp)).Should(Succeed()) + + // Run sync + Expect(fakeIpamServer.syncIPs(ctx)).To(Succeed()) + + // Check the cache after grace period Expect(fakeIpamServer.isIPAvailable(netip.MustParseAddr("10.0.0.0"), netip.MustParsePrefix("10.0.0.0/24"))).To(Equal(false)) Expect(fakeIpamServer.isIPAvailable(netip.MustParseAddr("10.0.0.1"), netip.MustParsePrefix("10.0.0.0/24"))).To(Equal(false)) Expect(fakeIpamServer.isIPAvailable(netip.MustParseAddr("10.0.0.2"), netip.MustParsePrefix("10.0.0.0/24"))).To(Equal(false)) diff --git a/pkg/utils/testutil/liqo.go b/pkg/utils/testutil/liqo.go index 4e87375695..9d9f9c373f 100644 --- a/pkg/utils/testutil/liqo.go +++ b/pkg/utils/testutil/liqo.go @@ -176,97 +176,77 @@ func FakeForgingOpts() *forge.ForgingOpts { } } -// FakeNetworkPodCIDR returns a fake Network of type PodCIDR. -func FakeNetworkPodCIDR() *ipamv1alpha1.Network { +// FakeNetwork returns a fake Network. +func FakeNetwork(name, cidr string, labels map[string]string) *ipamv1alpha1.Network { return &ipamv1alpha1.Network{ ObjectMeta: metav1.ObjectMeta{ - Name: "pod-cidr", + Name: name, Namespace: liqoconsts.DefaultLiqoNamespace, - Labels: map[string]string{ - liqoconsts.NetworkTypeLabelKey: string(liqoconsts.NetworkTypePodCIDR), - liqoconsts.NetworkNotRemappedLabelKey: liqoconsts.NetworkNotRemappedLabelValue, - }, + Labels: labels, }, Spec: ipamv1alpha1.NetworkSpec{ - CIDR: networkingv1beta1.CIDR(PodCIDR), + CIDR: networkingv1beta1.CIDR(cidr), }, Status: ipamv1alpha1.NetworkStatus{ - CIDR: networkingv1beta1.CIDR(PodCIDR), + CIDR: networkingv1beta1.CIDR(cidr), }, } } +// FakeNetworkPodCIDR returns a fake Network of type PodCIDR. +func FakeNetworkPodCIDR() *ipamv1alpha1.Network { + return FakeNetwork("pod-cidr", PodCIDR, map[string]string{ + liqoconsts.NetworkNotRemappedLabelKey: liqoconsts.NetworkNotRemappedLabelValue, + liqoconsts.NetworkTypeLabelKey: string(liqoconsts.NetworkTypePodCIDR), + }) +} + // FakeNetworkServiceCIDR returns a fake Network of type ServiceCIDR. func FakeNetworkServiceCIDR() *ipamv1alpha1.Network { - return &ipamv1alpha1.Network{ - ObjectMeta: metav1.ObjectMeta{ - Name: "service-cidr", - Namespace: liqoconsts.DefaultLiqoNamespace, - Labels: map[string]string{ - liqoconsts.NetworkTypeLabelKey: string(liqoconsts.NetworkTypeServiceCIDR), - liqoconsts.NetworkNotRemappedLabelKey: liqoconsts.NetworkNotRemappedLabelValue, - }, - }, - Spec: ipamv1alpha1.NetworkSpec{ - CIDR: networkingv1beta1.CIDR(ServiceCIDR), - }, - Status: ipamv1alpha1.NetworkStatus{ - CIDR: networkingv1beta1.CIDR(ServiceCIDR), - }, - } + return FakeNetwork("service-cidr", ServiceCIDR, map[string]string{ + liqoconsts.NetworkNotRemappedLabelKey: liqoconsts.NetworkNotRemappedLabelValue, + liqoconsts.NetworkTypeLabelKey: string(liqoconsts.NetworkTypeServiceCIDR), + }) } // FakeNetworkExternalCIDR returns a fake Network of type ExternalCIDR. func FakeNetworkExternalCIDR() *ipamv1alpha1.Network { - return &ipamv1alpha1.Network{ - ObjectMeta: metav1.ObjectMeta{ - Name: "external-cidr", - Namespace: liqoconsts.DefaultLiqoNamespace, - Labels: map[string]string{ - liqoconsts.NetworkTypeLabelKey: string(liqoconsts.NetworkTypeExternalCIDR), - }, - }, - Spec: ipamv1alpha1.NetworkSpec{ - CIDR: networkingv1beta1.CIDR(ExternalCIDR), - }, - Status: ipamv1alpha1.NetworkStatus{ - CIDR: networkingv1beta1.CIDR(ExternalCIDR), - }, - } + return FakeNetwork("external-cidr", ExternalCIDR, map[string]string{ + liqoconsts.NetworkTypeLabelKey: string(liqoconsts.NetworkTypeExternalCIDR), + }) } // FakeNetworkInternalCIDR returns a fake Network of type InternalCIDR. func FakeNetworkInternalCIDR() *ipamv1alpha1.Network { - return &ipamv1alpha1.Network{ - ObjectMeta: metav1.ObjectMeta{ - Name: "internal-cidr", - Namespace: liqoconsts.DefaultLiqoNamespace, - Labels: map[string]string{ - liqoconsts.NetworkTypeLabelKey: string(liqoconsts.NetworkTypeInternalCIDR), - }, - }, - Spec: ipamv1alpha1.NetworkSpec{ - CIDR: networkingv1beta1.CIDR(InternalCIDR), - }, - Status: ipamv1alpha1.NetworkStatus{ - CIDR: networkingv1beta1.CIDR(InternalCIDR), - }, - } + return FakeNetwork("internal-cidr", InternalCIDR, map[string]string{ + liqoconsts.NetworkTypeLabelKey: string(liqoconsts.NetworkTypeInternalCIDR), + }) } // FakeNetworkReservedSubnet returns a fake Network of type Reserved Subnet. func FakeNetworkReservedSubnet(i int) *ipamv1alpha1.Network { - return &ipamv1alpha1.Network{ + return FakeNetwork(ReservedSubnets[i], ReservedSubnets[i], map[string]string{ + liqoconsts.NetworkTypeLabelKey: string(liqoconsts.NetworkTypeReserved), + liqoconsts.NetworkNotRemappedLabelKey: liqoconsts.NetworkNotRemappedLabelValue, + }) +} + +// FakeIP returns a fake IP. +func FakeIP(name, ip, cidr string, labels map[string]string, networkRef *corev1.ObjectReference, masquerade bool) *ipamv1alpha1.IP { + return &ipamv1alpha1.IP{ ObjectMeta: metav1.ObjectMeta{ - Name: ReservedSubnets[i], + Name: name, Namespace: liqoconsts.DefaultLiqoNamespace, - Labels: map[string]string{ - liqoconsts.NetworkTypeLabelKey: string(liqoconsts.NetworkTypeReserved), - liqoconsts.NetworkNotRemappedLabelKey: liqoconsts.NetworkNotRemappedLabelValue, - }, + Labels: labels, }, - Spec: ipamv1alpha1.NetworkSpec{ - CIDR: networkingv1beta1.CIDR(ReservedSubnets[i]), + Spec: ipamv1alpha1.IPSpec{ + IP: networkingv1beta1.IP(ip), + NetworkRef: networkRef, + Masquerade: &masquerade, + }, + Status: ipamv1alpha1.IPStatus{ + IP: networkingv1beta1.IP(ip), + CIDR: networkingv1beta1.CIDR(cidr), }, } }