Refactor network namespace cache into mockable interface

Extract the NetnsCache map and getNetnsId method from ReconcilerCommon
into a new NetNsCache interface with ReconcilerNetNsCache implementation.
This enables proper unit testing by allowing tests to inject a
MockNetNsCache instead of relying on actual filesystem operations.

Changes:
- Define NetNsCache interface with GetNetNsId and Reset methods
- Implement ReconcilerNetNsCache with the original caching logic
- Update all reconcilers to use NetNsCache.GetNetNsId() instead of
  ReconcilerCommon.getNetnsId()
- Add MockNetNsCache for testing with predefined namespace mappings
- Initialize NetNsCache in main.go and test setup

Signed-off-by: Andreas Karis <[email protected]>

Author: andreaskaris

cmd/bpfman-agent/main.go

@@ -395,6 +395,7 @@ func main() {
 		NodeName:     nodeName,
 		Containers:   containerGetter,
 		Interfaces:   &sync.Map{},
+		NetNsCache:   &bpfmanagent.ReconcilerNetNsCache{},
 	}
 
 	if err = (&bpfmanagent.ClBpfApplicationReconciler{

controllers/bpfman-agent/cl_application_program.go

@@ -125,7 +125,7 @@ func (r *ClBpfApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Req
 	r.Logger = ctrl.Log.WithName("cluster-app")
 	r.finalizer = internal.ClBpfApplicationControllerFinalizer
 	r.recType = internal.ApplicationString
-	r.NetnsCache = make(map[string]uint64)
+	r.NetNsCache.Reset()
 
 	r.Logger.Info("Enter ClusterBpfApplication Reconcile", "Name", req.Name)
 

controllers/bpfman-agent/cl_application_program_test.go

@@ -313,6 +313,7 @@ func createFakeClusterReconciler(objs []runtime.Object, bpfApp *bpfmaniov1alpha1
 		BpfmanClient: cli,
 		NodeName:     fakeNode.Name,
 		ourNode:      fakeNode,
+		NetNsCache:   MockNetNsCache{"": ptr.To(uint64(12345))},
 	}
 	r := &ClBpfApplicationReconciler{
 		ReconcilerCommon: rc,

controllers/bpfman-agent/cl_tc_program.go

@@ -213,7 +213,7 @@ func (r *ClTcProgramReconciler) printAttachInfo(attachInfoState bpfmaniov1alpha1
 }
 
 func (r *ClTcProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.ClTcAttachInfoState) (*int, error) {
-	newNetnsId := r.getNetnsId(attachInfoState.NetnsPath)
+	newNetnsId := r.NetNsCache.GetNetNsId(attachInfoState.NetnsPath)
 	if newNetnsId == nil {
 		return nil, fmt.Errorf("failed to get netnsId for path %s", attachInfoState.NetnsPath)
 	}
@@ -225,7 +225,7 @@ func (r *ClTcProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.ClTcAt
 			a.Direction == attachInfoState.Direction &&
 			helpers.GetPriority(a.Priority) == helpers.GetPriority(attachInfoState.Priority) &&
 			reflect.DeepEqual(a.ProceedOn, attachInfoState.ProceedOn) &&
-			reflect.DeepEqual(r.getNetnsId(a.NetnsPath), newNetnsId) {
+			reflect.DeepEqual(r.NetNsCache.GetNetNsId(a.NetnsPath), newNetnsId) {
 			return &i, nil
 		}
 	}

controllers/bpfman-agent/cl_tcx_program.go

@@ -178,7 +178,7 @@ func (r *ClTcxProgramReconciler) printAttachInfo(attachInfoState bpfmaniov1alpha
 }
 
 func (r *ClTcxProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.ClTcxAttachInfoState) (*int, error) {
-	newNetnsId := r.getNetnsId(attachInfoState.NetnsPath)
+	newNetnsId := r.NetNsCache.GetNetNsId(attachInfoState.NetnsPath)
 	if newNetnsId == nil {
 		return nil, fmt.Errorf("failed to get netnsId for path %s", attachInfoState.NetnsPath)
 	}
@@ -189,7 +189,7 @@ func (r *ClTcxProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.ClTcx
 		if a.InterfaceName == attachInfoState.InterfaceName &&
 			a.Direction == attachInfoState.Direction &&
 			helpers.GetPriority(a.Priority) == helpers.GetPriority(attachInfoState.Priority) &&
-			reflect.DeepEqual(r.getNetnsId(a.NetnsPath), newNetnsId) {
+			reflect.DeepEqual(r.NetNsCache.GetNetNsId(a.NetnsPath), newNetnsId) {
 			return &i, nil
 		}
 	}

controllers/bpfman-agent/cl_xdp_program.go

@@ -202,7 +202,7 @@ func (r *ClXdpProgramReconciler) printAttachInfo(attachInfoState bpfmaniov1alpha
 }
 
 func (r *ClXdpProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.ClXdpAttachInfoState) (*int, error) {
-	newNetnsId := r.getNetnsId(attachInfoState.NetnsPath)
+	newNetnsId := r.NetNsCache.GetNetNsId(attachInfoState.NetnsPath)
 	if newNetnsId == nil {
 		return nil, fmt.Errorf("failed to get netnsId for path %s", attachInfoState.NetnsPath)
 	}
@@ -213,7 +213,7 @@ func (r *ClXdpProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.ClXdp
 		if a.InterfaceName == attachInfoState.InterfaceName &&
 			helpers.GetPriority(a.Priority) == helpers.GetPriority(attachInfoState.Priority) &&
 			reflect.DeepEqual(a.ProceedOn, attachInfoState.ProceedOn) &&
-			reflect.DeepEqual(r.getNetnsId(a.NetnsPath), newNetnsId) {
+			reflect.DeepEqual(r.NetNsCache.GetNetNsId(a.NetnsPath), newNetnsId) {
 			return &i, nil
 		}
 	}

controllers/bpfman-agent/common.go

@@ -74,7 +74,57 @@ type ReconcilerCommon struct {
 	Containers   ContainerGetter
 	ourNode      *v1.Node
 	Interfaces   *sync.Map
-	NetnsCache   map[string]uint64
+	NetNsCache   NetNsCache
+}
+
+type NetNsCache interface {
+	GetNetNsId(path string) *uint64
+	Reset()
+}
+
+type ReconcilerNetNsCache struct {
+	logger logr.Logger
+	cache  map[string]uint64
+}
+
+// getNetnsId returns the network namespace ID from the given path. If the path
+// is empty, it defaults to "/host/proc/1/ns/net".  If the file is a hard link,
+// it returns the inode number of the file.  If the file is a soft link, it
+// returns the inode of the file linked. If the path is not valid or the
+// conversion to Stat_t fails, it returns nil.
+func (rnnc *ReconcilerNetNsCache) GetNetNsId(path string) *uint64 {
+	if path == "" {
+		rnnc.logger.V(1).Info("Enter getNetnsId: Path is empty.  Using /host/proc/1/ns/net")
+		path = "/host/proc/1/ns/net"
+	} else {
+		rnnc.logger.V(1).Info("Enter getNetnsId", "Path", path)
+	}
+
+	// If path is in the cache, return the cached value
+	if id, ok := rnnc.cache[path]; ok {
+		rnnc.logger.V(1).Info("Exit getNetnsId: Found in cache", "Path", path, "inode", id)
+		return &id
+	}
+
+	info, err := os.Stat(path)
+	if err != nil {
+		rnnc.logger.V(1).Info("Exit getNetnsId: Failed to stat file", "path", path, "error", err)
+		return nil
+	}
+
+	stat, ok := info.Sys().(*syscall.Stat_t)
+	if !ok {
+		rnnc.logger.V(1).Info("Exit getNetnsId: Failed to convert to Stat_t", "path", path)
+		return nil
+	}
+
+	rnnc.cache[path] = stat.Ino
+	rnnc.logger.V(1).Info("Exit getNetnsId", "Path", path, "inode", stat.Ino)
+	return &stat.Ino
+}
+
+func (rnnc *ReconcilerNetNsCache) Reset() {
+	rnnc.cache = make(map[string]uint64)
 }
 
 // ApplicationReconciler is an interface that defines the methods needed to
@@ -609,39 +659,3 @@ func isInterfacesDiscoveryEnabled(interfaceSelector *bpfmaniov1alpha1.InterfaceS
 	}
 	return false
 }
-
-// getNetnsId returns the network namespace ID from the given path. If the path
-// is empty, it defaults to "/host/proc/1/ns/net".  If the file is a hard link,
-// it returns the inode number of the file.  If the file is a soft link, it
-// returns the inode of the file linked. If the path is not valid or the
-// conversion to Stat_t fails, it returns nil.
-func (r *ReconcilerCommon) getNetnsId(path string) *uint64 {
-	if path == "" {
-		r.Logger.V(1).Info("Enter getNetnsId: Path is empty.  Using /host/proc/1/ns/net")
-		path = "/host/proc/1/ns/net"
-	} else {
-		r.Logger.V(1).Info("Enter getNetnsId", "Path", path)
-	}
-
-	// If path is in the cache, return the cached value
-	if id, ok := r.NetnsCache[path]; ok {
-		r.Logger.V(1).Info("Exit getNetnsId: Found in cache", "Path", path, "inode", id)
-		return &id
-	}
-
-	info, err := os.Stat(path)
-	if err != nil {
-		r.Logger.V(1).Info("Exit getNetnsId: Failed to stat file", "path", path, "error", err)
-		return nil
-	}
-
-	stat, ok := info.Sys().(*syscall.Stat_t)
-	if !ok {
-		r.Logger.V(1).Info("Exit getNetnsId: Failed to convert to Stat_t", "path", path)
-		return nil
-	}
-
-	r.NetnsCache[path] = stat.Ino
-	r.Logger.V(1).Info("Exit getNetnsId", "Path", path, "inode", stat.Ino)
-	return &stat.Ino
-}

controllers/bpfman-agent/ns_application_program.go

@@ -126,7 +126,7 @@ func (r *NsBpfApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Req
 	r.Logger = ctrl.Log.WithName("namespace-app")
 	r.finalizer = internal.NsBpfApplicationControllerFinalizer
 	r.recType = internal.ApplicationString
-	r.NetnsCache = make(map[string]uint64)
+	r.NetNsCache.Reset()
 
 	r.Logger.Info("Enter BpfApplication Reconcile", "Name", req.Name)
 

controllers/bpfman-agent/ns_application_program_test.go

@@ -284,6 +284,7 @@ func createFakeNamespaceReconciler(objs []runtime.Object, bpfApp *bpfmaniov1alph
 		NodeName:     fakeNode.Name,
 		ourNode:      fakeNode,
 		Containers:   testContainers,
+		NetNsCache:   MockNetNsCache{"": ptr.To(uint64(12345))},
 	}
 	r := &NsBpfApplicationReconciler{
 		ReconcilerCommon: rc,

controllers/bpfman-agent/ns_tc_program.go

@@ -177,7 +177,7 @@ func (r *NsTcProgramReconciler) printAttachInfo(attachInfoState bpfmaniov1alpha1
 }
 
 func (r *NsTcProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.TcAttachInfoState) (*int, error) {
-	newNetnsId := r.getNetnsId(attachInfoState.NetnsPath)
+	newNetnsId := r.NetNsCache.GetNetNsId(attachInfoState.NetnsPath)
 	if newNetnsId == nil {
 		return nil, fmt.Errorf("failed to get netnsId for path %s", attachInfoState.NetnsPath)
 	}
@@ -189,7 +189,7 @@ func (r *NsTcProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.TcAtta
 			a.Direction == attachInfoState.Direction &&
 			helpers.GetPriority(a.Priority) == helpers.GetPriority(attachInfoState.Priority) &&
 			reflect.DeepEqual(a.ProceedOn, attachInfoState.ProceedOn) &&
-			reflect.DeepEqual(r.getNetnsId(a.NetnsPath), newNetnsId) {
+			reflect.DeepEqual(r.NetNsCache.GetNetNsId(a.NetnsPath), newNetnsId) {
 			return &i, nil
 		}
 	}