From 0849e640ff6c23237cb987b2336be391e7c69429 Mon Sep 17 00:00:00 2001 From: Andreas Karis Date: Thu, 29 Jan 2026 12:45:53 +0100 Subject: [PATCH 1/7] Change Priority field to pointer type in BPF program types Convert the Priority field from int32 to *int32 across all BPF program types (TC, TCX, XDP) and their cluster variants. This change: - Removes the default value of 1000 from kubebuilder annotations - Makes the Priority field truly optional in the API - Updates all AttachInfo and AttachInfoState structs - Adds a GetPriority helper function to handle nil pointer cases - Updates all reconcilers to use the helper function - Updates tests to use ptr.To() for priority values This allows users to omit the priority field when they want to use bpfman's default behavior, rather than always setting an explicit value. It also follows API best practices to set the default value via the controller, and not via the CRD definition. The status field should remain int32 because the controller always resolves a concrete priority value after reconciliation. Co-authored-by: Andreas Karis Co-authored-by: Andrew McDermott Signed-off-by: Andreas Karis --- apis/v1alpha1/cluster_tc_program_types.go | 3 +-- apis/v1alpha1/cluster_tcx_program_types.go | 3 +-- apis/v1alpha1/cluster_xdp_program_types.go | 3 +-- apis/v1alpha1/tc_program_types.go | 3 +-- apis/v1alpha1/tcx_program_types.go | 3 +-- apis/v1alpha1/xdp_program_types.go | 3 +-- .../cl_application_program_test.go | 7 ++++--- controllers/bpfman-agent/cl_tc_program.go | 3 ++- controllers/bpfman-agent/cl_tcx_program.go | 3 ++- controllers/bpfman-agent/cl_xdp_program.go | 3 ++- .../ns_application_program_test.go | 7 ++++--- controllers/bpfman-agent/ns_tc_program.go | 3 ++- controllers/bpfman-agent/ns_tcx_program.go | 3 ++- controllers/bpfman-agent/ns_xdp_program.go | 3 ++- .../cl_application_program_test.go | 3 ++- .../ns_application_program_test.go | 3 ++- pkg/helpers/helpers.go | 21 +++++++++++++------ 17 files changed, 45 insertions(+), 32 deletions(-) diff --git a/apis/v1alpha1/cluster_tc_program_types.go b/apis/v1alpha1/cluster_tc_program_types.go index 73978fd51..7af7620dd 100644 --- a/apis/v1alpha1/cluster_tc_program_types.go +++ b/apis/v1alpha1/cluster_tc_program_types.go @@ -74,8 +74,7 @@ type ClTcAttachInfo struct { // +optional // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=1000 - // +kubebuilder:default:=1000 - Priority int32 `json:"priority,omitempty"` + Priority *int32 `json:"priority,omitempty"` // proceedOn is an optional field and allows the user to call other TC programs // in a chain, or not call the next program in a chain based on the exit code diff --git a/apis/v1alpha1/cluster_tcx_program_types.go b/apis/v1alpha1/cluster_tcx_program_types.go index 1c629cda7..004fc027b 100644 --- a/apis/v1alpha1/cluster_tcx_program_types.go +++ b/apis/v1alpha1/cluster_tcx_program_types.go @@ -72,8 +72,7 @@ type ClTcxAttachInfo struct { // +optional // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=1000 - // +kubebuilder:default:=1000 - Priority int32 `json:"priority,omitempty"` + Priority *int32 `json:"priority,omitempty"` } type ClTcxProgramInfoState struct { diff --git a/apis/v1alpha1/cluster_xdp_program_types.go b/apis/v1alpha1/cluster_xdp_program_types.go index 3f25e801a..b135f19ea 100644 --- a/apis/v1alpha1/cluster_xdp_program_types.go +++ b/apis/v1alpha1/cluster_xdp_program_types.go @@ -61,8 +61,7 @@ type ClXdpAttachInfo struct { // +optional // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=1000 - // +kubebuilder:default:=1000 - Priority int32 `json:"priority,omitempty"` + Priority *int32 `json:"priority,omitempty"` // proceedOn is an optional field and allows the user to call other XDP // programs in a chain, or not call the next program in a chain based on the diff --git a/apis/v1alpha1/tc_program_types.go b/apis/v1alpha1/tc_program_types.go index efe56e3d4..44839eac6 100644 --- a/apis/v1alpha1/tc_program_types.go +++ b/apis/v1alpha1/tc_program_types.go @@ -70,8 +70,7 @@ type TcAttachInfo struct { // +optional // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=1000 - // +kubebuilder:default:=1000 - Priority int32 `json:"priority,omitempty"` + Priority *int32 `json:"priority,omitempty"` // proceedOn is an optional field and allows the user to call other TC programs // in a chain, or not call the next program in a chain based on the exit code diff --git a/apis/v1alpha1/tcx_program_types.go b/apis/v1alpha1/tcx_program_types.go index 9c0b54729..6f97b3157 100644 --- a/apis/v1alpha1/tcx_program_types.go +++ b/apis/v1alpha1/tcx_program_types.go @@ -71,8 +71,7 @@ type TcxAttachInfo struct { // +optional // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=1000 - // +kubebuilder:default:=1000 - Priority int32 `json:"priority,omitempty"` + Priority *int32 `json:"priority,omitempty"` } type TcxProgramInfoState struct { diff --git a/apis/v1alpha1/xdp_program_types.go b/apis/v1alpha1/xdp_program_types.go index 872c053ff..f49d33d6b 100644 --- a/apis/v1alpha1/xdp_program_types.go +++ b/apis/v1alpha1/xdp_program_types.go @@ -56,8 +56,7 @@ type XdpAttachInfo struct { // +optional // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=1000 - // +kubebuilder:default:=1000 - Priority int32 `json:"priority,omitempty"` + Priority *int32 `json:"priority,omitempty"` // proceedOn is an optional field and allows the user to call other XDP // programs in a chain, or not call the next program in a chain based on the diff --git a/controllers/bpfman-agent/cl_application_program_test.go b/controllers/bpfman-agent/cl_application_program_test.go index c124100ad..5e730ca4f 100644 --- a/controllers/bpfman-agent/cl_application_program_test.go +++ b/controllers/bpfman-agent/cl_application_program_test.go @@ -31,6 +31,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client/fake" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" @@ -127,7 +128,7 @@ func TestClBpfApplicationControllerCreate(t *testing.T) { xdpAttachInfo := bpfmaniov1alpha1.ClXdpAttachInfo{ InterfaceSelector: interfaceSelector, NetworkNamespaces: nil, - Priority: int32(priority), + Priority: ptr.To(int32(priority)), ProceedOn: proceedOn, } xdpProgram := bpfmaniov1alpha1.ClBpfApplicationProgram{ @@ -243,7 +244,7 @@ func TestClBpfApplicationControllerCreate(t *testing.T) { InterfaceSelector: interfaceSelector, NetworkNamespaces: nil, Direction: direction, - Priority: int32(priority), + Priority: ptr.To(int32(priority)), } tcProgram := bpfmaniov1alpha1.ClBpfApplicationProgram{ Name: tcBpfFunctionName, @@ -258,7 +259,7 @@ func TestClBpfApplicationControllerCreate(t *testing.T) { InterfaceSelector: interfaceSelector, NetworkNamespaces: nil, Direction: "ingress", - Priority: int32(priority), + Priority: ptr.To(int32(priority)), } tcxProgram := bpfmaniov1alpha1.ClBpfApplicationProgram{ Name: tcxBpfFunctionName, diff --git a/controllers/bpfman-agent/cl_tc_program.go b/controllers/bpfman-agent/cl_tc_program.go index edbacb499..a0531f2c1 100644 --- a/controllers/bpfman-agent/cl_tc_program.go +++ b/controllers/bpfman-agent/cl_tc_program.go @@ -23,6 +23,7 @@ import ( bpfmaniov1alpha1 "github.com/bpfman/bpfman-operator/apis/v1alpha1" internal "github.com/bpfman/bpfman-operator/internal" + "github.com/bpfman/bpfman-operator/pkg/helpers" gobpfman "github.com/bpfman/bpfman/clients/gobpfman/v1" "github.com/google/uuid" ) @@ -306,7 +307,7 @@ func (r *ClTcProgramReconciler) getExpectedLinks(ctx context.Context, attachInfo }, InterfaceName: interfaceName, NetnsPath: netnsPath, - Priority: attachInfo.Priority, + Priority: helpers.GetPriority(attachInfo.Priority), Direction: attachInfo.Direction, ProceedOn: attachInfo.ProceedOn, } diff --git a/controllers/bpfman-agent/cl_tcx_program.go b/controllers/bpfman-agent/cl_tcx_program.go index 6e7909d42..2868c694c 100644 --- a/controllers/bpfman-agent/cl_tcx_program.go +++ b/controllers/bpfman-agent/cl_tcx_program.go @@ -23,6 +23,7 @@ import ( bpfmaniov1alpha1 "github.com/bpfman/bpfman-operator/apis/v1alpha1" internal "github.com/bpfman/bpfman-operator/internal" + "github.com/bpfman/bpfman-operator/pkg/helpers" gobpfman "github.com/bpfman/bpfman/clients/gobpfman/v1" "github.com/google/uuid" ) @@ -270,7 +271,7 @@ func (r *ClTcxProgramReconciler) getExpectedLinks(ctx context.Context, attachInf }, InterfaceName: interfaceName, NetnsPath: netnsPath, - Priority: attachInfo.Priority, + Priority: helpers.GetPriority(attachInfo.Priority), Direction: attachInfo.Direction, } } diff --git a/controllers/bpfman-agent/cl_xdp_program.go b/controllers/bpfman-agent/cl_xdp_program.go index 9a6d55e45..9bde6a699 100644 --- a/controllers/bpfman-agent/cl_xdp_program.go +++ b/controllers/bpfman-agent/cl_xdp_program.go @@ -23,6 +23,7 @@ import ( bpfmaniov1alpha1 "github.com/bpfman/bpfman-operator/apis/v1alpha1" internal "github.com/bpfman/bpfman-operator/internal" + "github.com/bpfman/bpfman-operator/pkg/helpers" gobpfman "github.com/bpfman/bpfman/clients/gobpfman/v1" "github.com/google/uuid" ) @@ -294,7 +295,7 @@ func (r *ClXdpProgramReconciler) getExpectedLinks(ctx context.Context, attachInf }, InterfaceName: interfaceName, NetnsPath: netnsPath, - Priority: attachInfo.Priority, + Priority: helpers.GetPriority(attachInfo.Priority), ProceedOn: attachInfo.ProceedOn, } } diff --git a/controllers/bpfman-agent/ns_application_program_test.go b/controllers/bpfman-agent/ns_application_program_test.go index 5b368be6a..7b34b4ffd 100644 --- a/controllers/bpfman-agent/ns_application_program_test.go +++ b/controllers/bpfman-agent/ns_application_program_test.go @@ -35,6 +35,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" + "k8s.io/utils/ptr" ) func TestNsBpfApplicationReconcilerGetBpfAppState(t *testing.T) { @@ -142,7 +143,7 @@ func TestNsBpfApplicationControllerCreate(t *testing.T) { xdpAttachInfo := bpfmaniov1alpha1.XdpAttachInfo{ InterfaceSelector: interfaceSelector, NetworkNamespaces: fakeNetNamespaces, - Priority: int32(priority), + Priority: ptr.To(int32(priority)), ProceedOn: proceedOn, } @@ -160,7 +161,7 @@ func TestNsBpfApplicationControllerCreate(t *testing.T) { InterfaceSelector: interfaceSelector, NetworkNamespaces: fakeNetNamespaces, Direction: "ingress", - Priority: int32(priority), + Priority: ptr.To(int32(priority)), } tcxProgram := bpfmaniov1alpha1.BpfApplicationProgram{ Name: tcxBpfFunctionName, @@ -211,7 +212,7 @@ func TestNsBpfApplicationControllerCreate(t *testing.T) { tcAttachInfo := bpfmaniov1alpha1.TcAttachInfo{ InterfaceSelector: interfaceSelector, Direction: direction, - Priority: int32(priority), + Priority: ptr.To(int32(priority)), NetworkNamespaces: fakeNetNamespaces, ProceedOn: tcProceedOn, } diff --git a/controllers/bpfman-agent/ns_tc_program.go b/controllers/bpfman-agent/ns_tc_program.go index 492821ea3..188075cef 100644 --- a/controllers/bpfman-agent/ns_tc_program.go +++ b/controllers/bpfman-agent/ns_tc_program.go @@ -23,6 +23,7 @@ import ( bpfmaniov1alpha1 "github.com/bpfman/bpfman-operator/apis/v1alpha1" internal "github.com/bpfman/bpfman-operator/internal" + "github.com/bpfman/bpfman-operator/pkg/helpers" gobpfman "github.com/bpfman/bpfman/clients/gobpfman/v1" "github.com/google/uuid" ) @@ -294,7 +295,7 @@ func (r *NsTcProgramReconciler) getExpectedLinks(ctx context.Context, attachInfo }, InterfaceName: iface, NetnsPath: netnsPath, - Priority: attachInfo.Priority, + Priority: helpers.GetPriority(attachInfo.Priority), Direction: attachInfo.Direction, ProceedOn: attachInfo.ProceedOn, } diff --git a/controllers/bpfman-agent/ns_tcx_program.go b/controllers/bpfman-agent/ns_tcx_program.go index e0f675c79..4dec5f970 100644 --- a/controllers/bpfman-agent/ns_tcx_program.go +++ b/controllers/bpfman-agent/ns_tcx_program.go @@ -23,6 +23,7 @@ import ( bpfmaniov1alpha1 "github.com/bpfman/bpfman-operator/apis/v1alpha1" internal "github.com/bpfman/bpfman-operator/internal" + "github.com/bpfman/bpfman-operator/pkg/helpers" gobpfman "github.com/bpfman/bpfman/clients/gobpfman/v1" "github.com/google/uuid" ) @@ -293,7 +294,7 @@ func (r *NsTcxProgramReconciler) getExpectedLinks(ctx context.Context, attachInf }, InterfaceName: iface, NetnsPath: netnsPath, - Priority: attachInfo.Priority, + Priority: helpers.GetPriority(attachInfo.Priority), Direction: attachInfo.Direction, } nodeLinks = append(nodeLinks, link) diff --git a/controllers/bpfman-agent/ns_xdp_program.go b/controllers/bpfman-agent/ns_xdp_program.go index fe645ee65..0cee4707f 100644 --- a/controllers/bpfman-agent/ns_xdp_program.go +++ b/controllers/bpfman-agent/ns_xdp_program.go @@ -23,6 +23,7 @@ import ( bpfmaniov1alpha1 "github.com/bpfman/bpfman-operator/apis/v1alpha1" internal "github.com/bpfman/bpfman-operator/internal" + "github.com/bpfman/bpfman-operator/pkg/helpers" gobpfman "github.com/bpfman/bpfman/clients/gobpfman/v1" "github.com/google/uuid" ) @@ -292,7 +293,7 @@ func (r *NsXdpProgramReconciler) getExpectedLinks(ctx context.Context, attachInf }, InterfaceName: iface, NetnsPath: netnsPath, - Priority: attachInfo.Priority, + Priority: helpers.GetPriority(attachInfo.Priority), ProceedOn: attachInfo.ProceedOn, } nodeLinks = append(nodeLinks, link) diff --git a/controllers/bpfman-operator/cl_application_program_test.go b/controllers/bpfman-operator/cl_application_program_test.go index 216a4e17d..d28225231 100644 --- a/controllers/bpfman-operator/cl_application_program_test.go +++ b/controllers/bpfman-operator/cl_application_program_test.go @@ -31,6 +31,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -71,7 +72,7 @@ func appProgramReconcile(t *testing.T, multiCondition bool) { attachInfo := bpfmaniov1alpha1.ClXdpAttachInfo{ InterfaceSelector: interfaceSelector, NetworkNamespaces: nil, - Priority: int32(xdpPriority), + Priority: ptr.To(int32(xdpPriority)), ProceedOn: proceedOn, } diff --git a/controllers/bpfman-operator/ns_application_program_test.go b/controllers/bpfman-operator/ns_application_program_test.go index 6640b726f..d72ba1f7a 100644 --- a/controllers/bpfman-operator/ns_application_program_test.go +++ b/controllers/bpfman-operator/ns_application_program_test.go @@ -31,6 +31,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -71,7 +72,7 @@ func appNsProgramReconcile(t *testing.T, multiCondition bool) { }, }, }, - Priority: int32(xdpPriority), + Priority: ptr.To(int32(xdpPriority)), ProceedOn: proceedOn, } diff --git a/pkg/helpers/helpers.go b/pkg/helpers/helpers.go index fa33da347..711141850 100644 --- a/pkg/helpers/helpers.go +++ b/pkg/helpers/helpers.go @@ -21,7 +21,6 @@ import ( bpfmaniov1alpha1 "github.com/bpfman/bpfman-operator/apis/v1alpha1" bpfmanclientset "github.com/bpfman/bpfman-operator/pkg/client/clientset" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/discovery" "k8s.io/client-go/rest" @@ -33,11 +32,12 @@ import ( type ProgramType int32 const ( - Kprobe ProgramType = 2 - Tc ProgramType = 3 - Tracepoint ProgramType = 5 - Xdp ProgramType = 6 - Tracing ProgramType = 26 + Kprobe ProgramType = 2 + Tc ProgramType = 3 + Tracepoint ProgramType = 5 + Xdp ProgramType = 6 + Tracing ProgramType = 26 + defaultPriority = 1000 ) func (p ProgramType) Uint32() *uint32 { @@ -187,3 +187,12 @@ func IsBpfAppStateConditionPending(conditions []metav1.Condition) bool { return conditions[0].Type == string(bpfmaniov1alpha1.BpfAppCondPending) } + +// GetPriority reads a priority value. If priority is nil, return the default value (1000). Otherwise, return the value +// behind the pointer. +func GetPriority(priority *int32) int32 { + if priority == nil { + return defaultPriority + } + return *priority +} From 674c9b8876b729b4d9100be1adc6a82ac544d565 Mon Sep 17 00:00:00 2001 From: Andreas Karis Date: Thu, 29 Jan 2026 12:47:12 +0100 Subject: [PATCH 2/7] Regenerate CRDs and deepcopy code make generate && make manifests && make bundle Signed-off-by: Andreas Karis --- apis/v1alpha1/zz_generated.deepcopy.go | 30 +++++++++++++++++++ ...bpfman-operator.clusterserviceversion.yaml | 2 +- .../manifests/bpfman.io_bpfapplications.yaml | 3 -- .../bpfman.io_clusterbpfapplications.yaml | 3 -- .../crd/bases/bpfman.io_bpfapplications.yaml | 3 -- .../bpfman.io_clusterbpfapplications.yaml | 3 -- 6 files changed, 31 insertions(+), 13 deletions(-) diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index dd613ad7f..dc1e14bb4 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -990,6 +990,11 @@ func (in *ClTcAttachInfo) DeepCopyInto(out *ClTcAttachInfo) { *out = new(ClNetworkNamespaceSelector) (*in).DeepCopyInto(*out) } + if in.Priority != nil { + in, out := &in.Priority, &out.Priority + *out = new(int32) + **out = **in + } if in.ProceedOn != nil { in, out := &in.ProceedOn, &out.ProceedOn *out = make([]TcProceedOnValue, len(*in)) @@ -1081,6 +1086,11 @@ func (in *ClTcxAttachInfo) DeepCopyInto(out *ClTcxAttachInfo) { *out = new(ClNetworkNamespaceSelector) (*in).DeepCopyInto(*out) } + if in.Priority != nil { + in, out := &in.Priority, &out.Priority + *out = new(int32) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClTcxAttachInfo. @@ -1330,6 +1340,11 @@ func (in *ClXdpAttachInfo) DeepCopyInto(out *ClXdpAttachInfo) { *out = new(ClNetworkNamespaceSelector) (*in).DeepCopyInto(*out) } + if in.Priority != nil { + in, out := &in.Priority, &out.Priority + *out = new(int32) + **out = **in + } if in.ProceedOn != nil { in, out := &in.ProceedOn, &out.ProceedOn *out = make([]XdpProceedOnValue, len(*in)) @@ -1779,6 +1794,11 @@ func (in *TcAttachInfo) DeepCopyInto(out *TcAttachInfo) { *out = *in in.InterfaceSelector.DeepCopyInto(&out.InterfaceSelector) in.NetworkNamespaces.DeepCopyInto(&out.NetworkNamespaces) + if in.Priority != nil { + in, out := &in.Priority, &out.Priority + *out = new(int32) + **out = **in + } if in.ProceedOn != nil { in, out := &in.ProceedOn, &out.ProceedOn *out = make([]TcProceedOnValue, len(*in)) @@ -1866,6 +1886,11 @@ func (in *TcxAttachInfo) DeepCopyInto(out *TcxAttachInfo) { *out = *in in.InterfaceSelector.DeepCopyInto(&out.InterfaceSelector) in.NetworkNamespaces.DeepCopyInto(&out.NetworkNamespaces) + if in.Priority != nil { + in, out := &in.Priority, &out.Priority + *out = new(int32) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TcxAttachInfo. @@ -2029,6 +2054,11 @@ func (in *XdpAttachInfo) DeepCopyInto(out *XdpAttachInfo) { *out = *in in.InterfaceSelector.DeepCopyInto(&out.InterfaceSelector) in.NetworkNamespaces.DeepCopyInto(&out.NetworkNamespaces) + if in.Priority != nil { + in, out := &in.Priority, &out.Priority + *out = new(int32) + **out = **in + } if in.ProceedOn != nil { in, out := &in.ProceedOn, &out.ProceedOn *out = make([]XdpProceedOnValue, len(*in)) diff --git a/bundle/manifests/bpfman-operator.clusterserviceversion.yaml b/bundle/manifests/bpfman-operator.clusterserviceversion.yaml index 668197932..ff46061c3 100644 --- a/bundle/manifests/bpfman-operator.clusterserviceversion.yaml +++ b/bundle/manifests/bpfman-operator.clusterserviceversion.yaml @@ -1014,7 +1014,7 @@ metadata: capabilities: Basic Install categories: OpenShift Optional containerImage: quay.io/bpfman/bpfman-operator:latest - createdAt: "2026-01-15T18:23:35Z" + createdAt: "2026-01-29T11:46:57Z" description: The bpfman Operator is designed to manage eBPF programs for applications. features.operators.openshift.io/cnf: "false" features.operators.openshift.io/cni: "false" diff --git a/bundle/manifests/bpfman.io_bpfapplications.yaml b/bundle/manifests/bpfman.io_bpfapplications.yaml index 6776fdb82..5724c4368 100644 --- a/bundle/manifests/bpfman.io_bpfapplications.yaml +++ b/bundle/manifests/bpfman.io_bpfapplications.yaml @@ -452,7 +452,6 @@ spec: - pods type: object priority: - default: 1000 description: |- priority is an optional field and determines the execution order of the TC program relative to other TC programs attached to the same attachment point. @@ -676,7 +675,6 @@ spec: - pods type: object priority: - default: 1000 description: |- priority is an optional field and determines the execution order of the TCX program relative to other TCX programs attached to the same attachment @@ -1161,7 +1159,6 @@ spec: - pods type: object priority: - default: 1000 description: |- priority is an optional field and determines the execution order of the XDP program relative to other XDP programs attached to the same attachment diff --git a/bundle/manifests/bpfman.io_clusterbpfapplications.yaml b/bundle/manifests/bpfman.io_clusterbpfapplications.yaml index 10bf4705e..85925e01a 100644 --- a/bundle/manifests/bpfman.io_clusterbpfapplications.yaml +++ b/bundle/manifests/bpfman.io_clusterbpfapplications.yaml @@ -640,7 +640,6 @@ spec: - pods type: object priority: - default: 1000 description: |- priority is an optional field and determines the execution order of the TC program relative to other TC programs attached to the same attachment point. @@ -869,7 +868,6 @@ spec: - pods type: object priority: - default: 1000 description: |- priority is an optional field and determines the execution order of the TCX program relative to other TCX programs attached to the same attachment @@ -1444,7 +1442,6 @@ spec: - pods type: object priority: - default: 1000 description: |- priority is an optional field and determines the execution order of the XDP program relative to other XDP programs attached to the same attachment diff --git a/config/crd/bases/bpfman.io_bpfapplications.yaml b/config/crd/bases/bpfman.io_bpfapplications.yaml index 598b86664..2bc11b38b 100644 --- a/config/crd/bases/bpfman.io_bpfapplications.yaml +++ b/config/crd/bases/bpfman.io_bpfapplications.yaml @@ -452,7 +452,6 @@ spec: - pods type: object priority: - default: 1000 description: |- priority is an optional field and determines the execution order of the TC program relative to other TC programs attached to the same attachment point. @@ -676,7 +675,6 @@ spec: - pods type: object priority: - default: 1000 description: |- priority is an optional field and determines the execution order of the TCX program relative to other TCX programs attached to the same attachment @@ -1161,7 +1159,6 @@ spec: - pods type: object priority: - default: 1000 description: |- priority is an optional field and determines the execution order of the XDP program relative to other XDP programs attached to the same attachment diff --git a/config/crd/bases/bpfman.io_clusterbpfapplications.yaml b/config/crd/bases/bpfman.io_clusterbpfapplications.yaml index 199f3400d..a5345c67a 100644 --- a/config/crd/bases/bpfman.io_clusterbpfapplications.yaml +++ b/config/crd/bases/bpfman.io_clusterbpfapplications.yaml @@ -640,7 +640,6 @@ spec: - pods type: object priority: - default: 1000 description: |- priority is an optional field and determines the execution order of the TC program relative to other TC programs attached to the same attachment point. @@ -869,7 +868,6 @@ spec: - pods type: object priority: - default: 1000 description: |- priority is an optional field and determines the execution order of the TCX program relative to other TCX programs attached to the same attachment @@ -1444,7 +1442,6 @@ spec: - pods type: object priority: - default: 1000 description: |- priority is an optional field and determines the execution order of the XDP program relative to other XDP programs attached to the same attachment From 896276bc87604fa18bd77fef25d519bbe728875d Mon Sep 17 00:00:00 2001 From: Andreas Karis Date: Wed, 22 Oct 2025 13:03:26 +0200 Subject: [PATCH 3/7] Refactor BPF app tests to use table-driven pattern w/ helper func Convert cluster and namespace BPF application tests to table-driven structure for better maintainability and test coverage. Extract common test functionality into reusable helper functions: - createFakeClusterReconciler() and createFakeNamespaceReconciler() for setup - runClusterReconciler() and runNamespaceReconciler() for execution - verifyClusterBpfApplicationState() and verifyNamespaceBpfApplicationState() - verifyClusterBpfProgramState() and verifyNamespaceBpfProgramState() Inline program definitions within test cases, simplify reconciler setup in GetBpfAppState tests, and add comprehensive verification of program status across multiple reconciliation cycles. Signed-off-by: Andreas Karis --- .../cl_application_program_test.go | 517 ++++++++---------- .../ns_application_program_test.go | 461 +++++++--------- controllers/bpfman-agent/test_common.go | 89 +++ 3 files changed, 505 insertions(+), 562 deletions(-) diff --git a/controllers/bpfman-agent/cl_application_program_test.go b/controllers/bpfman-agent/cl_application_program_test.go index 5e730ca4f..21e8547c7 100644 --- a/controllers/bpfman-agent/cl_application_program_test.go +++ b/controllers/bpfman-agent/cl_application_program_test.go @@ -27,6 +27,7 @@ import ( testutils "github.com/bpfman/bpfman-operator/internal/test-utils" "github.com/stretchr/testify/require" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -39,13 +40,7 @@ import ( ) func TestClBpfApplicationReconcilerGetBpfAppState(t *testing.T) { - s := scheme.Scheme - s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, &bpfmaniov1alpha1.ClusterBpfApplication{}) - s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, &bpfmaniov1alpha1.ClusterBpfApplicationState{}) - s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, &bpfmaniov1alpha1.ClusterBpfApplicationStateList{}) - - // Create a mock ClusterBpfApplicationState that will be - // found. + // Create a mock ClusterBpfApplicationState that will be found. mockAppState := &bpfmaniov1alpha1.ClusterBpfApplicationState{ ObjectMeta: metav1.ObjectMeta{ Name: "test-app-state", @@ -55,33 +50,21 @@ func TestClBpfApplicationReconcilerGetBpfAppState(t *testing.T) { }, }, } - objs := []runtime.Object{mockAppState} - cl := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() - cli := agenttestutils.NewBpfmanClientFake() - rc := ReconcilerCommon{ - Client: cl, - Scheme: s, - BpfmanClient: cli, - NodeName: "test-node", - } - - r := &ClBpfApplicationReconciler{ - ReconcilerCommon: rc, - } - - // Set currentApp (required for getBpfAppState). - r.currentApp = &bpfmaniov1alpha1.ClusterBpfApplication{ + bpfApp := &bpfmaniov1alpha1.ClusterBpfApplication{ ObjectMeta: metav1.ObjectMeta{ Name: "test-app", }, } + fakeNode := testutils.NewNode("test-node") + r := createFakeClusterReconciler(objs, bpfApp, fakeNode) + // Set currentApp (required for getBpfAppState). + r.currentApp = bpfApp // Test getBpfAppState when currentAppState is nil. ctx := context.Background() result, err := r.getBpfAppState(ctx) - require.NoError(t, err) require.NotNil(t, result) require.Equal(t, "test-app-state", result.Name) @@ -90,215 +73,238 @@ func TestClBpfApplicationReconcilerGetBpfAppState(t *testing.T) { func TestClBpfApplicationControllerCreate(t *testing.T) { var ( // global config - appProgramName = "fakeAppProgram" - namespace = "bpfman" - bytecodePath = "/tmp/hello.o" - - fentryBpfFunctionName = "FentryTest" - fexitBpfFunctionName = "FexitTest" - uprobeBpfFunctionName = "UprobeTest" - uretprobeBpfFunctionName = "UretprobeTest" - kprobeBpfFunctionName = "KprobeTest" - kretprobeBpfFunctionName = "KretprobeTest" - tracepointBpfFunctionName = "TracepointTest" - tcBpfFunctionName = "TcTest" - tcxBpfFunctionName = "TcxTest" - xdpBpfFunctionName = "XdpTest" - - direction = bpfmaniov1alpha1.TCIngress - AttachName = "AttachNameTest" - priority = 50 - fakeNode = testutils.NewNode("fake-control-plane") - fakeInt0 = "eth0" - fakePid = int32(1000) - // fakeInt1 = "eth1" + fakePid = int32(4490) + fakeNode = testutils.NewNode("fake-control-plane") + interfaceSelector = bpfmaniov1alpha1.InterfaceSelector{ + Interfaces: []string{fakeInt0}, + } + proceedOn = []bpfmaniov1alpha1.XdpProceedOnValue{ + bpfmaniov1alpha1.XdpProceedOnValue("pass"), + bpfmaniov1alpha1.XdpProceedOnValue("dispatcher_return"), + } ctx = context.TODO() ) - programs := []bpfmaniov1alpha1.ClBpfApplicationProgram{} - - fakeInts := []string{fakeInt0} - - interfaceSelector := bpfmaniov1alpha1.InterfaceSelector{ - Interfaces: fakeInts, - } - - proceedOn := []bpfmaniov1alpha1.XdpProceedOnValue{bpfmaniov1alpha1.XdpProceedOnValue("pass"), - bpfmaniov1alpha1.XdpProceedOnValue("dispatcher_return")} - xdpAttachInfo := bpfmaniov1alpha1.ClXdpAttachInfo{ - InterfaceSelector: interfaceSelector, - NetworkNamespaces: nil, - Priority: ptr.To(int32(priority)), - ProceedOn: proceedOn, - } - xdpProgram := bpfmaniov1alpha1.ClBpfApplicationProgram{ - Name: xdpBpfFunctionName, - Type: bpfmaniov1alpha1.ProgTypeXDP, - XDP: &bpfmaniov1alpha1.ClXdpProgramInfo{ - Links: []bpfmaniov1alpha1.ClXdpAttachInfo{xdpAttachInfo}, - }, - } - programs = append(programs, xdpProgram) - - fentryProgram := bpfmaniov1alpha1.ClBpfApplicationProgram{ - Name: fentryBpfFunctionName, - Type: bpfmaniov1alpha1.ProgTypeFentry, - FEntry: &bpfmaniov1alpha1.ClFentryProgramInfo{ - ClFentryLoadInfo: bpfmaniov1alpha1.ClFentryLoadInfo{ - Function: AttachName, - }, - Links: []bpfmaniov1alpha1.ClFentryAttachInfo{ - {}, - }, - }, - } - programs = append(programs, fentryProgram) - - fexitProgram := bpfmaniov1alpha1.ClBpfApplicationProgram{ - Name: fexitBpfFunctionName, - Type: bpfmaniov1alpha1.ProgTypeFexit, - FExit: &bpfmaniov1alpha1.ClFexitProgramInfo{ - ClFexitLoadInfo: bpfmaniov1alpha1.ClFexitLoadInfo{ - Function: AttachName, - }, - Links: []bpfmaniov1alpha1.ClFexitAttachInfo{ - {Mode: bpfmaniov1alpha1.Attach}, - }, - }, - } - programs = append(programs, fexitProgram) + // Set development Logger, so we can see all logs in tests. + logf.SetLogger(zap.New(zap.UseFlagOptions(&zap.Options{Development: true}))) - kprobeProgram := bpfmaniov1alpha1.ClBpfApplicationProgram{ - Name: kprobeBpfFunctionName, - Type: bpfmaniov1alpha1.ProgTypeKprobe, - KProbe: &bpfmaniov1alpha1.ClKprobeProgramInfo{ - Links: []bpfmaniov1alpha1.ClKprobeAttachInfo{ + tcs := []struct { + testName string + programs []bpfmaniov1alpha1.ClBpfApplicationProgram + }{ + { + testName: "application test", + programs: []bpfmaniov1alpha1.ClBpfApplicationProgram{ + // XDP Program { - Function: AttachName, - Offset: 200, + Name: testXdpBpfFunctionName, + Type: bpfmaniov1alpha1.ProgTypeXDP, + XDP: &bpfmaniov1alpha1.ClXdpProgramInfo{ + Links: []bpfmaniov1alpha1.ClXdpAttachInfo{ + { + InterfaceSelector: interfaceSelector, + NetworkNamespaces: nil, + Priority: ptr.To(int32(testPriority)), + ProceedOn: proceedOn, + }, + }, + }, }, - }, - }, - } - programs = append(programs, kprobeProgram) - - kretprobeProgram := bpfmaniov1alpha1.ClBpfApplicationProgram{ - Name: kretprobeBpfFunctionName, - Type: bpfmaniov1alpha1.ProgTypeKretprobe, - KRetProbe: &bpfmaniov1alpha1.ClKretprobeProgramInfo{ - Links: []bpfmaniov1alpha1.ClKretprobeAttachInfo{ + // Fentry Program { - Function: AttachName, + Name: testFentryBpfFunctionName, + Type: bpfmaniov1alpha1.ProgTypeFentry, + FEntry: &bpfmaniov1alpha1.ClFentryProgramInfo{ + ClFentryLoadInfo: bpfmaniov1alpha1.ClFentryLoadInfo{ + Function: testAttachName, + }, + Links: []bpfmaniov1alpha1.ClFentryAttachInfo{{}}, + }, }, - }, - }, - } - programs = append(programs, kretprobeProgram) - - uprobeProgram := bpfmaniov1alpha1.ClBpfApplicationProgram{ - Name: uprobeBpfFunctionName, - Type: bpfmaniov1alpha1.ProgTypeUprobe, - UProbe: &bpfmaniov1alpha1.ClUprobeProgramInfo{ - Links: []bpfmaniov1alpha1.ClUprobeAttachInfo{ + // Fexit Program { - Function: AttachName, - Offset: 200, - Target: "/bin/bash", - Pid: &fakePid, + Name: testFexitBpfFunctionName, + Type: bpfmaniov1alpha1.ProgTypeFexit, + FExit: &bpfmaniov1alpha1.ClFexitProgramInfo{ + ClFexitLoadInfo: bpfmaniov1alpha1.ClFexitLoadInfo{ + Function: testAttachName, + }, + Links: []bpfmaniov1alpha1.ClFexitAttachInfo{ + {Mode: bpfmaniov1alpha1.Attach}, + }, + }, }, - }, - }, - } - programs = append(programs, uprobeProgram) - - uretprobeProgram := bpfmaniov1alpha1.ClBpfApplicationProgram{ - Name: uretprobeBpfFunctionName, - Type: bpfmaniov1alpha1.ProgTypeUretprobe, - URetProbe: &bpfmaniov1alpha1.ClUprobeProgramInfo{ - Links: []bpfmaniov1alpha1.ClUprobeAttachInfo{ + // Kprobe Program { - Function: AttachName, - Offset: 100, - Target: "/bin/bash", - Pid: &fakePid, + Name: testKprobeBpfFunctionName, + Type: bpfmaniov1alpha1.ProgTypeKprobe, + KProbe: &bpfmaniov1alpha1.ClKprobeProgramInfo{ + Links: []bpfmaniov1alpha1.ClKprobeAttachInfo{ + { + Function: testAttachName, + Offset: 200, + }, + }, + }, }, - }, - }, - } - programs = append(programs, uretprobeProgram) - - tracepointProgram := bpfmaniov1alpha1.ClBpfApplicationProgram{ - Name: tracepointBpfFunctionName, - Type: bpfmaniov1alpha1.ProgTypeTracepoint, - TracePoint: &bpfmaniov1alpha1.ClTracepointProgramInfo{ - Links: []bpfmaniov1alpha1.ClTracepointAttachInfo{ + // Kretprobe Program + { + Name: testKretprobeBpfFunctionName, + Type: bpfmaniov1alpha1.ProgTypeKretprobe, + KRetProbe: &bpfmaniov1alpha1.ClKretprobeProgramInfo{ + Links: []bpfmaniov1alpha1.ClKretprobeAttachInfo{ + { + Function: testAttachName, + }, + }, + }, + }, + // Uprobe Program + { + Name: testUprobeBpfFunctionName, + Type: bpfmaniov1alpha1.ProgTypeUprobe, + UProbe: &bpfmaniov1alpha1.ClUprobeProgramInfo{ + Links: []bpfmaniov1alpha1.ClUprobeAttachInfo{ + { + Function: testAttachName, + Offset: 200, + Target: "/bin/bash", + Pid: &fakePid, + }, + }, + }, + }, + // Uretprobe Program + { + Name: testUretprobeBpfFunctionName, + Type: bpfmaniov1alpha1.ProgTypeUretprobe, + URetProbe: &bpfmaniov1alpha1.ClUprobeProgramInfo{ + Links: []bpfmaniov1alpha1.ClUprobeAttachInfo{ + { + Function: testAttachName, + Offset: 100, + Target: "/bin/bash", + Pid: &fakePid, + }, + }, + }, + }, + // Tracepoint Program + { + Name: testTracepointBpfFunctionName, + Type: bpfmaniov1alpha1.ProgTypeTracepoint, + TracePoint: &bpfmaniov1alpha1.ClTracepointProgramInfo{ + Links: []bpfmaniov1alpha1.ClTracepointAttachInfo{ + { + Name: testAttachName, + }, + }, + }, + }, + // TC Program + { + Name: testTcBpfFunctionName, + Type: bpfmaniov1alpha1.ProgTypeTC, + TC: &bpfmaniov1alpha1.ClTcProgramInfo{ + Links: []bpfmaniov1alpha1.ClTcAttachInfo{ + { + InterfaceSelector: interfaceSelector, + NetworkNamespaces: nil, + Direction: testDirectionIngress, + Priority: ptr.To(int32(testPriority)), + }, + }, + }, + }, + // TCX Program { - Name: AttachName, + Name: testTcxBpfFunctionName, + Type: bpfmaniov1alpha1.ProgTypeTCX, + TCX: &bpfmaniov1alpha1.ClTcxProgramInfo{ + Links: []bpfmaniov1alpha1.ClTcxAttachInfo{ + { + InterfaceSelector: interfaceSelector, + NetworkNamespaces: nil, + Direction: testDirectionIngress, + Priority: ptr.To(int32(testPriority)), + }, + }, + }, }, }, }, } - programs = append(programs, tracepointProgram) - tcAttachInfo := bpfmaniov1alpha1.ClTcAttachInfo{ - InterfaceSelector: interfaceSelector, - NetworkNamespaces: nil, - Direction: direction, - Priority: ptr.To(int32(priority)), - } - tcProgram := bpfmaniov1alpha1.ClBpfApplicationProgram{ - Name: tcBpfFunctionName, - Type: bpfmaniov1alpha1.ProgTypeTC, - TC: &bpfmaniov1alpha1.ClTcProgramInfo{ - Links: []bpfmaniov1alpha1.ClTcAttachInfo{tcAttachInfo}, - }, - } - programs = append(programs, tcProgram) - - tcxAttachInfo := bpfmaniov1alpha1.ClTcxAttachInfo{ - InterfaceSelector: interfaceSelector, - NetworkNamespaces: nil, - Direction: "ingress", - Priority: ptr.To(int32(priority)), - } - tcxProgram := bpfmaniov1alpha1.ClBpfApplicationProgram{ - Name: tcxBpfFunctionName, - Type: bpfmaniov1alpha1.ProgTypeTCX, - TCX: &bpfmaniov1alpha1.ClTcxProgramInfo{ - Links: []bpfmaniov1alpha1.ClTcxAttachInfo{tcxAttachInfo}, - }, - } - programs = append(programs, tcxProgram) - - bpfApp := &bpfmaniov1alpha1.ClusterBpfApplication{ - ObjectMeta: metav1.ObjectMeta{ - Name: appProgramName, - }, - Spec: bpfmaniov1alpha1.ClBpfApplicationSpec{ - BpfAppCommon: bpfmaniov1alpha1.BpfAppCommon{ - NodeSelector: metav1.LabelSelector{}, - ByteCode: bpfmaniov1alpha1.ByteCodeSelector{ - Path: &bytecodePath, + for _, tc := range tcs { + bpfApp := &bpfmaniov1alpha1.ClusterBpfApplication{ + ObjectMeta: metav1.ObjectMeta{ + Name: testAppProgramName, + }, + Spec: bpfmaniov1alpha1.ClBpfApplicationSpec{ + BpfAppCommon: bpfmaniov1alpha1.BpfAppCommon{ + NodeSelector: metav1.LabelSelector{}, + ByteCode: bpfmaniov1alpha1.ByteCodeSelector{ + Path: ptr.To(testBytecodePath), + }, }, + Programs: tc.programs, }, - Programs: programs, - }, + } + + // Objects to track in the fake client. + objs := []runtime.Object{fakeNode, bpfApp} + r := createFakeClusterReconciler(objs, bpfApp, fakeNode) + + // Mock request to simulate Reconcile() being called on an event for a + // watched resource. + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: testAppProgramName, + Namespace: testNamespace, + }, + } + + // First reconcile should create the ClusterBpfApplicationState object. + r.Logger.Info("First reconcile") + runReconciler(t, ctx, r, req, r.Logger) + // Check if the BpfApplicationState Object was created successfully. + bpfAppState, err := r.getBpfAppState(ctx) + require.NoError(t, err) + verifyBpfApplicationState(t, bpfAppState, fakeNode, testAppProgramName, bpfmaniov1alpha1.BpfAppStateCondPending) + + // Do a second reconcile to make sure that the programs are loaded and attached. + r.Logger.Info("Second reconcile") + runReconciler(t, ctx, r, req, r.Logger) + // Check if the BpfApplicationState Object was created successfully. + bpfAppState2, err := r.getBpfAppState(ctx) + require.NoError(t, err) + verifyBpfApplicationState(t, bpfAppState2, fakeNode, testAppProgramName, bpfmaniov1alpha1.BpfAppStateCondSuccess) + verifyClusterBpfProgramState(t, bpfAppState2, tc.programs) + + // Do a 3rd reconcile and make sure it doesn't change. + r.Logger.Info("Third reconcile") + runReconciler(t, ctx, r, req, r.Logger) + // Check if the BpfApplicationState Object was created successfully. + bpfAppState3, err := r.getBpfAppState(ctx) + require.NoError(t, err) + verifyBpfApplicationState(t, bpfAppState3, fakeNode, testAppProgramName, bpfmaniov1alpha1.BpfAppStateCondSuccess) + // Check that the bpfAppState was not updated. + require.True(t, reflect.DeepEqual(bpfAppState2, bpfAppState3)) } +} - // Objects to track in the fake client. - objs := []runtime.Object{fakeNode, bpfApp} - +// createFakeClusterReconciler creates a fake ClBpfApplicationReconciler for testing purposes. +// It initializes the scheme with BPF application types, creates a fake Kubernetes client, +// and returns a configured reconciler instance. +func createFakeClusterReconciler(objs []runtime.Object, bpfApp *bpfmaniov1alpha1.ClusterBpfApplication, + fakeNode *v1.Node) *ClBpfApplicationReconciler { // Register operator types with the runtime scheme. s := scheme.Scheme - s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, bpfApp) - s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, &bpfmaniov1alpha1.ClusterBpfApplicationList{}) - s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, &bpfmaniov1alpha1.ClusterBpfApplication{}) - s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, &bpfmaniov1alpha1.ClusterBpfApplicationStateList{}) - s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, &bpfmaniov1alpha1.ClusterBpfApplicationState{}) + registerBpfApplicationScheme(s, true, bpfApp) // Create a fake client to mock API calls. - cl := fake.NewClientBuilder().WithStatusSubresource(bpfApp).WithStatusSubresource(&bpfmaniov1alpha1.ClusterBpfApplicationState{}).WithRuntimeObjects(objs...).Build() - + cl := fake.NewClientBuilder().WithStatusSubresource(bpfApp).WithStatusSubresource( + &bpfmaniov1alpha1.ClusterBpfApplicationState{}).WithRuntimeObjects(objs...).Build() cli := agenttestutils.NewBpfmanClientFake() rc := ReconcilerCommon{ @@ -308,99 +314,18 @@ func TestClBpfApplicationControllerCreate(t *testing.T) { NodeName: fakeNode.Name, ourNode: fakeNode, } - - // Set development Logger, so we can see all logs in tests. - logf.SetLogger(zap.New(zap.UseFlagOptions(&zap.Options{Development: true}))) - - // Create a ReconcileMemcached object with the scheme and fake client. r := &ClBpfApplicationReconciler{ ReconcilerCommon: rc, } + return r +} - // Mock request to simulate Reconcile() being called on an event for a - // watched resource . - req := reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: appProgramName, - Namespace: namespace, - }, - } - - // First reconcile should create the ClusterBpfApplicationState object - r.Logger.Info("First reconcile") - res, err := r.Reconcile(ctx, req) - require.NoError(t, err) - - r.Logger.Info("First reconcile", "res:", res, "err:", err) - - // Require no requeue - require.False(t, res.Requeue) - - // Check if the BpfApplicationState Object was created successfully - bpfAppState, err := r.getBpfAppState(ctx) - require.NoError(t, err) - - // Make sure we got bpfAppState from the api server - require.NotEqual(t, nil, bpfAppState) - - require.Equal(t, 1, len(bpfAppState.Status.Conditions)) - require.Equal(t, string(bpfmaniov1alpha1.BpfAppStateCondPending), bpfAppState.Status.Conditions[0].Type) - - require.Equal(t, fakeNode.Name, bpfAppState.Labels[internal.K8sHostLabel]) - - require.Equal(t, appProgramName, bpfAppState.Labels[internal.BpfAppStateOwner]) - - require.Equal(t, internal.ClBpfApplicationControllerFinalizer, bpfAppState.Finalizers[0]) - - // Do a second reconcile to make sure that the programs are loaded and attached. - r.Logger.Info("Second reconcile") - res, err = r.Reconcile(ctx, req) - require.NoError(t, err) - - r.Logger.Info("Second reconcile", "res:", res, "err:", err) - - // Require no requeue - require.False(t, res.Requeue) - - // Check if the BpfApplicationState Object was created successfully - bpfAppState2, err := r.getBpfAppState(ctx) - require.NoError(t, err) - - // Make sure we got bpfAppState from the api server - require.NotEqual(t, nil, bpfAppState2) - - require.Equal(t, 1, len(bpfAppState2.Status.Conditions)) - require.Equal(t, string(bpfmaniov1alpha1.BpfAppStateCondSuccess), bpfAppState2.Status.Conditions[0].Type) - - require.Equal(t, fakeNode.Name, bpfAppState2.Labels[internal.K8sHostLabel]) - - require.Equal(t, appProgramName, bpfAppState2.Labels[internal.BpfAppStateOwner]) - - require.Equal(t, internal.ClBpfApplicationControllerFinalizer, bpfAppState2.Finalizers[0]) - - for _, program := range bpfAppState2.Status.Programs { - r.Logger.Info("ProgramAttachStatus check", "program", program.Name, "status", program.ProgramLinkStatus) +// verifyClusterBpfProgramState checks that all BPF programs in the application state have +// successfully attached and that the number of programs matches the expected count. +func verifyClusterBpfProgramState(t *testing.T, bpfAppState *bpfmaniov1alpha1.ClusterBpfApplicationState, + programs []bpfmaniov1alpha1.ClBpfApplicationProgram) { + for _, program := range bpfAppState.Status.Programs { require.Equal(t, bpfmaniov1alpha1.ProgAttachSuccess, program.ProgramLinkStatus) } - - // Do a 3rd reconcile and make sure it doesn't change - r.Logger.Info("Third reconcile") - res, err = r.Reconcile(ctx, req) - require.NoError(t, err) - - // Require no requeue - require.False(t, res.Requeue) - - r.Logger.Info("Third reconcile", "res:", res, "err:", err) - - // Check if the BpfApplicationState Object was created successfully - bpfAppState3, err := r.getBpfAppState(ctx) - require.NoError(t, err) - - // Make sure we got bpfAppState from the api server and didn't create a new - // one. - require.NotEqual(t, nil, bpfAppState3) - - // Check that the bpfAppState was not updated - require.True(t, reflect.DeepEqual(bpfAppState2, bpfAppState3)) + require.Equal(t, len(programs), len(bpfAppState.Status.Programs)) } diff --git a/controllers/bpfman-agent/ns_application_program_test.go b/controllers/bpfman-agent/ns_application_program_test.go index 7b34b4ffd..218b822d2 100644 --- a/controllers/bpfman-agent/ns_application_program_test.go +++ b/controllers/bpfman-agent/ns_application_program_test.go @@ -31,6 +31,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/controller-runtime/pkg/reconcile" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -39,11 +40,6 @@ import ( ) func TestNsBpfApplicationReconcilerGetBpfAppState(t *testing.T) { - s := scheme.Scheme - s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, &bpfmaniov1alpha1.BpfApplication{}) - s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, &bpfmaniov1alpha1.BpfApplicationState{}) - s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, &bpfmaniov1alpha1.BpfApplicationStateList{}) - // Create a mock BpfApplicationState that will be found. mockAppState := &bpfmaniov1alpha1.BpfApplicationState{ ObjectMeta: metav1.ObjectMeta{ @@ -55,33 +51,22 @@ func TestNsBpfApplicationReconcilerGetBpfAppState(t *testing.T) { }, }, } - objs := []runtime.Object{mockAppState} - cl := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() - cli := agenttestutils.NewBpfmanClientFake() - rc := ReconcilerCommon{ - Client: cl, - Scheme: s, - BpfmanClient: cli, - NodeName: "test-node", - } - - r := &NsBpfApplicationReconciler{ - ReconcilerCommon: rc, - } - - // Set currentApp (required for getBpfAppState). - r.currentApp = &bpfmaniov1alpha1.BpfApplication{ + bpfApp := &bpfmaniov1alpha1.BpfApplication{ ObjectMeta: metav1.ObjectMeta{ Name: "test-app", Namespace: "test-namespace", }, } + fakeNode := testutils.NewNode("test-node") + r := createFakeNamespaceReconciler(objs, bpfApp, fakeNode, nil) + // Set currentApp (required for getBpfAppState). + r.currentApp = bpfApp + // Test getBpfAppState when currentAppState is nil. ctx := context.Background() result, err := r.getBpfAppState(ctx) - require.NoError(t, err) require.NotNil(t, result) require.Equal(t, "test-app-state", result.Name) @@ -89,285 +74,229 @@ func TestNsBpfApplicationReconcilerGetBpfAppState(t *testing.T) { func TestNsBpfApplicationControllerCreate(t *testing.T) { var ( - // global config - appProgramName = "fakeAppProgram" - namespace = "bpfman" - bytecodePath = "/tmp/hello.o" - - xdpBpfFunctionName = "XdpTest" - tcxBpfFunctionName = "TcxTest" - uprobeBpfFunctionName = "UprobeTest" - uretprobeBpfFunctionName = "UretprobeTest" - tcBpfFunctionName = "TcTest" - - direction = bpfmaniov1alpha1.TCIngress - AttachName = "AttachNameTest" - priority = 50 - fakeNode = testutils.NewNode("fake-control-plane") - fakeInt0 = "eth0" - // fakeInt1 = "eth1" - fakePodName = "my-pod" - fakeContainerName = "my-container-1" - fakePid = int32(4490) - + fakePid = int32(1000) + fakeNode = testutils.NewNode("fake-control-plane") + interfaceSelector = bpfmaniov1alpha1.InterfaceSelector{ + Interfaces: []string{fakeInt0}, + } + fakeNetNamespaces = bpfmaniov1alpha1.NetworkNamespaceSelector{ + Pods: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "test", + }, + }, + } + proceedOn = []bpfmaniov1alpha1.XdpProceedOnValue{ + bpfmaniov1alpha1.XdpProceedOnValue("pass"), + bpfmaniov1alpha1.XdpProceedOnValue("dispatcher_return"), + } + tcProceedOn = []bpfmaniov1alpha1.TcProceedOnValue{bpfmaniov1alpha1.TcProceedOnValue("ok"), + bpfmaniov1alpha1.TcProceedOnValue("shot")} + fakeContainers = bpfmaniov1alpha1.ContainerSelector{ + Pods: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "test", + }, + }, + } ctx = context.TODO() ) - programs := []bpfmaniov1alpha1.BpfApplicationProgram{} + // Set development Logger, so we can see all logs in tests. + logf.SetLogger(zap.New(zap.UseFlagOptions(&zap.Options{Development: true}))) - fakeContainers := bpfmaniov1alpha1.ContainerSelector{ - Pods: metav1.LabelSelector{ - MatchLabels: map[string]string{ - "app": "test", + tcs := []struct { + testName string + programs []bpfmaniov1alpha1.BpfApplicationProgram + }{ + { + testName: "application test", + programs: []bpfmaniov1alpha1.BpfApplicationProgram{ + // XDP Program + { + Name: testXdpBpfFunctionName, + Type: bpfmaniov1alpha1.ProgTypeXDP, + XDP: &bpfmaniov1alpha1.XdpProgramInfo{ + Links: []bpfmaniov1alpha1.XdpAttachInfo{ + { + InterfaceSelector: interfaceSelector, + NetworkNamespaces: fakeNetNamespaces, + Priority: ptr.To(int32(testPriority)), + ProceedOn: proceedOn, + }, + }, + }, + }, + // TCX Program + { + Name: testTcxBpfFunctionName, + Type: bpfmaniov1alpha1.ProgTypeTCX, + TCX: &bpfmaniov1alpha1.TcxProgramInfo{ + Links: []bpfmaniov1alpha1.TcxAttachInfo{ + { + InterfaceSelector: interfaceSelector, + NetworkNamespaces: fakeNetNamespaces, + Direction: testDirectionIngress, + Priority: ptr.To(int32(testPriority)), + }, + }, + }, + }, + // Uprobe Program + { + Name: testUprobeBpfFunctionName, + Type: bpfmaniov1alpha1.ProgTypeUprobe, + UProbe: &bpfmaniov1alpha1.UprobeProgramInfo{ + Links: []bpfmaniov1alpha1.UprobeAttachInfo{ + { + Function: testAttachName, + Offset: 200, + Target: "/bin/bash", + Pid: &fakePid, + Containers: fakeContainers, + }, + }, + }, + }, + // Uretprobe Program + { + Name: testUretprobeBpfFunctionName, + Type: bpfmaniov1alpha1.ProgTypeUretprobe, + URetProbe: &bpfmaniov1alpha1.UprobeProgramInfo{ + Links: []bpfmaniov1alpha1.UprobeAttachInfo{ + { + Function: testAttachName, + Offset: 0, + Target: "/bin/bash", + Pid: &fakePid, + Containers: fakeContainers, + }, + }, + }, + }, + // TC Program + { + Name: testTcBpfFunctionName, + Type: bpfmaniov1alpha1.ProgTypeTC, + TC: &bpfmaniov1alpha1.TcProgramInfo{ + Links: []bpfmaniov1alpha1.TcAttachInfo{ + { + InterfaceSelector: interfaceSelector, + Direction: testDirectionIngress, + Priority: ptr.To(int32(testPriority)), + NetworkNamespaces: fakeNetNamespaces, + ProceedOn: tcProceedOn, + }, + }, + }, + }, }, }, } - fakeNetNamespaces := bpfmaniov1alpha1.NetworkNamespaceSelector{ - Pods: metav1.LabelSelector{ - MatchLabels: map[string]string{ - "app": "test", + for _, tc := range tcs { + bpfApp := &bpfmaniov1alpha1.BpfApplication{ + ObjectMeta: metav1.ObjectMeta{ + Name: testAppProgramName, + Namespace: testNamespace, }, - }, - } - - fakeInts := []string{fakeInt0} - - interfaceSelector := bpfmaniov1alpha1.InterfaceSelector{ - Interfaces: fakeInts, - } - - proceedOn := []bpfmaniov1alpha1.XdpProceedOnValue{bpfmaniov1alpha1.XdpProceedOnValue("pass"), - bpfmaniov1alpha1.XdpProceedOnValue("dispatcher_return")} - - xdpAttachInfo := bpfmaniov1alpha1.XdpAttachInfo{ - InterfaceSelector: interfaceSelector, - NetworkNamespaces: fakeNetNamespaces, - Priority: ptr.To(int32(priority)), - ProceedOn: proceedOn, - } - - xdpProgram := bpfmaniov1alpha1.BpfApplicationProgram{ - Name: xdpBpfFunctionName, - Type: bpfmaniov1alpha1.ProgTypeXDP, - XDP: &bpfmaniov1alpha1.XdpProgramInfo{ - Links: []bpfmaniov1alpha1.XdpAttachInfo{xdpAttachInfo}, - }, - } - - programs = append(programs, xdpProgram) - - tcxAttachInfo := bpfmaniov1alpha1.TcxAttachInfo{ - InterfaceSelector: interfaceSelector, - NetworkNamespaces: fakeNetNamespaces, - Direction: "ingress", - Priority: ptr.To(int32(priority)), - } - tcxProgram := bpfmaniov1alpha1.BpfApplicationProgram{ - Name: tcxBpfFunctionName, - Type: bpfmaniov1alpha1.ProgTypeTCX, - TCX: &bpfmaniov1alpha1.TcxProgramInfo{ - Links: []bpfmaniov1alpha1.TcxAttachInfo{tcxAttachInfo}, - }, - } - programs = append(programs, tcxProgram) - - uprobeProgram := bpfmaniov1alpha1.BpfApplicationProgram{ - Name: uprobeBpfFunctionName, - Type: bpfmaniov1alpha1.ProgTypeUprobe, - UProbe: &bpfmaniov1alpha1.UprobeProgramInfo{ - Links: []bpfmaniov1alpha1.UprobeAttachInfo{ - { - Function: AttachName, - Offset: 200, - Target: "/bin/bash", - Pid: &fakePid, - Containers: fakeContainers, + Spec: bpfmaniov1alpha1.BpfApplicationSpec{ + BpfAppCommon: bpfmaniov1alpha1.BpfAppCommon{ + NodeSelector: metav1.LabelSelector{}, + ByteCode: bpfmaniov1alpha1.ByteCodeSelector{ + Path: ptr.To(testBytecodePath), + }, }, + Programs: tc.programs, }, - }, - } - programs = append(programs, uprobeProgram) + } - uretprobeProgram := bpfmaniov1alpha1.BpfApplicationProgram{ - Name: uretprobeBpfFunctionName, - Type: bpfmaniov1alpha1.ProgTypeUretprobe, - URetProbe: &bpfmaniov1alpha1.UprobeProgramInfo{ - Links: []bpfmaniov1alpha1.UprobeAttachInfo{ + testContainers := FakeContainerGetter{ + containerList: &[]ContainerInfo{ { - Function: AttachName, - Offset: 0, - Target: "/bin/bash", - Pid: &fakePid, - Containers: fakeContainers, + podName: fakePodName, + containerName: fakeContainerName, + pid: fakePid, }, }, - }, - } - programs = append(programs, uretprobeProgram) - - tcProceedOn := []bpfmaniov1alpha1.TcProceedOnValue{bpfmaniov1alpha1.TcProceedOnValue("ok"), - bpfmaniov1alpha1.TcProceedOnValue("shot")} - - tcAttachInfo := bpfmaniov1alpha1.TcAttachInfo{ - InterfaceSelector: interfaceSelector, - Direction: direction, - Priority: ptr.To(int32(priority)), - NetworkNamespaces: fakeNetNamespaces, - ProceedOn: tcProceedOn, - } - tcProgram := bpfmaniov1alpha1.BpfApplicationProgram{ - Name: tcBpfFunctionName, - Type: bpfmaniov1alpha1.ProgTypeTC, - TC: &bpfmaniov1alpha1.TcProgramInfo{ - Links: []bpfmaniov1alpha1.TcAttachInfo{tcAttachInfo}, - }, - } - programs = append(programs, tcProgram) - - bpfApp := &bpfmaniov1alpha1.BpfApplication{ - ObjectMeta: metav1.ObjectMeta{ - Name: appProgramName, - Namespace: namespace, - }, - Spec: bpfmaniov1alpha1.BpfApplicationSpec{ - BpfAppCommon: bpfmaniov1alpha1.BpfAppCommon{ - NodeSelector: metav1.LabelSelector{}, - ByteCode: bpfmaniov1alpha1.ByteCodeSelector{ - Path: &bytecodePath, - }, + } + + // Objects to track in the fake client. + objs := []runtime.Object{fakeNode, bpfApp} + r := createFakeNamespaceReconciler(objs, bpfApp, fakeNode, &testContainers) + + // Mock request to simulate Reconcile() being called on an event for a + // watched resource. + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: testAppProgramName, + Namespace: testNamespace, }, - Programs: programs, - }, + } + + // First reconcile should create the BpfApplicationState object. + r.Logger.Info("First reconcile") + runReconciler(t, ctx, r, req, r.Logger) + // Check if the BpfApplicationState Object was created successfully. + bpfAppState, err := r.getBpfAppState(ctx) + require.NoError(t, err) + verifyBpfApplicationState(t, bpfAppState, fakeNode, testAppProgramName, bpfmaniov1alpha1.BpfAppStateCondPending) + + // Do a second reconcile to make sure that the programs are loaded and attached. + r.Logger.Info("Second reconcile") + runReconciler(t, ctx, r, req, r.Logger) + // Check if the BpfApplicationState Object was created successfully. + bpfAppState2, err := r.getBpfAppState(ctx) + require.NoError(t, err) + verifyBpfApplicationState(t, bpfAppState2, fakeNode, testAppProgramName, bpfmaniov1alpha1.BpfAppStateCondSuccess) + verifyNamespaceBpfProgramState(t, bpfAppState2, tc.programs) + + // Do a 3rd reconcile and make sure it doesn't change. + r.Logger.Info("Third reconcile") + runReconciler(t, ctx, r, req, r.Logger) + // Check if the BpfApplicationState Object was created successfully. + bpfAppState3, err := r.getBpfAppState(ctx) + require.NoError(t, err) + verifyBpfApplicationState(t, bpfAppState3, fakeNode, testAppProgramName, bpfmaniov1alpha1.BpfAppStateCondSuccess) + // Check that the bpfAppState was not updated. + require.True(t, reflect.DeepEqual(bpfAppState2, bpfAppState3)) } +} - // Objects to track in the fake client. - objs := []runtime.Object{fakeNode, bpfApp} - +// createFakeNamespaceReconciler creates a fake NsBpfApplicationReconciler for testing purposes. +// It initializes the scheme with BPF application types, creates a fake Kubernetes client with +// a container getter, and returns a configured reconciler instance. +func createFakeNamespaceReconciler(objs []runtime.Object, bpfApp *bpfmaniov1alpha1.BpfApplication, fakeNode *v1.Node, + testContainers *FakeContainerGetter) *NsBpfApplicationReconciler { // Register operator types with the runtime scheme. s := scheme.Scheme - s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, bpfApp) - s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, &bpfmaniov1alpha1.BpfApplicationList{}) - s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, &bpfmaniov1alpha1.BpfApplication{}) - s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, &bpfmaniov1alpha1.BpfApplicationStateList{}) - s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, &bpfmaniov1alpha1.BpfApplicationState{}) + registerBpfApplicationScheme(s, false, bpfApp) // Create a fake client to mock API calls. - cl := fake.NewClientBuilder().WithStatusSubresource(bpfApp).WithStatusSubresource(&bpfmaniov1alpha1.BpfApplicationState{}).WithRuntimeObjects(objs...).Build() - + cl := fake.NewClientBuilder().WithStatusSubresource(bpfApp).WithStatusSubresource( + &bpfmaniov1alpha1.BpfApplicationState{}).WithRuntimeObjects(objs...).Build() cli := agenttestutils.NewBpfmanClientFake() - testContainers := FakeContainerGetter{ - containerList: &[]ContainerInfo{ - { - podName: fakePodName, - containerName: fakeContainerName, - pid: fakePid, - }, - }, - } - rc := ReconcilerCommon{ Client: cl, Scheme: s, BpfmanClient: cli, NodeName: fakeNode.Name, ourNode: fakeNode, - Containers: &testContainers, + Containers: testContainers, } - - // Set development Logger, so we can see all logs in tests. - logf.SetLogger(zap.New(zap.UseFlagOptions(&zap.Options{Development: true}))) - - // Create a ReconcileMemcached object with the scheme and fake client. r := &NsBpfApplicationReconciler{ ReconcilerCommon: rc, } + return r +} - // Mock request to simulate Reconcile() being called on an event for a - // watched resource . - req := reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: appProgramName, - Namespace: namespace, - }, - } - - // First reconcile should create the BpfApplicationState object - r.Logger.Info("First reconcile") - res, err := r.Reconcile(ctx, req) - require.NoError(t, err) - - r.Logger.Info("First reconcile", "res:", res, "err:", err) - - // Require no requeue - require.False(t, res.Requeue) - - // Check if the BpfApplicationState Object was created successfully - bpfAppState, err := r.getBpfAppState(ctx) - require.NoError(t, err) - - // Make sure we got bpfAppState from the api server - require.NotEqual(t, nil, bpfAppState) - - require.Equal(t, 1, len(bpfAppState.Status.Conditions)) - require.Equal(t, string(bpfmaniov1alpha1.BpfAppStateCondPending), bpfAppState.Status.Conditions[0].Type) - - require.Equal(t, fakeNode.Name, bpfAppState.Labels[internal.K8sHostLabel]) - - require.Equal(t, appProgramName, bpfAppState.Labels[internal.BpfAppStateOwner]) - - require.Equal(t, internal.NsBpfApplicationControllerFinalizer, bpfAppState.Finalizers[0]) - - // Do a second reconcile to make sure that the programs are loaded and attached. - r.Logger.Info("Second reconcile") - res, err = r.Reconcile(ctx, req) - require.NoError(t, err) - - r.Logger.Info("Second reconcile", "res:", res, "err:", err) - - // Require no requeue - require.False(t, res.Requeue) - - // Check if the BpfApplicationState Object was created successfully - bpfAppState2, err := r.getBpfAppState(ctx) - require.NoError(t, err) - - // Make sure we got bpfAppState from the api server - require.NotEqual(t, nil, bpfAppState2) - - require.Equal(t, 1, len(bpfAppState2.Status.Conditions)) - require.Equal(t, string(bpfmaniov1alpha1.BpfAppStateCondSuccess), bpfAppState2.Status.Conditions[0].Type) - - require.Equal(t, fakeNode.Name, bpfAppState2.Labels[internal.K8sHostLabel]) - - require.Equal(t, appProgramName, bpfAppState2.Labels[internal.BpfAppStateOwner]) - - require.Equal(t, internal.NsBpfApplicationControllerFinalizer, bpfAppState2.Finalizers[0]) - - for _, program := range bpfAppState2.Status.Programs { - r.Logger.Info("ProgramAttachStatus check", "program", program.Name, "status", program.ProgramLinkStatus) +// verifyNamespaceBpfProgramState checks that all BPF programs in the application state have +// successfully attached and that the number of programs matches the expected count. +func verifyNamespaceBpfProgramState(t *testing.T, bpfAppState *bpfmaniov1alpha1.BpfApplicationState, + programs []bpfmaniov1alpha1.BpfApplicationProgram) { + for _, program := range bpfAppState.Status.Programs { require.Equal(t, bpfmaniov1alpha1.ProgAttachSuccess, program.ProgramLinkStatus) } - - // Do a 3rd reconcile and make sure it doesn't change - r.Logger.Info("Third reconcile") - res, err = r.Reconcile(ctx, req) - require.NoError(t, err) - - // Require no requeue - require.False(t, res.Requeue) - - r.Logger.Info("Third reconcile", "res:", res, "err:", err) - - // Check if the BpfApplicationState Object was created successfully - bpfAppState3, err := r.getBpfAppState(ctx) - require.NoError(t, err) - - // Make sure we got bpfAppState from the api server and didn't create a new - // one. - require.NotEqual(t, nil, bpfAppState3) - - // Check that the bpfAppState was not updated - require.True(t, reflect.DeepEqual(bpfAppState2, bpfAppState3)) + require.Equal(t, len(programs), len(bpfAppState.Status.Programs)) } diff --git a/controllers/bpfman-agent/test_common.go b/controllers/bpfman-agent/test_common.go index 17b54373e..50b3bdfc8 100644 --- a/controllers/bpfman-agent/test_common.go +++ b/controllers/bpfman-agent/test_common.go @@ -21,14 +21,43 @@ import ( "testing" bpfmaniov1alpha1 "github.com/bpfman/bpfman-operator/apis/v1alpha1" + "github.com/bpfman/bpfman-operator/internal" "github.com/go-logr/logr" + "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/stretchr/testify/require" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" clientGoFake "k8s.io/client-go/kubernetes/fake" ) +const ( + // global config + testAppProgramName = "fakeAppProgram" + testNamespace = "bpfman" + testBytecodePath = "/tmp/hello.o" + + testDirectionIngress = bpfmaniov1alpha1.TCIngress + testAttachName = "AttachNameTest" + testPriority = 50 + fakeInt0 = "eth0" + + fakePodName = "my-pod" + fakeContainerName = "my-container-1" + + testFentryBpfFunctionName = "FentryTest" + testFexitBpfFunctionName = "FexitTest" + testUprobeBpfFunctionName = "UprobeTest" + testUretprobeBpfFunctionName = "UretprobeTest" + testKprobeBpfFunctionName = "KprobeTest" + testKretprobeBpfFunctionName = "KretprobeTest" + testTracepointBpfFunctionName = "TracepointTest" + testTcBpfFunctionName = "TcTest" + testTcxBpfFunctionName = "TcxTest" + testXdpBpfFunctionName = "XdpTest" +) + type FakeContainerGetter struct { containerList *[]ContainerInfo } @@ -111,3 +140,63 @@ func TestGetPods(t *testing.T) { require.Len(t, podList.Items, 1) require.Equal(t, "test-pod", podList.Items[0].Name) } + +// verifyBpfApplicationState validates the common properties of a BPF application state object. +// It checks that the state has the expected condition, correct node and owner labels, and proper +// finalizer. This function works with both ClusterBpfApplicationState and BpfApplicationState types, +// but panics if another type is provided. +func verifyBpfApplicationState(t *testing.T, bpfAppState any, fakeNode *v1.Node, appProgramName string, + expectedCondition bpfmaniov1alpha1.BpfApplicationStateConditionType) { + var conditions []metav1.Condition + var labels map[string]string + var finalizers []string + var expectedFinalizer string + switch b := bpfAppState.(type) { + case *bpfmaniov1alpha1.ClusterBpfApplicationState: + conditions = b.Status.Conditions + labels = b.Labels + finalizers = b.Finalizers + expectedFinalizer = internal.ClBpfApplicationControllerFinalizer + case *bpfmaniov1alpha1.BpfApplicationState: + conditions = b.Status.Conditions + labels = b.Labels + finalizers = b.Finalizers + expectedFinalizer = internal.NsBpfApplicationControllerFinalizer + default: + panic("invalid type for bpfAppState") + } + + require.NotEqual(t, nil, bpfAppState) + require.Equal(t, 1, len(conditions)) + require.Equal(t, string(expectedCondition), conditions[0].Type) + require.Equal(t, fakeNode.Name, labels[internal.K8sHostLabel]) + require.Equal(t, appProgramName, labels[internal.BpfAppStateOwner]) + require.Equal(t, expectedFinalizer, finalizers[0]) +} + +// runReconciler executes a reconciliation cycle and verifies that it completes +// successfully without errors and without requiring a requeue. +func runReconciler(t *testing.T, ctx context.Context, r reconcile.Reconciler, req reconcile.Request, + logger logr.Logger) { + res, err := r.Reconcile(ctx, req) + require.NoError(t, err) + logger.Info("Reconcile result", "res:", res, "err:", err) + // Require no requeue. + require.False(t, res.Requeue) +} + +// registerBpfApplicationScheme is a helper to register bpf application schemes for the mock reconciler. +func registerBpfApplicationScheme(s *runtime.Scheme, isClusterScoped bool, bpfApp runtime.Object) { + if isClusterScoped { + s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, &bpfmaniov1alpha1.ClusterBpfApplication{}) + s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, &bpfmaniov1alpha1.ClusterBpfApplicationList{}) + s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, &bpfmaniov1alpha1.ClusterBpfApplicationState{}) + s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, &bpfmaniov1alpha1.ClusterBpfApplicationStateList{}) + } else { + s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, &bpfmaniov1alpha1.BpfApplication{}) + s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, &bpfmaniov1alpha1.BpfApplicationList{}) + s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, &bpfmaniov1alpha1.BpfApplicationState{}) + s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, &bpfmaniov1alpha1.BpfApplicationStateList{}) + } + s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, bpfApp) +} From 543e73928f9dc129bd6de9f97570b4a60715fb9e Mon Sep 17 00:00:00 2001 From: Andreas Karis Date: Thu, 23 Oct 2025 00:44:32 +0200 Subject: [PATCH 4/7] 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 --- cmd/bpfman-agent/main.go | 1 + .../bpfman-agent/cl_application_program.go | 2 +- .../cl_application_program_test.go | 1 + controllers/bpfman-agent/cl_tc_program.go | 4 +- controllers/bpfman-agent/cl_tcx_program.go | 4 +- controllers/bpfman-agent/cl_xdp_program.go | 4 +- controllers/bpfman-agent/common.go | 89 +++++++++++-------- .../bpfman-agent/ns_application_program.go | 2 +- .../ns_application_program_test.go | 1 + controllers/bpfman-agent/ns_tc_program.go | 4 +- controllers/bpfman-agent/ns_tcx_program.go | 4 +- controllers/bpfman-agent/ns_xdp_program.go | 4 +- controllers/bpfman-agent/test_common.go | 8 ++ 13 files changed, 77 insertions(+), 51 deletions(-) diff --git a/cmd/bpfman-agent/main.go b/cmd/bpfman-agent/main.go index c9f7e717c..03b1240bc 100644 --- a/cmd/bpfman-agent/main.go +++ b/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{ diff --git a/controllers/bpfman-agent/cl_application_program.go b/controllers/bpfman-agent/cl_application_program.go index 64cbf58b4..6fa1c09fc 100644 --- a/controllers/bpfman-agent/cl_application_program.go +++ b/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) diff --git a/controllers/bpfman-agent/cl_application_program_test.go b/controllers/bpfman-agent/cl_application_program_test.go index 21e8547c7..8269e0f00 100644 --- a/controllers/bpfman-agent/cl_application_program_test.go +++ b/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, diff --git a/controllers/bpfman-agent/cl_tc_program.go b/controllers/bpfman-agent/cl_tc_program.go index a0531f2c1..7f93577e2 100644 --- a/controllers/bpfman-agent/cl_tc_program.go +++ b/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 && a.Priority == 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 } } diff --git a/controllers/bpfman-agent/cl_tcx_program.go b/controllers/bpfman-agent/cl_tcx_program.go index 2868c694c..00ac8febf 100644 --- a/controllers/bpfman-agent/cl_tcx_program.go +++ b/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 && a.Priority == attachInfoState.Priority && - reflect.DeepEqual(r.getNetnsId(a.NetnsPath), newNetnsId) { + reflect.DeepEqual(r.NetNsCache.GetNetNsId(a.NetnsPath), newNetnsId) { return &i, nil } } diff --git a/controllers/bpfman-agent/cl_xdp_program.go b/controllers/bpfman-agent/cl_xdp_program.go index 9bde6a699..3f6dd2b60 100644 --- a/controllers/bpfman-agent/cl_xdp_program.go +++ b/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 && a.Priority == 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 } } diff --git a/controllers/bpfman-agent/common.go b/controllers/bpfman-agent/common.go index 601b58ceb..0a690c9a3 100644 --- a/controllers/bpfman-agent/common.go +++ b/controllers/bpfman-agent/common.go @@ -74,7 +74,58 @@ 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 +} + +// Reset reinitializes the cache to an empty state. +func (rnnc *ReconcilerNetNsCache) Reset() { + rnnc.cache = make(map[string]uint64) } // ApplicationReconciler is an interface that defines the methods needed to @@ -609,39 +660,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 -} diff --git a/controllers/bpfman-agent/ns_application_program.go b/controllers/bpfman-agent/ns_application_program.go index a642e0642..8a678674e 100644 --- a/controllers/bpfman-agent/ns_application_program.go +++ b/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) diff --git a/controllers/bpfman-agent/ns_application_program_test.go b/controllers/bpfman-agent/ns_application_program_test.go index 218b822d2..1bbf2a64a 100644 --- a/controllers/bpfman-agent/ns_application_program_test.go +++ b/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{"/host/proc/1000/ns/net": ptr.To(uint64(12345))}, } r := &NsBpfApplicationReconciler{ ReconcilerCommon: rc, diff --git a/controllers/bpfman-agent/ns_tc_program.go b/controllers/bpfman-agent/ns_tc_program.go index 188075cef..a91f0ee4f 100644 --- a/controllers/bpfman-agent/ns_tc_program.go +++ b/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 && a.Priority == 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 } } diff --git a/controllers/bpfman-agent/ns_tcx_program.go b/controllers/bpfman-agent/ns_tcx_program.go index 4dec5f970..58d51621f 100644 --- a/controllers/bpfman-agent/ns_tcx_program.go +++ b/controllers/bpfman-agent/ns_tcx_program.go @@ -176,7 +176,7 @@ func (r *NsTcxProgramReconciler) printAttachInfo(attachInfoState bpfmaniov1alpha } func (r *NsTcxProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.TcxAttachInfoState) (*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) } @@ -187,7 +187,7 @@ func (r *NsTcxProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.TcxAt if a.InterfaceName == attachInfoState.InterfaceName && a.Direction == attachInfoState.Direction && a.Priority == attachInfoState.Priority && - reflect.DeepEqual(r.getNetnsId(a.NetnsPath), newNetnsId) { + reflect.DeepEqual(r.NetNsCache.GetNetNsId(a.NetnsPath), newNetnsId) { return &i, nil } } diff --git a/controllers/bpfman-agent/ns_xdp_program.go b/controllers/bpfman-agent/ns_xdp_program.go index 0cee4707f..69b5c43da 100644 --- a/controllers/bpfman-agent/ns_xdp_program.go +++ b/controllers/bpfman-agent/ns_xdp_program.go @@ -175,7 +175,7 @@ func (r *NsXdpProgramReconciler) printAttachInfo(attachInfoState bpfmaniov1alpha } func (r *NsXdpProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.XdpAttachInfoState) (*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) } @@ -186,7 +186,7 @@ func (r *NsXdpProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.XdpAt if a.InterfaceName == attachInfoState.InterfaceName && a.Priority == 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 } } diff --git a/controllers/bpfman-agent/test_common.go b/controllers/bpfman-agent/test_common.go index 50b3bdfc8..fc968897a 100644 --- a/controllers/bpfman-agent/test_common.go +++ b/controllers/bpfman-agent/test_common.go @@ -200,3 +200,11 @@ func registerBpfApplicationScheme(s *runtime.Scheme, isClusterScoped bool, bpfAp } s.AddKnownTypes(bpfmaniov1alpha1.SchemeGroupVersion, bpfApp) } + +type MockNetNsCache map[string]*uint64 + +func (mnnc MockNetNsCache) GetNetNsId(path string) *uint64 { + return mnnc[path] +} + +func (mnnc MockNetNsCache) Reset() {} From ffe7fee0e7c457d5d44a1b41df181df70a61cea0 Mon Sep 17 00:00:00 2001 From: Andreas Karis Date: Thu, 23 Oct 2025 02:17:15 +0200 Subject: [PATCH 5/7] test: verify priority field handling in BPF program links Add unit tests for XDP, TC, and TCX programs to verify that: - nil priority defaults to 1000 - explicit priority values are correctly set - zero priority is allowed The tests cover both cluster-scoped and namespace-scoped BPF applications, ensuring the priority field is properly handled and reflected in the program status. Co-authored-by: Andrew McDermott Co-authored-by: Andreas Karis Signed-off-by: Andreas Karis --- .../cl_application_program_test.go | 170 ++++++++++++++++- .../ns_application_program_test.go | 171 +++++++++++++++++- 2 files changed, 339 insertions(+), 2 deletions(-) diff --git a/controllers/bpfman-agent/cl_application_program_test.go b/controllers/bpfman-agent/cl_application_program_test.go index 8269e0f00..112b01ebd 100644 --- a/controllers/bpfman-agent/cl_application_program_test.go +++ b/controllers/bpfman-agent/cl_application_program_test.go @@ -18,6 +18,7 @@ package bpfmanagent import ( "context" + "fmt" "reflect" "testing" @@ -25,6 +26,7 @@ import ( agenttestutils "github.com/bpfman/bpfman-operator/controllers/bpfman-agent/internal/test-utils" "github.com/bpfman/bpfman-operator/internal" testutils "github.com/bpfman/bpfman-operator/internal/test-utils" + "github.com/bpfman/bpfman-operator/pkg/helpers" "github.com/stretchr/testify/require" v1 "k8s.io/api/core/v1" @@ -82,6 +84,8 @@ func TestClBpfApplicationControllerCreate(t *testing.T) { bpfmaniov1alpha1.XdpProceedOnValue("pass"), bpfmaniov1alpha1.XdpProceedOnValue("dispatcher_return"), } + tcProceedOn = []bpfmaniov1alpha1.TcProceedOnValue{bpfmaniov1alpha1.TcProceedOnValue("ok"), + bpfmaniov1alpha1.TcProceedOnValue("shot")} ctx = context.TODO() ) @@ -233,6 +237,147 @@ func TestClBpfApplicationControllerCreate(t *testing.T) { }, }, }, + { + testName: "link priority test XDP", + programs: []bpfmaniov1alpha1.ClBpfApplicationProgram{ + // XDP Program + { + Name: fmt.Sprintf("%s-%d", testXdpBpfFunctionName, 0), + Type: bpfmaniov1alpha1.ProgTypeXDP, + XDP: &bpfmaniov1alpha1.ClXdpProgramInfo{ + Links: []bpfmaniov1alpha1.ClXdpAttachInfo{ + { + InterfaceSelector: interfaceSelector, + NetworkNamespaces: nil, + Priority: nil, // nil priority should yield 1000. + ProceedOn: proceedOn, + }, + }, + }, + }, + { + Name: fmt.Sprintf("%s-%d", testXdpBpfFunctionName, 1), + Type: bpfmaniov1alpha1.ProgTypeXDP, + XDP: &bpfmaniov1alpha1.ClXdpProgramInfo{ + Links: []bpfmaniov1alpha1.ClXdpAttachInfo{ + { + InterfaceSelector: interfaceSelector, + NetworkNamespaces: nil, + Priority: ptr.To(int32(999)), + ProceedOn: proceedOn, + }, + }, + }, + }, + { + Name: fmt.Sprintf("%s-%d", testXdpBpfFunctionName, 2), + Type: bpfmaniov1alpha1.ProgTypeXDP, + XDP: &bpfmaniov1alpha1.ClXdpProgramInfo{ + Links: []bpfmaniov1alpha1.ClXdpAttachInfo{ + { + InterfaceSelector: interfaceSelector, + NetworkNamespaces: nil, + Priority: ptr.To(int32(0)), // 0 priority should be allowed. + ProceedOn: proceedOn, + }, + }, + }, + }, + }, + }, + { + testName: "link priority test TC", + programs: []bpfmaniov1alpha1.ClBpfApplicationProgram{ + // TC Program + { + Name: fmt.Sprintf("%s-%d", testTcBpfFunctionName, 0), + Type: bpfmaniov1alpha1.ProgTypeTC, + TC: &bpfmaniov1alpha1.ClTcProgramInfo{ + Links: []bpfmaniov1alpha1.ClTcAttachInfo{ + { + InterfaceSelector: interfaceSelector, + NetworkNamespaces: nil, + Priority: nil, // nil priority should yield 1000. + ProceedOn: tcProceedOn, + }, + }, + }, + }, + { + Name: fmt.Sprintf("%s-%d", testTcBpfFunctionName, 1), + Type: bpfmaniov1alpha1.ProgTypeTC, + TC: &bpfmaniov1alpha1.ClTcProgramInfo{ + Links: []bpfmaniov1alpha1.ClTcAttachInfo{ + { + InterfaceSelector: interfaceSelector, + NetworkNamespaces: nil, + Priority: ptr.To(int32(999)), + ProceedOn: tcProceedOn, + }, + }, + }, + }, + { + Name: fmt.Sprintf("%s-%d", testTcBpfFunctionName, 2), + Type: bpfmaniov1alpha1.ProgTypeTC, + TC: &bpfmaniov1alpha1.ClTcProgramInfo{ + Links: []bpfmaniov1alpha1.ClTcAttachInfo{ + { + InterfaceSelector: interfaceSelector, + NetworkNamespaces: nil, + Priority: ptr.To(int32(0)), // 0 priority should be allowed. + ProceedOn: tcProceedOn, + }, + }, + }, + }, + }, + }, + { + testName: "link priority test TCX", + programs: []bpfmaniov1alpha1.ClBpfApplicationProgram{ + // TCX Program + { + Name: fmt.Sprintf("%s-%d", testTcxBpfFunctionName, 0), + Type: bpfmaniov1alpha1.ProgTypeTCX, + TCX: &bpfmaniov1alpha1.ClTcxProgramInfo{ + Links: []bpfmaniov1alpha1.ClTcxAttachInfo{ + { + InterfaceSelector: interfaceSelector, + NetworkNamespaces: nil, + Priority: nil, // nil priority should yield 1000. + }, + }, + }, + }, + { + Name: fmt.Sprintf("%s-%d", testTcxBpfFunctionName, 1), + Type: bpfmaniov1alpha1.ProgTypeTCX, + TCX: &bpfmaniov1alpha1.ClTcxProgramInfo{ + Links: []bpfmaniov1alpha1.ClTcxAttachInfo{ + { + InterfaceSelector: interfaceSelector, + NetworkNamespaces: nil, + Priority: ptr.To(int32(999)), + }, + }, + }, + }, + { + Name: fmt.Sprintf("%s-%d", testTcxBpfFunctionName, 2), + Type: bpfmaniov1alpha1.ProgTypeTCX, + TCX: &bpfmaniov1alpha1.ClTcxProgramInfo{ + Links: []bpfmaniov1alpha1.ClTcxAttachInfo{ + { + InterfaceSelector: interfaceSelector, + NetworkNamespaces: nil, + Priority: ptr.To(int32(0)), // 0 priority should be allowed. + }, + }, + }, + }, + }, + }, } for _, tc := range tcs { @@ -325,8 +470,31 @@ func createFakeClusterReconciler(objs []runtime.Object, bpfApp *bpfmaniov1alpha1 // successfully attached and that the number of programs matches the expected count. func verifyClusterBpfProgramState(t *testing.T, bpfAppState *bpfmaniov1alpha1.ClusterBpfApplicationState, programs []bpfmaniov1alpha1.ClBpfApplicationProgram) { + require.Equal(t, len(programs), len(bpfAppState.Status.Programs)) + + numMatches := 0 for _, program := range bpfAppState.Status.Programs { require.Equal(t, bpfmaniov1alpha1.ProgAttachSuccess, program.ProgramLinkStatus) + for _, expectedProgram := range programs { + if program.Name != expectedProgram.Name { + continue + } + switch { + case program.XDP != nil: + if program.XDP.Links[0].Priority != helpers.GetPriority(expectedProgram.XDP.Links[0].Priority) { + continue + } + case program.TC != nil: + if program.TC.Links[0].Priority != helpers.GetPriority(expectedProgram.TC.Links[0].Priority) { + continue + } + case program.TCX != nil: + if program.TCX.Links[0].Priority != helpers.GetPriority(expectedProgram.TCX.Links[0].Priority) { + continue + } + } + numMatches++ + } } - require.Equal(t, len(programs), len(bpfAppState.Status.Programs)) + require.Equal(t, len(programs), numMatches) } diff --git a/controllers/bpfman-agent/ns_application_program_test.go b/controllers/bpfman-agent/ns_application_program_test.go index 1bbf2a64a..8db81f50a 100644 --- a/controllers/bpfman-agent/ns_application_program_test.go +++ b/controllers/bpfman-agent/ns_application_program_test.go @@ -18,6 +18,7 @@ package bpfmanagent import ( "context" + "fmt" "reflect" "testing" @@ -25,6 +26,7 @@ import ( agenttestutils "github.com/bpfman/bpfman-operator/controllers/bpfman-agent/internal/test-utils" "github.com/bpfman/bpfman-operator/internal" testutils "github.com/bpfman/bpfman-operator/internal/test-utils" + "github.com/bpfman/bpfman-operator/pkg/helpers" "github.com/stretchr/testify/require" "sigs.k8s.io/controller-runtime/pkg/client/fake" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -192,6 +194,150 @@ func TestNsBpfApplicationControllerCreate(t *testing.T) { }, }, }, + { + testName: "link priority test XDP", + programs: []bpfmaniov1alpha1.BpfApplicationProgram{ + // XDP Program + { + Name: fmt.Sprintf("%s-%d", testXdpBpfFunctionName, 0), + Type: bpfmaniov1alpha1.ProgTypeXDP, + XDP: &bpfmaniov1alpha1.XdpProgramInfo{ + Links: []bpfmaniov1alpha1.XdpAttachInfo{ + { + InterfaceSelector: interfaceSelector, + NetworkNamespaces: fakeNetNamespaces, + Priority: nil, // nil priority should yield 1000. + ProceedOn: proceedOn, + }, + }, + }, + }, + { + Name: fmt.Sprintf("%s-%d", testXdpBpfFunctionName, 1), + Type: bpfmaniov1alpha1.ProgTypeXDP, + XDP: &bpfmaniov1alpha1.XdpProgramInfo{ + Links: []bpfmaniov1alpha1.XdpAttachInfo{ + { + InterfaceSelector: interfaceSelector, + NetworkNamespaces: fakeNetNamespaces, + Priority: ptr.To(int32(999)), + ProceedOn: proceedOn, + }, + }, + }, + }, + { + Name: fmt.Sprintf("%s-%d", testXdpBpfFunctionName, 2), + Type: bpfmaniov1alpha1.ProgTypeXDP, + XDP: &bpfmaniov1alpha1.XdpProgramInfo{ + Links: []bpfmaniov1alpha1.XdpAttachInfo{ + { + InterfaceSelector: interfaceSelector, + NetworkNamespaces: fakeNetNamespaces, + Priority: ptr.To(int32(0)), // 0 priority should be allowed. + ProceedOn: proceedOn, + }, + }, + }, + }, + }, + }, + { + testName: "link priority test TC", + programs: []bpfmaniov1alpha1.BpfApplicationProgram{ + // TC Program + { + Name: fmt.Sprintf("%s-%d", testTcBpfFunctionName, 0), + Type: bpfmaniov1alpha1.ProgTypeTC, + TC: &bpfmaniov1alpha1.TcProgramInfo{ + Links: []bpfmaniov1alpha1.TcAttachInfo{ + { + InterfaceSelector: interfaceSelector, + NetworkNamespaces: fakeNetNamespaces, + Priority: nil, // nil priority should yield 1000. + ProceedOn: tcProceedOn, + }, + }, + }, + }, + { + Name: fmt.Sprintf("%s-%d", testTcBpfFunctionName, 1), + Type: bpfmaniov1alpha1.ProgTypeTC, + TC: &bpfmaniov1alpha1.TcProgramInfo{ + Links: []bpfmaniov1alpha1.TcAttachInfo{ + { + InterfaceSelector: interfaceSelector, + NetworkNamespaces: fakeNetNamespaces, + Priority: ptr.To(int32(999)), + ProceedOn: tcProceedOn, + }, + }, + }, + }, + { + Name: fmt.Sprintf("%s-%d", testTcBpfFunctionName, 2), + Type: bpfmaniov1alpha1.ProgTypeTC, + TC: &bpfmaniov1alpha1.TcProgramInfo{ + Links: []bpfmaniov1alpha1.TcAttachInfo{ + { + InterfaceSelector: interfaceSelector, + NetworkNamespaces: fakeNetNamespaces, + Priority: ptr.To(int32(0)), // 0 priority should be allowed. + ProceedOn: tcProceedOn, + }, + }, + }, + }, + }, + }, + { + testName: "link priority test TCX", + programs: []bpfmaniov1alpha1.BpfApplicationProgram{ + // TCX Program + { + Name: fmt.Sprintf("%s-%d", testTcxBpfFunctionName, 0), + Type: bpfmaniov1alpha1.ProgTypeTCX, + TCX: &bpfmaniov1alpha1.TcxProgramInfo{ + Links: []bpfmaniov1alpha1.TcxAttachInfo{ + { + InterfaceSelector: interfaceSelector, + NetworkNamespaces: fakeNetNamespaces, + Direction: testDirectionIngress, + Priority: nil, // nil priority should yield 1000. + }, + }, + }, + }, + { + Name: fmt.Sprintf("%s-%d", testTcxBpfFunctionName, 1), + Type: bpfmaniov1alpha1.ProgTypeTCX, + TCX: &bpfmaniov1alpha1.TcxProgramInfo{ + Links: []bpfmaniov1alpha1.TcxAttachInfo{ + { + InterfaceSelector: interfaceSelector, + NetworkNamespaces: fakeNetNamespaces, + Direction: testDirectionIngress, + Priority: ptr.To(int32(999)), + }, + }, + }, + }, + { + Name: fmt.Sprintf("%s-%d", testTcxBpfFunctionName, 2), + Type: bpfmaniov1alpha1.ProgTypeTCX, + TCX: &bpfmaniov1alpha1.TcxProgramInfo{ + Links: []bpfmaniov1alpha1.TcxAttachInfo{ + { + InterfaceSelector: interfaceSelector, + NetworkNamespaces: fakeNetNamespaces, + Direction: testDirectionIngress, + Priority: ptr.To(int32(0)), // 0 priority should be allowed. + }, + }, + }, + }, + }, + }, } for _, tc := range tcs { @@ -296,8 +442,31 @@ func createFakeNamespaceReconciler(objs []runtime.Object, bpfApp *bpfmaniov1alph // successfully attached and that the number of programs matches the expected count. func verifyNamespaceBpfProgramState(t *testing.T, bpfAppState *bpfmaniov1alpha1.BpfApplicationState, programs []bpfmaniov1alpha1.BpfApplicationProgram) { + require.Equal(t, len(programs), len(bpfAppState.Status.Programs)) + + numMatches := 0 for _, program := range bpfAppState.Status.Programs { require.Equal(t, bpfmaniov1alpha1.ProgAttachSuccess, program.ProgramLinkStatus) + for _, expectedProgram := range programs { + if program.Name != expectedProgram.Name { + continue + } + switch { + case program.XDP != nil: + if program.XDP.Links[0].Priority != helpers.GetPriority(expectedProgram.XDP.Links[0].Priority) { + continue + } + case program.TC != nil: + if program.TC.Links[0].Priority != helpers.GetPriority(expectedProgram.TC.Links[0].Priority) { + continue + } + case program.TCX != nil: + if program.TCX.Links[0].Priority != helpers.GetPriority(expectedProgram.TCX.Links[0].Priority) { + continue + } + } + numMatches++ + } } - require.Equal(t, len(programs), len(bpfAppState.Status.Programs)) + require.Equal(t, len(programs), numMatches) } From eda234b156a30af3d57096fa7e5916497a4ae3ad Mon Sep 17 00:00:00 2001 From: Andreas Karis Date: Thu, 23 Oct 2025 14:43:09 +0200 Subject: [PATCH 6/7] test-integration: Add priority ordering verification for BPF links Add integration tests to verify BPF program links are correctly ordered by priority across XDP, TC, and TCX program types. The new verification framework validates link ordering on each cluster node by comparing ClusterBpfApplicationState data against actual bpfman daemon state. Co-authored-by: amcdermo@redhat.com Co-authored-by: Andreas Karis Signed-off-by: Andreas Karis --- test/integration/common.go | 223 +++++++++++++++++++++++++++++++ test/integration/metrics_test.go | 8 +- test/integration/tc_test.go | 80 ++++++++++- test/integration/tcx_test.go | 80 ++++++++++- test/integration/xdp_test.go | 80 ++++++++++- 5 files changed, 460 insertions(+), 11 deletions(-) diff --git a/test/integration/common.go b/test/integration/common.go index 64d56a6b6..7f1db93e9 100644 --- a/test/integration/common.go +++ b/test/integration/common.go @@ -5,12 +5,23 @@ package integration import ( "bytes" + "fmt" "regexp" + "slices" "strconv" "strings" "testing" + "github.com/bpfman/bpfman-operator/apis/v1alpha1" "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +const ( + bpfmanNamespace = "bpfman" + bpfmanContainer = "bpfman" + bpfmanDaemonSelector = "name=bpfman-daemon" ) func doKprobeCheck(t *testing.T, output *bytes.Buffer) bool { @@ -131,3 +142,215 @@ func doProbeCommonCheck(t *testing.T, output *bytes.Buffer, str string) (bool, i } return false, 0 } + +// clusterBpfApplicationStateSuccess returns a function that checks if the expected number of +// ClusterBpfApplications matching the label selector have reached a successful state. +func clusterBpfApplicationStateSuccess(t *testing.T, labelSelector string, numExpected int) func() bool { + return func() bool { + // Fetch all ClusterBpfApplications matching the label selector. + apps, err := bpfmanClient.BpfmanV1alpha1().ClusterBpfApplications().List(ctx, metav1.ListOptions{ + LabelSelector: labelSelector, + }) + require.NoError(t, err) + + // Count how many applications have reached success state. + numMatches := 0 + for _, app := range apps.Items { + c := meta.FindStatusCondition(app.Status.Conditions, string(v1alpha1.BpfAppStateCondSuccess)) + if c != nil && c.Status == metav1.ConditionTrue { + numMatches++ + } + } + // Return true if the number of successful applications matches expected count. + return numMatches == numExpected + } +} + +// verifyClusterBpfApplicationPriority returns a function that verifies BPF program links are ordered +// correctly according to their priority values on each node. +func verifyClusterBpfApplicationPriority(t *testing.T, labelSelector string) func() bool { + return func() bool { + // Fetch all ClusterBpfApplications matching the label selector. + apps, err := bpfmanClient.BpfmanV1alpha1().ClusterBpfApplications().List(ctx, metav1.ListOptions{ + LabelSelector: labelSelector, + }) + require.NoError(t, err) + + // Fetch all ClusterBpfApplicationStates to get per-node link information. + appStates, err := bpfmanClient.BpfmanV1alpha1().ClusterBpfApplicationStates().List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + + // Build a map of node names to their associated links from ClusterBpfApplicationStates. + nodeLinks := map[string][]link{} + for _, app := range apps.Items { + for _, appState := range appStates.Items { + for _, ownerRef := range appState.OwnerReferences { + // Skip if this appState is not controlled by the current app. + if ownerRef.Controller == nil || !*ownerRef.Controller { + continue + } + if ownerRef.UID != app.UID { + continue + } + // Initialize the slice for this node if needed. + if nodeLinks[appState.Status.Node] == nil { + nodeLinks[appState.Status.Node] = []link{} + } + // Extract and append links from this appState. + nodeLinks[appState.Status.Node] = append( + nodeLinks[appState.Status.Node], + getClusterBpfApplicationStateLinks(t, appState)..., + ) + } + } + } + // Verify link ordering on each node by directly querying bpfman daemon inside the pod. + for node, appStateLinks := range nodeLinks { + bpfmanLinks := []link{} + // Find the bpfman daemon pod running on this node. + pods, err := env.Cluster().Client().CoreV1().Pods(bpfmanNamespace).List(ctx, metav1.ListOptions{ + LabelSelector: bpfmanDaemonSelector, + FieldSelector: fmt.Sprintf("spec.nodeName=%s", node), + }) + require.NoError(t, err) + require.Len(t, pods.Items, 1) + // Query each link from bpfman and verify that bpfman get link matches the output from + // ClusterBpfApplicationState. + for _, appStateLink := range appStateLinks { + cmd := []string{"./bpfman", "get", "link", fmt.Sprintf("%d", appStateLink.linkId)} + var bpfmanOut, bpfmanErr bytes.Buffer + err := podExec(ctx, t, pods.Items[0], bpfmanContainer, &bpfmanOut, &bpfmanErr, cmd) + require.NoError(t, err) + t.Logf("bpfman get link output:\n%s", bpfmanOut.String()) + // Parse the bpfman output and verify it matches. + bpfmanLink := parseLink(bpfmanOut.String()) + require.True(t, linkOutputMatchesLink(t, bpfmanLink, appStateLink)) + bpfmanLinks = append(bpfmanLinks, bpfmanLink) + } + // Verify that links are ordered correctly by priority (match priority to expected position). + require.True(t, verifyLinkOrder(bpfmanLinks), "position in slice should match priority", bpfmanLinks) + } + return true + } +} + +// link represents a BPF program link with its metadata including link ID, network interface, +// namespace path, priority, and position in the link chain. +type link struct { + linkId uint32 + interfaceName string + netnsPath string + priority int32 + position int32 +} + +// parseLink parses the output from "bpfman get link" command and converts it to a link struct. +func parseLink(out string) link { + l := link{} + lines := bytes.Split([]byte(out), []byte("\n")) + + for _, line := range lines { + parts := bytes.SplitN(line, []byte(":"), 2) + if len(parts) != 2 { + continue + } + key := bytes.TrimSpace(parts[0]) + value := bytes.TrimSpace(parts[1]) + + switch string(key) { + case "Link ID": + fmt.Sscanf(string(value), "%d", &l.linkId) + case "Interface": + l.interfaceName = string(value) + case "Network Namespace": + if string(value) != "None" { + l.netnsPath = string(value) + } + case "Priority": + fmt.Sscanf(string(value), "%d", &l.priority) + case "Position": + fmt.Sscanf(string(value), "%d", &l.position) + } + } + + return l +} + +// getClusterBpfApplicationStateLinks extracts link information from a ClusterBpfApplicationState +// for XDP, TC, and TCX program types. +func getClusterBpfApplicationStateLinks(t *testing.T, appState v1alpha1.ClusterBpfApplicationState) []link { + links := []link{} + // Iterate through all programs in the application state. + for _, program := range appState.Status.Programs { + switch program.Type { + case v1alpha1.ProgTypeXDP: + // Extract XDP program links. + for _, l := range program.XDP.Links { + require.NotNil(t, l.LinkId) + links = append(links, link{ + linkId: *l.LinkId, + interfaceName: l.InterfaceName, + netnsPath: l.NetnsPath, + priority: l.Priority, + }) + } + case v1alpha1.ProgTypeTC: + // Extract TC program links. + for _, l := range program.TC.Links { + require.NotNil(t, l.LinkId) + links = append(links, link{ + linkId: *l.LinkId, + interfaceName: l.InterfaceName, + netnsPath: l.NetnsPath, + priority: l.Priority, + }) + } + case v1alpha1.ProgTypeTCX: + // Extract TCX program links. + for _, l := range program.TCX.Links { + require.NotNil(t, l.LinkId) + links = append(links, link{ + linkId: *l.LinkId, + interfaceName: l.InterfaceName, + netnsPath: l.NetnsPath, + priority: l.Priority, + }) + } + } + } + return links +} + +// linkOutputMatchesLink compares a link parsed from bpfman output with an expected link state. +func linkOutputMatchesLink(t *testing.T, linkFromOutput, l link) bool { + t.Logf("Comparing output and desired link state; got:\n%+v\nwanted:\n%+v", linkFromOutput, l) + return l.linkId == linkFromOutput.linkId && + l.interfaceName == linkFromOutput.interfaceName && + l.netnsPath == linkFromOutput.netnsPath && + l.priority == linkFromOutput.priority +} + +// verifyLinkOrder verifies that links' positions match their priorities. +// Side-effect: this orders `links` in place by position. +func verifyLinkOrder(links []link) bool { + // Order elements by position. + slices.SortFunc(links, func(a, b link) int { + if a.position < b.position { + return -1 + } + if a.position > b.position { + return 1 + } + return 0 + }) + + // Now, make sure that the priority of each element is >= the preceding element. + oldI := 0 + for i := 1; i < len(links); i++ { + if links[i].priority < links[oldI].priority { + return false + } + oldI = i + } + return true +} diff --git a/test/integration/metrics_test.go b/test/integration/metrics_test.go index c3583bff0..0f1814ec1 100644 --- a/test/integration/metrics_test.go +++ b/test/integration/metrics_test.go @@ -179,7 +179,7 @@ func testMetricsProxySelfTest(ctx context.Context, t *testing.T, pod corev1.Pod, cmd := []string{"env", "TOKEN=" + token, "/metrics-proxy", "test"} var stdout, stderr bytes.Buffer - err := podExec(ctx, t, pod, &stdout, &stderr, cmd) + err := podExec(ctx, t, pod, "", &stdout, &stderr, cmd) // For self-test, we expect exit code 0 for success, non-zero // for failure. Parse the JSON regardless of exit code to get @@ -244,7 +244,8 @@ func testMetricsProxySelfTest(ctx context.Context, t *testing.T, pod corev1.Pod, t.Logf("All self-tests passed successfully on pod %s", pod.Name) } -func podExec(ctx context.Context, t *testing.T, pod corev1.Pod, stdout, stderr *bytes.Buffer, cmd []string) error { +// podExec executes a command in a pod's container and captures stdout/stderr output. +func podExec(ctx context.Context, t *testing.T, pod corev1.Pod, container string, stdout, stderr *bytes.Buffer, cmd []string) error { t.Helper() kubeConfig, err := config.GetConfig() if err != nil { @@ -269,6 +270,9 @@ func podExec(ctx context.Context, t *testing.T, pod corev1.Pod, stdout, stderr * Stderr: true, TTY: false, } + if container != "" { + execOptions.Container = container + } req.VersionedParams(execOptions, scheme.ParameterCodec) diff --git a/test/integration/tc_test.go b/test/integration/tc_test.go index f3053f25d..fd3c92b3a 100644 --- a/test/integration/tc_test.go +++ b/test/integration/tc_test.go @@ -5,7 +5,7 @@ package integration import ( "bytes" - "context" + "fmt" "io" "testing" "time" @@ -14,20 +14,23 @@ import ( "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" ) const ( tcGoCounterKustomize = "https://github.com/bpfman/bpfman/examples/config/default/go-tc-counter/?timeout=120&ref=main" tcGoCounterUserspaceNs = "go-tc-counter" tcGoCounterUserspaceDsName = "go-tc-counter-ds" + tcGoCounterBytecodeName = "go-tc-counter-example" + tcByteCodeLabelSelector = "app.kubernetes.io/name=tcprogram" ) func TestTcGoCounter(t *testing.T) { t.Log("deploying tc counter program") require.NoError(t, clusters.KustomizeDeployForCluster(ctx, env.Cluster(), tcGoCounterKustomize)) - addCleanup(func(context.Context) error { + t.Cleanup(func() { cleanupLog("cleaning up tc counter program") - return clusters.KustomizeDeleteForCluster(ctx, env.Cluster(), tcGoCounterKustomize) + clusters.KustomizeDeleteForCluster(ctx, env.Cluster(), tcGoCounterKustomize) }) t.Log("waiting for go tc counter userspace daemon to be available") @@ -41,6 +44,7 @@ func TestTcGoCounter(t *testing.T) { pods, err := env.Cluster().Client().CoreV1().Pods(tcGoCounterUserspaceNs).List(ctx, metav1.ListOptions{LabelSelector: "name=go-tc-counter"}) require.NoError(t, err) + require.Len(t, pods.Items, 1) gotcCounterPod := pods.Items[0] req := env.Cluster().Client().CoreV1().Pods(tcGoCounterUserspaceNs).GetLogs(gotcCounterPod.Name, &corev1.PodLogOptions{}) @@ -57,3 +61,73 @@ func TestTcGoCounter(t *testing.T) { return doTcCheck(t, output) }, 30*time.Second, time.Second) } + +func TestTcGoCounterLinkPriority(t *testing.T) { + priorities := []*int32{ + nil, + ptr.To(int32(0)), + ptr.To(int32(500)), + ptr.To(int32(1000)), + } + + t.Log("deploying tc counter program") + require.NoError(t, clusters.KustomizeDeployForCluster(ctx, env.Cluster(), tcGoCounterKustomize)) + t.Cleanup(func() { + cleanupLog("cleaning up tc counter program") + clusters.KustomizeDeleteForCluster(ctx, env.Cluster(), tcGoCounterKustomize) + + cleanupLog("cleaning up tc counter bytecode") + bpfmanClient.BpfmanV1alpha1().ClusterBpfApplications().DeleteCollection(ctx, metav1.DeleteOptions{}, + metav1.ListOptions{ + LabelSelector: tcByteCodeLabelSelector, + }) + }) + + t.Log("creating copies of bytecode using the same link") + cba, err := bpfmanClient.BpfmanV1alpha1().ClusterBpfApplications().Get(ctx, tcGoCounterBytecodeName, metav1.GetOptions{}) + require.NoError(t, err) + name := cba.Name + cba.ObjectMeta = metav1.ObjectMeta{ + Labels: cba.Labels, + } + for i, priority := range priorities { + cba.Name = fmt.Sprintf("%s-%d", name, i) + cba.Spec.Programs[0].TC.Links[0].Priority = priority + _, err := bpfmanClient.BpfmanV1alpha1().ClusterBpfApplications().Create(ctx, cba, metav1.CreateOptions{}) + require.NoError(t, err) + } + // Add priority 55 from the kustomize deployment as well. + priorities = append(priorities, ptr.To(int32(55))) + + t.Log("waiting for bytecode to be attached successfully") + require.Eventually(t, clusterBpfApplicationStateSuccess(t, tcByteCodeLabelSelector, len(priorities)), 2*time.Minute, 10*time.Second) + require.Eventually(t, verifyClusterBpfApplicationPriority(t, tcByteCodeLabelSelector), 1*time.Minute, 10*time.Second) + + t.Log("waiting for go tc counter userspace daemon to be available") + require.Eventually(t, func() bool { + daemon, err := env.Cluster().Client().AppsV1().DaemonSets(tcGoCounterUserspaceNs).Get(ctx, tcGoCounterUserspaceDsName, metav1.GetOptions{}) + require.NoError(t, err) + return daemon.Status.DesiredNumberScheduled == daemon.Status.NumberAvailable + }, + // Wait 5 minutes since cosign is slow, https://github.com/bpfman/bpfman/issues/1043 + 5*time.Minute, 10*time.Second) + + pods, err := env.Cluster().Client().CoreV1().Pods(tcGoCounterUserspaceNs).List(ctx, metav1.ListOptions{LabelSelector: "name=go-tc-counter"}) + require.NoError(t, err) + require.Len(t, pods.Items, 1) + goTcCounterPod := pods.Items[0] + + req := env.Cluster().Client().CoreV1().Pods(tcGoCounterUserspaceNs).GetLogs(goTcCounterPod.Name, &corev1.PodLogOptions{}) + + require.Eventually(t, func() bool { + logs, err := req.Stream(ctx) + require.NoError(t, err) + defer logs.Close() + output := new(bytes.Buffer) + _, err = io.Copy(output, logs) + require.NoError(t, err) + t.Logf("counter pod log %s", output.String()) + + return doTcCheck(t, output) + }, 30*time.Second, time.Second) +} diff --git a/test/integration/tcx_test.go b/test/integration/tcx_test.go index e6648f5dd..54fe66353 100644 --- a/test/integration/tcx_test.go +++ b/test/integration/tcx_test.go @@ -5,7 +5,7 @@ package integration import ( "bytes" - "context" + "fmt" "io" "testing" "time" @@ -14,20 +14,23 @@ import ( "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" ) const ( tcxGoCounterKustomize = "https://github.com/bpfman/bpfman/examples/config/default/go-tcx-counter/?timeout=120&ref=main" tcxGoCounterUserspaceNs = "go-tcx-counter" tcxGoCounterUserspaceDsName = "go-tcx-counter-ds" + tcxGoCounterBytecodeName = "go-tcx-counter-example" + tcxByteCodeLabelSelector = "app.kubernetes.io/name=tcxprogram" ) func TestTcxGoCounter(t *testing.T) { t.Log("deploying tcx counter program") require.NoError(t, clusters.KustomizeDeployForCluster(ctx, env.Cluster(), tcxGoCounterKustomize)) - addCleanup(func(context.Context) error { + t.Cleanup(func() { cleanupLog("cleaning up tcx counter program") - return clusters.KustomizeDeleteForCluster(ctx, env.Cluster(), tcxGoCounterKustomize) + clusters.KustomizeDeleteForCluster(ctx, env.Cluster(), tcxGoCounterKustomize) }) t.Log("waiting for go tcx counter userspace daemon to be available") @@ -41,6 +44,7 @@ func TestTcxGoCounter(t *testing.T) { pods, err := env.Cluster().Client().CoreV1().Pods(tcxGoCounterUserspaceNs).List(ctx, metav1.ListOptions{LabelSelector: "name=go-tcx-counter"}) require.NoError(t, err) + require.Len(t, pods.Items, 1) gotcxCounterPod := pods.Items[0] req := env.Cluster().Client().CoreV1().Pods(tcxGoCounterUserspaceNs).GetLogs(gotcxCounterPod.Name, &corev1.PodLogOptions{}) @@ -57,3 +61,73 @@ func TestTcxGoCounter(t *testing.T) { return doTcxCheck(t, output) }, 30*time.Second, time.Second) } + +func TestTcxGoCounterLinkPriority(t *testing.T) { + priorities := []*int32{ + nil, + ptr.To(int32(0)), + ptr.To(int32(500)), + ptr.To(int32(1000)), + } + + t.Log("deploying tcx counter program") + require.NoError(t, clusters.KustomizeDeployForCluster(ctx, env.Cluster(), tcxGoCounterKustomize)) + t.Cleanup(func() { + cleanupLog("cleaning up tcx counter program") + clusters.KustomizeDeleteForCluster(ctx, env.Cluster(), tcxGoCounterKustomize) + + cleanupLog("cleaning up tcx counter bytecode") + bpfmanClient.BpfmanV1alpha1().ClusterBpfApplications().DeleteCollection(ctx, metav1.DeleteOptions{}, + metav1.ListOptions{ + LabelSelector: tcxByteCodeLabelSelector, + }) + }) + + t.Log("creating copies of bytecode using the same link") + cba, err := bpfmanClient.BpfmanV1alpha1().ClusterBpfApplications().Get(ctx, tcxGoCounterBytecodeName, metav1.GetOptions{}) + require.NoError(t, err) + name := cba.Name + cba.ObjectMeta = metav1.ObjectMeta{ + Labels: cba.Labels, + } + for i, priority := range priorities { + cba.Name = fmt.Sprintf("%s-%d", name, i) + cba.Spec.Programs[0].TCX.Links[0].Priority = priority + _, err := bpfmanClient.BpfmanV1alpha1().ClusterBpfApplications().Create(ctx, cba, metav1.CreateOptions{}) + require.NoError(t, err) + } + // Add priority 55 from the kustomize deployment as well. + priorities = append(priorities, ptr.To(int32(55))) + + t.Log("waiting for bytecode to be attached successfully") + require.Eventually(t, clusterBpfApplicationStateSuccess(t, tcxByteCodeLabelSelector, len(priorities)), 2*time.Minute, 10*time.Second) + require.Eventually(t, verifyClusterBpfApplicationPriority(t, tcxByteCodeLabelSelector), 1*time.Minute, 10*time.Second) + + t.Log("waiting for go tcx counter userspace daemon to be available") + require.Eventually(t, func() bool { + daemon, err := env.Cluster().Client().AppsV1().DaemonSets(tcxGoCounterUserspaceNs).Get(ctx, tcxGoCounterUserspaceDsName, metav1.GetOptions{}) + require.NoError(t, err) + return daemon.Status.DesiredNumberScheduled == daemon.Status.NumberAvailable + }, + // Wait 5 minutes since cosign is slow, https://github.com/bpfman/bpfman/issues/1043 + 5*time.Minute, 10*time.Second) + + pods, err := env.Cluster().Client().CoreV1().Pods(tcxGoCounterUserspaceNs).List(ctx, metav1.ListOptions{LabelSelector: "name=go-tcx-counter"}) + require.NoError(t, err) + require.Len(t, pods.Items, 1) + goTcxCounterPod := pods.Items[0] + + req := env.Cluster().Client().CoreV1().Pods(tcxGoCounterUserspaceNs).GetLogs(goTcxCounterPod.Name, &corev1.PodLogOptions{}) + + require.Eventually(t, func() bool { + logs, err := req.Stream(ctx) + require.NoError(t, err) + defer logs.Close() + output := new(bytes.Buffer) + _, err = io.Copy(output, logs) + require.NoError(t, err) + t.Logf("counter pod log %s", output.String()) + + return doTcxCheck(t, output) + }, 30*time.Second, time.Second) +} diff --git a/test/integration/xdp_test.go b/test/integration/xdp_test.go index 928e6feab..aaac9686b 100644 --- a/test/integration/xdp_test.go +++ b/test/integration/xdp_test.go @@ -5,7 +5,7 @@ package integration import ( "bytes" - "context" + "fmt" "io" "testing" "time" @@ -14,20 +14,23 @@ import ( "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" ) const ( xdpGoCounterKustomize = "https://github.com/bpfman/bpfman/examples/config/default/go-xdp-counter/?timeout=120&ref=main" xdpGoCounterUserspaceNs = "go-xdp-counter" xdpGoCounterUserspaceDsName = "go-xdp-counter-ds" + xdpGoCounterBytecodeName = "go-xdp-counter-example" + xdpByteCodeLabelSelector = "app.kubernetes.io/name=xdpprogram" ) func TestXdpGoCounter(t *testing.T) { t.Log("deploying xdp counter program") require.NoError(t, clusters.KustomizeDeployForCluster(ctx, env.Cluster(), xdpGoCounterKustomize)) - addCleanup(func(context.Context) error { + t.Cleanup(func() { cleanupLog("cleaning up xdp counter program") - return clusters.KustomizeDeleteForCluster(ctx, env.Cluster(), xdpGoCounterKustomize) + clusters.KustomizeDeleteForCluster(ctx, env.Cluster(), xdpGoCounterKustomize) }) t.Log("waiting for go xdp counter userspace daemon to be available") @@ -41,6 +44,77 @@ func TestXdpGoCounter(t *testing.T) { pods, err := env.Cluster().Client().CoreV1().Pods(xdpGoCounterUserspaceNs).List(ctx, metav1.ListOptions{LabelSelector: "name=go-xdp-counter"}) require.NoError(t, err) + require.Len(t, pods.Items, 1) + goXdpCounterPod := pods.Items[0] + + req := env.Cluster().Client().CoreV1().Pods(xdpGoCounterUserspaceNs).GetLogs(goXdpCounterPod.Name, &corev1.PodLogOptions{}) + + require.Eventually(t, func() bool { + logs, err := req.Stream(ctx) + require.NoError(t, err) + defer logs.Close() + output := new(bytes.Buffer) + _, err = io.Copy(output, logs) + require.NoError(t, err) + t.Logf("counter pod log %s", output.String()) + + return doXdpCheck(t, output) + }, 30*time.Second, time.Second) +} + +func TestXdpGoCounterLinkPriority(t *testing.T) { + priorities := []*int32{ + nil, + ptr.To(int32(0)), + ptr.To(int32(500)), + ptr.To(int32(1000)), + } + + t.Log("deploying xdp counter program") + require.NoError(t, clusters.KustomizeDeployForCluster(ctx, env.Cluster(), xdpGoCounterKustomize)) + t.Cleanup(func() { + cleanupLog("cleaning up xdp counter program") + clusters.KustomizeDeleteForCluster(ctx, env.Cluster(), xdpGoCounterKustomize) + + cleanupLog("cleaning up xdp counter bytecode") + bpfmanClient.BpfmanV1alpha1().ClusterBpfApplications().DeleteCollection(ctx, metav1.DeleteOptions{}, + metav1.ListOptions{ + LabelSelector: xdpByteCodeLabelSelector, + }) + }) + + t.Log("creating copies of bytecode using the same link") + cba, err := bpfmanClient.BpfmanV1alpha1().ClusterBpfApplications().Get(ctx, xdpGoCounterBytecodeName, metav1.GetOptions{}) + require.NoError(t, err) + name := cba.Name + cba.ObjectMeta = metav1.ObjectMeta{ + Labels: cba.Labels, + } + for i, priority := range priorities { + cba.Name = fmt.Sprintf("%s-%d", name, i) + cba.Spec.Programs[0].XDP.Links[0].Priority = priority + _, err := bpfmanClient.BpfmanV1alpha1().ClusterBpfApplications().Create(ctx, cba, metav1.CreateOptions{}) + require.NoError(t, err) + } + // Add priority 55 from the kustomize deployment as well. + priorities = append(priorities, ptr.To(int32(55))) + + t.Log("waiting for bytecode to be attached successfully") + require.Eventually(t, clusterBpfApplicationStateSuccess(t, xdpByteCodeLabelSelector, len(priorities)), 2*time.Minute, 10*time.Second) + require.Eventually(t, verifyClusterBpfApplicationPriority(t, xdpByteCodeLabelSelector), 1*time.Minute, 10*time.Second) + + t.Log("waiting for go xdp counter userspace daemon to be available") + require.Eventually(t, func() bool { + daemon, err := env.Cluster().Client().AppsV1().DaemonSets(xdpGoCounterUserspaceNs).Get(ctx, xdpGoCounterUserspaceDsName, metav1.GetOptions{}) + require.NoError(t, err) + return daemon.Status.DesiredNumberScheduled == daemon.Status.NumberAvailable + }, + // Wait 5 minutes since cosign is slow, https://github.com/bpfman/bpfman/issues/1043 + 5*time.Minute, 10*time.Second) + + pods, err := env.Cluster().Client().CoreV1().Pods(xdpGoCounterUserspaceNs).List(ctx, metav1.ListOptions{LabelSelector: "name=go-xdp-counter"}) + require.NoError(t, err) + require.Len(t, pods.Items, 1) goXdpCounterPod := pods.Items[0] req := env.Cluster().Client().CoreV1().Pods(xdpGoCounterUserspaceNs).GetLogs(goXdpCounterPod.Name, &corev1.PodLogOptions{}) From ae70f533ae76551f1a752b3704d59faf5240b1c9 Mon Sep 17 00:00:00 2001 From: Andrew McDermott Date: Mon, 12 Jan 2026 12:45:50 +0000 Subject: [PATCH 7/7] Define DefaultAttachPriority as public API constant Move the default attach priority value to the API package as a public constant, providing a single authoritative definition. The helper function now references this constant rather than a private value. This ensures the default is defined alongside the types that use it and can be referenced by documentation and external consumers. Signed-off-by: Andrew McDermott --- apis/v1alpha1/shared_types.go | 6 ++++++ pkg/helpers/helpers.go | 17 ++++++++--------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/apis/v1alpha1/shared_types.go b/apis/v1alpha1/shared_types.go index 3e20dc29d..c2e444c07 100644 --- a/apis/v1alpha1/shared_types.go +++ b/apis/v1alpha1/shared_types.go @@ -20,6 +20,12 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +// DefaultAttachPriority is the default priority used when attaching BPF +// programs (XDP, TC, TCX) to network interfaces. Priority determines execution +// order relative to other programs at the same attachment point, where lower +// values indicate higher precedence. Valid range is 0-1000. +const DefaultAttachPriority int32 = 1000 + type InterfaceDiscovery struct { // interfaceAutoDiscovery is an optional field. When enabled, the agent // monitors the creation and deletion of interfaces and automatically diff --git a/pkg/helpers/helpers.go b/pkg/helpers/helpers.go index 711141850..c374e5686 100644 --- a/pkg/helpers/helpers.go +++ b/pkg/helpers/helpers.go @@ -32,12 +32,11 @@ import ( type ProgramType int32 const ( - Kprobe ProgramType = 2 - Tc ProgramType = 3 - Tracepoint ProgramType = 5 - Xdp ProgramType = 6 - Tracing ProgramType = 26 - defaultPriority = 1000 + Kprobe ProgramType = 2 + Tc ProgramType = 3 + Tracepoint ProgramType = 5 + Xdp ProgramType = 6 + Tracing ProgramType = 26 ) func (p ProgramType) Uint32() *uint32 { @@ -188,11 +187,11 @@ func IsBpfAppStateConditionPending(conditions []metav1.Condition) bool { return conditions[0].Type == string(bpfmaniov1alpha1.BpfAppCondPending) } -// GetPriority reads a priority value. If priority is nil, return the default value (1000). Otherwise, return the value -// behind the pointer. +// GetPriority reads a priority value. If priority is nil, return +// DefaultAttachPriority. Otherwise, return the value behind the pointer. func GetPriority(priority *int32) int32 { if priority == nil { - return defaultPriority + return bpfmaniov1alpha1.DefaultAttachPriority } return *priority }