Refactor GC and AddressPool manifests and misc changes

Signed-off-by: terassyi <[email protected]>
terassyi · Mar 30, 2024 · 508ff04 · 508ff04
1 parent 7490a8a
commit 508ff04
Show file tree

Hide file tree

Showing 17 changed files with 156 additions and 84 deletions.
diff --git a/README.md b/README.md
@@ -46,7 +46,7 @@ $ cd e2e
 $ make setup
 $ make kubernetes MODE=cni
 $ make install-sart MODE=cni
-$ make kubernetes-e2e MODE=cni
+$ make cni-e2e MODE=cni
 ```
 
 After running e2e test, we can find pods are running.

diff --git a/e2e/Makefile b/e2e/Makefile
@@ -6,6 +6,7 @@ BINDIR := $(abspath $(PWD)/bin)
 MANIFESTDIR := $(abspath $(PWD)/../manifests/base)
 LB_MANIFESTDIR := $(abspath $(PWD)/../manifests/lb)
 CNI_MANIFESTDIR := $(abspath $(PWD)/../manifests/cni)
+DUAL_MANIFESTDIR := $(abspath $(PWD)/../manifests/dual)
 TOPOLOGYDIR := $(abspath $(PWD)/topology)
 
 KIND := $(BINDIR)/kind
@@ -124,18 +125,22 @@ endif
 
 .PHONY: install-sart
 install-sart: $(MANIFESTDIR)/crd/sart.yaml
-ifeq ($(MODE),lb)
-	$(KUSTOMIZE) build $(LB_MANIFESTDIR) | $(KUBECTL) apply -f -
-else
+ifeq ($(MODE),cni)
 	$(KUSTOMIZE) build $(CNI_MANIFESTDIR) | $(KUBECTL) apply -f -
+else
+ifeq ($(MODE),dual)
+	$(KUSTOMIZE) build $(DUAL_MANIFESTDIR) | $(KUBECTL) apply -f -
+else
+	$(KUSTOMIZE) build $(LB_MANIFESTDIR) | $(KUBECTL) apply -f -
+endif
 endif
 
 .PHONY: sample
 sample:
-ifeq ($(MODE), lb)
-	$(KUSTOMIZE) build $(MANIFESTDIR)/sample | $(KUBECTL) apply -f -
-else
+ifeq ($(MODE), cni)
 	$(KUSTOMIZE) build $(CNI_MANIFESTDIR)/sample | $(KUBECTL) apply -f -
+else
+	$(KUSTOMIZE) build $(LB_MANIFESTDIR)/sample | $(KUBECTL) apply -f -
 endif
 
 

diff --git a/e2e/topology/kubernetes-cni.yaml.tmpl b/e2e/topology/kubernetes-cni.yaml.tmpl
@@ -15,6 +15,7 @@ topology:
         - ip addr add 5.5.5.5/32 dev dummy0 scope global
         - ip link set up dev dummy0
         - ip route add 6.6.6.6/32 via 192.168.0.2 dev net2
+        - ip route add 172.18.0.0/16 nexthop via 169.252.1.1 dev net0 weight 1 nexthop via 169.252.2.1 dev net1 weight 1
         - sysctl -w net.ipv4.fib_multipath_hash_policy=1
         - sysctl -p
         # Boiler plate to make FRR work
@@ -50,6 +51,7 @@ topology:
         - ip route add {{ .Worker }}/32 dev net1
         - ip route add {{ .Worker2 }}/32 dev net2
         - ip route add {{ .Worker3 }}/32 dev net3
+        - ip route add 192.168.0.0/24 via 169.252.1.0 dev net4
         - ip link add dummy0 type dummy
         - ip addr add 9.9.9.9/32 dev dummy0
         - ip link set up dev dummy0
@@ -101,6 +103,7 @@ topology:
         - ip route add {{ .Worker }}/32 dev net1
         - ip route add {{ .Worker2 }}/32 dev net2
         - ip route add {{ .Worker3 }}/32 dev net3
+        - ip route add 192.168.0.0/24 via 169.252.2.0 dev net4
         - sysctl -w net.ipv4.fib_multipath_hash_policy=1
         - sysctl -p
         # Boiler plate to make FRR work
@@ -148,8 +151,7 @@ topology:
         - ip route add {{ .Worker }}/32 src {{ .ControlPlane }} nexthop via 169.254.1.1 weight 1 nexthop via 169.253.1.1 weight 1
         - ip route add {{ .Worker2 }}/32 src {{ .ControlPlane }} nexthop via 169.254.1.1 weight 1 nexthop via 169.253.1.1 weight 1
         - ip route add {{ .Worker3 }}/32 src {{ .ControlPlane }} nexthop via 169.254.1.1 weight 1 nexthop via 169.253.1.1 weight 1
-        - ip route add 192.168.0.0/24 via 169.254.1.1 dev net0
-        - ip route add 192.168.1.0/24 via 169.253.1.1 dev net1
+        - ip route add 192.168.0.0/24 nexthop via 169.254.1.1 dev net0 weight 1 nexthop via 169.253.1.1 dev net1 weight 1
         - ip route add 6.6.6.6/32 via 169.254.1.1 dev net0
         - ip route add 9.9.9.9/32 via 169.254.1.1 dev net0
         - ip route add 7.7.7.7/32 via 169.253.1.1 dev net1
@@ -168,8 +170,7 @@ topology:
         - ip route add {{ .ControlPlane }}/32 src {{ .Worker }} nexthop via 169.254.2.1 weight 1 nexthop via 169.253.2.1 weight 1
         - ip route add {{ .Worker2 }}/32 src {{ .Worker }} nexthop via 169.254.2.1 weight 1 nexthop via 169.253.2.1 weight 1
         - ip route add {{ .Worker3 }}/32 src {{ .Worker }} nexthop via 169.254.2.1 weight 1 nexthop via 169.253.2.1 weight 1
-        - ip route add 192.168.0.0/24 via 169.254.2.1 dev net0
-        - ip route add 192.168.1.0/24 via 169.253.2.1 dev net1
+        - ip route add 192.168.0.0/24 nexthop via 169.254.2.1 dev net0 weight 1 nexthop via 169.253.2.1 dev net1 weight 1
         - ip route add 6.6.6.6/32 via 169.254.2.1 dev net0
         - ip route add 9.9.9.9/32 via 169.254.2.1 dev net0
         - ip route add 7.7.7.7/32 via 169.253.2.1 dev net1
@@ -188,8 +189,7 @@ topology:
         - ip route add {{ .ControlPlane }}/32 src {{ .Worker2 }} nexthop via 169.254.3.1 weight 1 nexthop via 169.253.3.1 weight 1
         - ip route add {{ .Worker }}/32 src {{ .Worker2 }} nexthop via 169.254.3.1 weight 1 nexthop via 169.253.3.1 weight 1
         - ip route add {{ .Worker3 }}/32 src {{ .Worker2 }} nexthop via 169.254.3.1 weight 1 nexthop via 169.253.3.1 weight 1
-        - ip route add 192.168.0.0/24 via 169.254.3.1 dev net0
-        - ip route add 192.168.1.0/24 via 169.253.3.1 dev net1
+        - ip route add 192.168.0.0/24 nexthop via 169.254.3.1 dev net0 weight 1 nexthop via 169.253.3.1 dev net1 weight 1
         - ip route add 6.6.6.6/32 via 169.254.3.1 dev net0
         - ip route add 9.9.9.9/32 via 169.254.3.1 dev net0
         - ip route add 7.7.7.7/32 via 169.253.3.1 dev net1
@@ -208,8 +208,7 @@ topology:
         - ip route add {{ .ControlPlane }}/32 src {{ .Worker3 }} nexthop via 169.254.4.1 weight 1 nexthop via 169.253.4.1 weight 1
         - ip route add {{ .Worker }}/32 src {{ .Worker3 }} nexthop via 169.254.4.1 weight 1 nexthop via 169.253.4.1 weight 1
         - ip route add {{ .Worker2 }}/32 src {{ .Worker3 }} nexthop via 169.254.4.1 weight 1 nexthop via 169.253.4.1 weight 1
-        - ip route add 192.168.0.0/24 via 169.254.4.1 dev net0
-        - ip route add 192.168.1.0/24 via 169.253.4.1 dev net1
+        - ip route add 192.168.0.0/24 nexthop via 169.254.4.1 dev net0 weight 1 nexthop via 169.253.4.1 dev net1 weight 1
         - ip route add 6.6.6.6/32 via 169.254.4.1 dev net0
         - ip route add 9.9.9.9/32 via 169.254.4.1 dev net0
         - ip route add 7.7.7.7/32 via 169.253.4.1 dev net1
@@ -218,7 +217,8 @@ topology:
       image: nicolaka/netshoot:latest
       exec:
         - ip addr add 192.168.0.2/24 dev net0 scope global
-        - ip route change default dev net0
+        - ip route add 10.0.1.0/24 via 192.168.0.1 dev net0
+        - ip route add 10.0.100.0/24 via 192.168.0.1 dev net0
         - sysctl -w net.ipv4.fib_multipath_hash_policy=1
         - sysctl -p
   links:

diff --git a/manifests/cni/sample/cluster_bgp_spine0.yaml b/manifests/cni/sample/cluster_bgp_spine0.yaml
@@ -1,7 +1,7 @@
 apiVersion: sart.terassyi.net/v1alpha2
 kind: ClusterBGP
 metadata:
-  name: clusterbgp-a
+  name: clusterbgp-spine0
 spec:
   nodeSelector:
     bgp: a

diff --git a/manifests/cni/sample/cluster_bgp_spine1.yaml b/manifests/cni/sample/cluster_bgp_spine1.yaml
@@ -1,7 +1,7 @@
 apiVersion: sart.terassyi.net/v1alpha2
 kind: ClusterBGP
 metadata:
-  name: clusterbgp-b
+  name: clusterbgp-spine1
 spec:
   nodeSelector:
     bgp: a

diff --git a/manifests/cni/sample/kustomization.yaml b/manifests/cni/sample/kustomization.yaml
@@ -1,6 +1,6 @@
 resources:
-  - cluster_bgp_a.yaml
-  - cluster_bgp_b.yaml
+  - cluster_bgp_spine0.yaml
+  - cluster_bgp_spine1.yaml
   - peer_template.yaml
   - bgp_peer.yaml
   - namespace.yaml

diff --git a/manifests/dual/kustomization.yaml b/manifests/dual/kustomization.yaml
@@ -0,0 +1,6 @@
+resources:
+  - ../cni
+
+patchesStrategicMerge:
+  - agent-patch.yaml
+  - controller-patch.yaml
diff --git a/manifests/lb/sample/lb_address_pool.yaml b/manifests/lb/sample/lb_address_pool.yaml
@@ -6,7 +6,6 @@ spec:
   cidr: 10.0.1.0/24
   type: service
   allocType: bit
-  blockSize: 24
   autoAssign: true
 ---
 apiVersion: sart.terassyi.net/v1alpha2

diff --git a/sartd/src/kubernetes/src/agent/cni/gc.rs b/sartd/src/kubernetes/src/agent/cni/gc.rs
@@ -1,10 +1,20 @@
-use std::{collections::HashMap, sync::Arc, time::Duration};
+use std::{
+    collections::{HashMap, HashSet},
+    sync::Arc,
+    time::Duration,
+};
 
-use kube::{api::{DeleteParams, ListParams}, Api, Client, ResourceExt};
+use kube::{
+    api::{DeleteParams, ListParams},
+    Api, Client, ResourceExt,
+};
 
 use sartd_ipam::manager::AllocatorSet;
 
-use crate::{agent::reconciler::node_bgp::ENV_HOSTNAME, crd::address_block::{AddressBlock, ADDRESS_BLOCK_NODE_LABEL}};
+use crate::{
+    agent::reconciler::node_bgp::ENV_HOSTNAME,
+    crd::address_block::{AddressBlock, ADDRESS_BLOCK_NODE_LABEL},
+};
 
 pub struct GarbageCollector {
     interval: Duration,
@@ -48,7 +58,8 @@ impl GarbageCollector {
             let address_block_api = Api::<AddressBlock>::all(self.client.clone());
 
             // let label_selector = form
-            let list_params = ListParams::default().labels(&format!("{}={}", ADDRESS_BLOCK_NODE_LABEL, self.node));
+            let list_params = ListParams::default()
+                .labels(&format!("{}={}", ADDRESS_BLOCK_NODE_LABEL, self.node));
             let block_list = match address_block_api.list(&list_params).await {
                 Ok(list) => list,
                 Err(e) => {
@@ -57,6 +68,9 @@ impl GarbageCollector {
                 }
             };
 
+            let mut unused = HashMap::new();
+            let mut used = HashSet::new();
+
             {
                 let alloc_set = self.allocator.clone();
                 let allocator = alloc_set.inner.lock().unwrap();
@@ -69,47 +83,53 @@ impl GarbageCollector {
                                 match self.blocks.get_mut(&block.name) {
                                     Some(status) => {
                                         if GarbageCollectorMarker::Unused.eq(status) {
-                                            *status = GarbageCollectorMarker::Deleted;
+                                            unused.insert(
+                                                block.name.clone(),
+                                                GarbageCollectorMarker::Deleted,
+                                            );
                                             tracing::info!(
                                                 block = block.name,
                                                 gc_mark =? GarbageCollectorMarker::Deleted,
                                                 "Update GC marker",
                                             );
                                         }
-                                    },
+                                    }
                                     None => {
-                                        self.blocks.insert(block.name.clone(), GarbageCollectorMarker::Unused);
+                                        unused.insert(
+                                            block.name.clone(),
+                                            GarbageCollectorMarker::Unused,
+                                        );
                                         tracing::info!(
                                             block = block.name,
                                             gc_mark =? GarbageCollectorMarker::Unused,
                                             "Add GC marker",
                                         );
                                     }
                                 }
+                            } else {
+                                used.insert(block.name.clone());
                             }
-                        },
-                        None => {
-                            match self.blocks.get_mut(&ab.name_any()) {
-                                Some(status) => {
-                                    if GarbageCollectorMarker::Unused.eq(status) {
-                                        *status = GarbageCollectorMarker::Deleted;
-                                        tracing::info!(
-                                            block = ab.name_any(),
-                                            gc_mark =? GarbageCollectorMarker::Deleted,
-                                            "Update GC marker",
-                                        );
-                                    }
-                                },
-                                None => {
-                                    self.blocks.insert(ab.name_any(), GarbageCollectorMarker::Unused);
+                        }
+                        None => match self.blocks.get_mut(&ab.name_any()) {
+                            Some(status) => {
+                                if GarbageCollectorMarker::Unused.eq(status) {
+                                    unused.insert(ab.name_any(), GarbageCollectorMarker::Deleted);
                                     tracing::info!(
                                         block = ab.name_any(),
-                                        gc_mark =? GarbageCollectorMarker::Unused,
-                                        "Add GC marker",
+                                        gc_mark =? GarbageCollectorMarker::Deleted,
+                                        "Update GC marker",
                                     );
                                 }
                             }
-                        }
+                            None => {
+                                unused.insert(ab.name_any(), GarbageCollectorMarker::Unused);
+                                tracing::info!(
+                                    block = ab.name_any(),
+                                    gc_mark =? GarbageCollectorMarker::Unused,
+                                    "Add GC marker",
+                                );
+                            }
+                        },
                     }
                 }
 
@@ -118,28 +138,40 @@ impl GarbageCollector {
                         match self.blocks.get_mut(block_name) {
                             Some(status) => {
                                 if GarbageCollectorMarker::Unused.eq(status) {
-                                    *status = GarbageCollectorMarker::Deleted;
+                                    unused.insert(
+                                        block_name.clone(),
+                                        GarbageCollectorMarker::Deleted,
+                                    );
+                                    tracing::info!(
+                                        block = block_name,
+                                        gc_mark =? GarbageCollectorMarker::Deleted,
+                                        "Update GC marker",
+                                    );
                                 }
-                                tracing::info!(
-                                    block = block_name,
-                                    gc_mark =? GarbageCollectorMarker::Deleted,
-                                    "Update GC marker",
-                                );
                             }
                             None => {
-                                self.blocks
-                                    .insert(block_name.clone(), GarbageCollectorMarker::Unused);
+                                unused.insert(block_name.clone(), GarbageCollectorMarker::Unused);
                                 tracing::info!(
                                     block = block_name,
                                     gc_mark =? GarbageCollectorMarker::Unused,
                                     "Add GC marker",
                                 );
                             }
                         }
+                    } else {
+                        used.insert(block_name.clone());
                     }
                 }
             }
 
+            for (k, v) in unused.iter() {
+                self.blocks.insert(k.clone(), *v);
+            }
+
+            for k in used.iter() {
+                self.blocks.remove(k);
+            }
+
             let mut deleted_keys = Vec::new();
             for (block, status) in self.blocks.iter() {
                 if GarbageCollectorMarker::Deleted.eq(status) {

diff --git a/sartd/src/kubernetes/src/agent/cni/server.rs b/sartd/src/kubernetes/src/agent/cni/server.rs
@@ -4,6 +4,7 @@ use std::{
     path::Path,
     str::FromStr,
     sync::Arc,
+    time::Duration,
 };
 
 use bytes::Bytes;
@@ -43,6 +44,7 @@ use crate::{
 
 use super::{
     error::Error,
+    gc::{self, GarbageCollector},
     pod::{PodAllocation, PodInfo},
 };
 
@@ -86,6 +88,11 @@ impl CNIServer {
         let inner = self.inner.lock().await;
         inner.recover().await
     }
+
+    async fn garbage_collector(&self, gc_interval: Duration) -> GarbageCollector {
+        let inner = self.inner.lock().await;
+        inner.garbage_collector(gc_interval)
+    }
 }
 
 impl CNIServerInner {
@@ -555,6 +562,10 @@ impl CNIServerInner {
         Ok(())
     }
 
+    fn garbage_collector(&self, gc_interval: Duration) -> GarbageCollector {
+        gc::GarbageCollector::new(gc_interval, self.client.clone(), self.allocator.clone())
+    }
+
     async fn get_pool(&self, pod_info: &PodInfo, pod: &Pod) -> Result<String, Error> {
         let namespace_api = Api::<Namespace>::all(self.client.clone());
         let address_pool_api = Api::<AddressPool>::all(self.client.clone());
@@ -732,6 +743,13 @@ pub async fn run(endpoint: &str, mut server: CNIServer) {
 
     server.recover().await.unwrap();
 
+    let mut garbage_collector = server.garbage_collector(Duration::from_secs(60)).await;
+
+    tracing::info!("Start Garbage collector");
+    tokio::spawn(async move {
+        garbage_collector.run().await;
+    });
+
     if endpoint.contains(".sock") {
         // use UNIX Domain Socket
         // FIXME: gRPC server via UNIX Domain Socket doesn't work