From 8cd3df73fbcddf307080aeeeb01bbdd5bf5a7c0d Mon Sep 17 00:00:00 2001 From: Andreas Karis Date: Tue, 21 Oct 2025 19:00:27 +0200 Subject: [PATCH 01/11] 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 - Regenerates CRDs and deepcopy code - 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. Signed-off-by: Andreas Karis --- apis/v1alpha1/cluster_tc_program_types.go | 5 +- apis/v1alpha1/cluster_tcx_program_types.go | 5 +- apis/v1alpha1/cluster_xdp_program_types.go | 5 +- apis/v1alpha1/tc_program_types.go | 5 +- apis/v1alpha1/tcx_program_types.go | 5 +- apis/v1alpha1/xdp_program_types.go | 5 +- apis/v1alpha1/zz_generated.deepcopy.go | 60 +++++++++++++++++++ .../manifests/bpfman.io_bpfapplications.yaml | 3 - .../bpfman.io_bpfapplicationstates.yaml | 3 - .../bpfman.io_clusterbpfapplications.yaml | 3 - ...bpfman.io_clusterbpfapplicationstates.yaml | 3 - .../crd/bases/bpfman.io_bpfapplications.yaml | 3 - .../bases/bpfman.io_bpfapplicationstates.yaml | 3 - .../bpfman.io_clusterbpfapplications.yaml | 3 - ...bpfman.io_clusterbpfapplicationstates.yaml | 3 - .../cl_application_program_test.go | 7 ++- controllers/bpfman-agent/cl_tc_program.go | 7 ++- controllers/bpfman-agent/cl_tcx_program.go | 7 ++- controllers/bpfman-agent/cl_xdp_program.go | 7 ++- .../ns_application_program_test.go | 7 ++- controllers/bpfman-agent/ns_tc_program.go | 7 ++- controllers/bpfman-agent/ns_tcx_program.go | 7 ++- controllers/bpfman-agent/ns_xdp_program.go | 7 ++- .../cl_application_program_test.go | 3 +- .../ns_application_program_test.go | 3 +- pkg/helpers/helpers.go | 31 ++++++++-- 26 files changed, 133 insertions(+), 74 deletions(-) diff --git a/apis/v1alpha1/cluster_tc_program_types.go b/apis/v1alpha1/cluster_tc_program_types.go index 73978fd51..93f3d6652 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 @@ -127,7 +126,7 @@ type ClTcAttachInfoState struct { // +required // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=1000 - Priority int32 `json:"priority"` + Priority *int32 `json:"priority,omitempty"` // proceedOn is the provisioned list of proceedOn values. proceedOn allows the // user to call other TC programs in a chain, or not call the next program in a diff --git a/apis/v1alpha1/cluster_tcx_program_types.go b/apis/v1alpha1/cluster_tcx_program_types.go index 1c629cda7..e163120bd 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 { @@ -110,5 +109,5 @@ type ClTcxAttachInfoState struct { // +required // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=1000 - Priority int32 `json:"priority"` + Priority *int32 `json:"priority,omitempty"` } diff --git a/apis/v1alpha1/cluster_xdp_program_types.go b/apis/v1alpha1/cluster_xdp_program_types.go index 3f25e801a..dede988c2 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 @@ -107,7 +106,7 @@ type ClXdpAttachInfoState struct { // +required // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=1000 - Priority int32 `json:"priority"` + Priority *int32 `json:"priority,omitempty"` // proceedOn is the provisioned list of proceedOn values. proceedOn allows the // user to call other TC programs in a chain, or not call the next program in a diff --git a/apis/v1alpha1/tc_program_types.go b/apis/v1alpha1/tc_program_types.go index efe56e3d4..fdcc6eebc 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 @@ -123,7 +122,7 @@ type TcAttachInfoState struct { // +required // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=1000 - Priority int32 `json:"priority"` + Priority *int32 `json:"priority,omitempty"` // proceedOn is the provisioned list of proceedOn values. proceedOn allows the // user to call other TC programs in a chain, or not call the next program in a diff --git a/apis/v1alpha1/tcx_program_types.go b/apis/v1alpha1/tcx_program_types.go index 9c0b54729..cf8c136a1 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 { @@ -109,5 +108,5 @@ type TcxAttachInfoState struct { // +required // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=1000 - Priority int32 `json:"priority"` + Priority *int32 `json:"priority,omitempty"` } diff --git a/apis/v1alpha1/xdp_program_types.go b/apis/v1alpha1/xdp_program_types.go index 872c053ff..c9cc5c8c8 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 @@ -102,7 +101,7 @@ type XdpAttachInfoState struct { // +required // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=1000 - Priority int32 `json:"priority"` + Priority *int32 `json:"priority,omitempty"` // proceedOn is the provisioned list of proceedOn values. proceedOn allows the // user to call other TC programs in a chain, or not call the next program in a diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index 89bc0dab2..6d2ea1ebc 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)) @@ -1011,6 +1016,11 @@ func (in *ClTcAttachInfo) DeepCopy() *ClTcAttachInfo { func (in *ClTcAttachInfoState) DeepCopyInto(out *ClTcAttachInfoState) { *out = *in in.AttachInfoStateCommon.DeepCopyInto(&out.AttachInfoStateCommon) + 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 +1091,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. @@ -1097,6 +1112,11 @@ func (in *ClTcxAttachInfo) DeepCopy() *ClTcxAttachInfo { func (in *ClTcxAttachInfoState) DeepCopyInto(out *ClTcxAttachInfoState) { *out = *in in.AttachInfoStateCommon.DeepCopyInto(&out.AttachInfoStateCommon) + 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 ClTcxAttachInfoState. @@ -1330,6 +1350,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)) @@ -1351,6 +1376,11 @@ func (in *ClXdpAttachInfo) DeepCopy() *ClXdpAttachInfo { func (in *ClXdpAttachInfoState) DeepCopyInto(out *ClXdpAttachInfoState) { *out = *in in.AttachInfoStateCommon.DeepCopyInto(&out.AttachInfoStateCommon) + 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)) @@ -1759,6 +1789,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)) @@ -1780,6 +1815,11 @@ func (in *TcAttachInfo) DeepCopy() *TcAttachInfo { func (in *TcAttachInfoState) DeepCopyInto(out *TcAttachInfoState) { *out = *in in.AttachInfoStateCommon.DeepCopyInto(&out.AttachInfoStateCommon) + 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)) @@ -1846,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. @@ -1862,6 +1907,11 @@ func (in *TcxAttachInfo) DeepCopy() *TcxAttachInfo { func (in *TcxAttachInfoState) DeepCopyInto(out *TcxAttachInfoState) { *out = *in in.AttachInfoStateCommon.DeepCopyInto(&out.AttachInfoStateCommon) + 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 TcxAttachInfoState. @@ -2009,6 +2059,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)) @@ -2030,6 +2085,11 @@ func (in *XdpAttachInfo) DeepCopy() *XdpAttachInfo { func (in *XdpAttachInfoState) DeepCopyInto(out *XdpAttachInfoState) { *out = *in in.AttachInfoStateCommon.DeepCopyInto(&out.AttachInfoStateCommon) + 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.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_bpfapplicationstates.yaml b/bundle/manifests/bpfman.io_bpfapplicationstates.yaml index 4423ddd49..20b87adaa 100644 --- a/bundle/manifests/bpfman.io_bpfapplicationstates.yaml +++ b/bundle/manifests/bpfman.io_bpfapplicationstates.yaml @@ -275,7 +275,6 @@ spec: - interfaceName - linkStatus - netnsPath - - priority - proceedOn - shouldAttach - uuid @@ -346,7 +345,6 @@ spec: - interfaceName - linkStatus - netnsPath - - priority - shouldAttach - uuid type: object @@ -598,7 +596,6 @@ spec: - interfaceName - linkStatus - netnsPath - - priority - proceedOn - shouldAttach - uuid 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/bundle/manifests/bpfman.io_clusterbpfapplicationstates.yaml b/bundle/manifests/bpfman.io_clusterbpfapplicationstates.yaml index 5ee01d9da..aebe008b7 100644 --- a/bundle/manifests/bpfman.io_clusterbpfapplicationstates.yaml +++ b/bundle/manifests/bpfman.io_clusterbpfapplicationstates.yaml @@ -478,7 +478,6 @@ spec: - direction - interfaceName - linkStatus - - priority - proceedOn - shouldAttach - uuid @@ -548,7 +547,6 @@ spec: - direction - interfaceName - linkStatus - - priority - shouldAttach - uuid type: object @@ -872,7 +870,6 @@ spec: required: - interfaceName - linkStatus - - priority - proceedOn - shouldAttach - uuid 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_bpfapplicationstates.yaml b/config/crd/bases/bpfman.io_bpfapplicationstates.yaml index 40ae123e9..49f03768a 100644 --- a/config/crd/bases/bpfman.io_bpfapplicationstates.yaml +++ b/config/crd/bases/bpfman.io_bpfapplicationstates.yaml @@ -275,7 +275,6 @@ spec: - interfaceName - linkStatus - netnsPath - - priority - proceedOn - shouldAttach - uuid @@ -346,7 +345,6 @@ spec: - interfaceName - linkStatus - netnsPath - - priority - shouldAttach - uuid type: object @@ -598,7 +596,6 @@ spec: - interfaceName - linkStatus - netnsPath - - priority - proceedOn - shouldAttach - uuid 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 diff --git a/config/crd/bases/bpfman.io_clusterbpfapplicationstates.yaml b/config/crd/bases/bpfman.io_clusterbpfapplicationstates.yaml index c097dbfbd..610f3b094 100644 --- a/config/crd/bases/bpfman.io_clusterbpfapplicationstates.yaml +++ b/config/crd/bases/bpfman.io_clusterbpfapplicationstates.yaml @@ -478,7 +478,6 @@ spec: - direction - interfaceName - linkStatus - - priority - proceedOn - shouldAttach - uuid @@ -548,7 +547,6 @@ spec: - direction - interfaceName - linkStatus - - priority - shouldAttach - uuid type: object @@ -872,7 +870,6 @@ spec: required: - interfaceName - linkStatus - - priority - proceedOn - shouldAttach - uuid 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..ee8ce76a7 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" ) @@ -131,7 +132,7 @@ func (r *ClTcProgramReconciler) getAttachRequest() *gobpfman.AttachRequest { } attachInfo := &gobpfman.TCAttachInfo{ - Priority: r.currentLink.Priority, + Priority: helpers.GetPriority(r.currentLink.Priority), Iface: r.currentLink.InterfaceName, Direction: directionToStr(r.currentLink.Direction), ProceedOn: tcProceedOnToInt(r.currentLink.ProceedOn), @@ -222,7 +223,7 @@ func (r *ClTcProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.ClTcAt // same: InterfaceName, Direction, Priority, ProceedOn, and network namespace. if a.InterfaceName == attachInfoState.InterfaceName && a.Direction == attachInfoState.Direction && - a.Priority == attachInfoState.Priority && + helpers.GetPriority(a.Priority) == helpers.GetPriority(attachInfoState.Priority) && reflect.DeepEqual(a.ProceedOn, attachInfoState.ProceedOn) && reflect.DeepEqual(r.getNetnsId(a.NetnsPath), newNetnsId) { return &i, nil @@ -306,7 +307,7 @@ func (r *ClTcProgramReconciler) getExpectedLinks(ctx context.Context, attachInfo }, InterfaceName: interfaceName, NetnsPath: netnsPath, - Priority: attachInfo.Priority, + Priority: helpers.GetPriorityPointer(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..8caca88d6 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" ) @@ -97,7 +98,7 @@ func (r *ClTcxProgramReconciler) getAttachRequest() *gobpfman.AttachRequest { } attachInfo := &gobpfman.TCXAttachInfo{ - Priority: r.currentLink.Priority, + Priority: helpers.GetPriority(r.currentLink.Priority), Iface: r.currentLink.InterfaceName, Direction: directionToStr(r.currentLink.Direction), Metadata: map[string]string{internal.UuidMetadataKey: string(r.currentLink.UUID)}, @@ -187,7 +188,7 @@ func (r *ClTcxProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.ClTcx // same: InterfaceName, Direction, Priority, and network namespace. if a.InterfaceName == attachInfoState.InterfaceName && a.Direction == attachInfoState.Direction && - a.Priority == attachInfoState.Priority && + helpers.GetPriority(a.Priority) == helpers.GetPriority(attachInfoState.Priority) && reflect.DeepEqual(r.getNetnsId(a.NetnsPath), newNetnsId) { return &i, nil } @@ -270,7 +271,7 @@ func (r *ClTcxProgramReconciler) getExpectedLinks(ctx context.Context, attachInf }, InterfaceName: interfaceName, NetnsPath: netnsPath, - Priority: attachInfo.Priority, + Priority: helpers.GetPriorityPointer(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..4dc3e5870 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" ) @@ -121,7 +122,7 @@ func (r *ClXdpProgramReconciler) getAttachRequest() *gobpfman.AttachRequest { } attachInfo := &gobpfman.XDPAttachInfo{ - Priority: r.currentLink.Priority, + Priority: helpers.GetPriority(r.currentLink.Priority), Iface: r.currentLink.InterfaceName, ProceedOn: xdpProceedOnToInt(r.currentLink.ProceedOn), Metadata: map[string]string{internal.UuidMetadataKey: string(r.currentLink.UUID)}, @@ -210,7 +211,7 @@ func (r *ClXdpProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.ClXdp // attachInfoState is the same as a if the the following fields are the // same: InterfaceName, Priority, ProceedOn, and network namespace. if a.InterfaceName == attachInfoState.InterfaceName && - a.Priority == attachInfoState.Priority && + helpers.GetPriority(a.Priority) == helpers.GetPriority(attachInfoState.Priority) && reflect.DeepEqual(a.ProceedOn, attachInfoState.ProceedOn) && reflect.DeepEqual(r.getNetnsId(a.NetnsPath), newNetnsId) { return &i, nil @@ -294,7 +295,7 @@ func (r *ClXdpProgramReconciler) getExpectedLinks(ctx context.Context, attachInf }, InterfaceName: interfaceName, NetnsPath: netnsPath, - Priority: attachInfo.Priority, + Priority: helpers.GetPriorityPointer(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..47cb36db0 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" ) @@ -96,7 +97,7 @@ func (r *NsTcProgramReconciler) getNamespace() string { func (r *NsTcProgramReconciler) getAttachRequest() *gobpfman.AttachRequest { attachInfo := &gobpfman.TCAttachInfo{ - Priority: r.currentLink.Priority, + Priority: helpers.GetPriority(r.currentLink.Priority), Iface: r.currentLink.InterfaceName, Direction: directionToStr(r.currentLink.Direction), ProceedOn: tcProceedOnToInt(r.currentLink.ProceedOn), @@ -186,7 +187,7 @@ func (r *NsTcProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.TcAtta // same: InterfaceName, Direction, Priority, ProceedOn, and network namespace. if a.InterfaceName == attachInfoState.InterfaceName && a.Direction == attachInfoState.Direction && - a.Priority == attachInfoState.Priority && + helpers.GetPriority(a.Priority) == helpers.GetPriority(attachInfoState.Priority) && reflect.DeepEqual(a.ProceedOn, attachInfoState.ProceedOn) && reflect.DeepEqual(r.getNetnsId(a.NetnsPath), newNetnsId) { return &i, nil @@ -294,7 +295,7 @@ func (r *NsTcProgramReconciler) getExpectedLinks(ctx context.Context, attachInfo }, InterfaceName: iface, NetnsPath: netnsPath, - Priority: attachInfo.Priority, + Priority: helpers.GetPriorityPointer(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..94816b15d 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" ) @@ -96,7 +97,7 @@ func (r *NsTcxProgramReconciler) getNamespace() string { func (r *NsTcxProgramReconciler) getAttachRequest() *gobpfman.AttachRequest { attachInfo := &gobpfman.TCXAttachInfo{ - Priority: r.currentLink.Priority, + Priority: helpers.GetPriority(r.currentLink.Priority), Iface: r.currentLink.InterfaceName, Direction: directionToStr(r.currentLink.Direction), Metadata: map[string]string{internal.UuidMetadataKey: string(r.currentLink.UUID)}, @@ -185,7 +186,7 @@ func (r *NsTcxProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.TcxAt // same: InterfaceName, Direction, Priority, and network namespace. if a.InterfaceName == attachInfoState.InterfaceName && a.Direction == attachInfoState.Direction && - a.Priority == attachInfoState.Priority && + helpers.GetPriority(a.Priority) == helpers.GetPriority(attachInfoState.Priority) && reflect.DeepEqual(r.getNetnsId(a.NetnsPath), newNetnsId) { return &i, nil } @@ -293,7 +294,7 @@ func (r *NsTcxProgramReconciler) getExpectedLinks(ctx context.Context, attachInf }, InterfaceName: iface, NetnsPath: netnsPath, - Priority: attachInfo.Priority, + Priority: helpers.GetPriorityPointer(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..6980758ad 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" ) @@ -96,7 +97,7 @@ func (r *NsXdpProgramReconciler) getNamespace() string { func (r *NsXdpProgramReconciler) getAttachRequest() *gobpfman.AttachRequest { attachInfo := &gobpfman.XDPAttachInfo{ - Priority: r.currentLink.Priority, + Priority: helpers.GetPriority(r.currentLink.Priority), Iface: r.currentLink.InterfaceName, ProceedOn: xdpProceedOnToInt(r.currentLink.ProceedOn), Metadata: map[string]string{internal.UuidMetadataKey: string(r.currentLink.UUID)}, @@ -183,7 +184,7 @@ func (r *NsXdpProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.XdpAt // attachInfoState is the same as a if the the following fields are the // same: InterfaceName, Priority, ProceedOn, and network namespace. if a.InterfaceName == attachInfoState.InterfaceName && - a.Priority == attachInfoState.Priority && + helpers.GetPriority(a.Priority) == helpers.GetPriority(attachInfoState.Priority) && reflect.DeepEqual(a.ProceedOn, attachInfoState.ProceedOn) && reflect.DeepEqual(r.getNetnsId(a.NetnsPath), newNetnsId) { return &i, nil @@ -292,7 +293,7 @@ func (r *NsXdpProgramReconciler) getExpectedLinks(ctx context.Context, attachInf }, InterfaceName: iface, NetnsPath: netnsPath, - Priority: attachInfo.Priority, + Priority: helpers.GetPriorityPointer(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..2e7e38e6d 100644 --- a/pkg/helpers/helpers.go +++ b/pkg/helpers/helpers.go @@ -21,11 +21,11 @@ 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" "k8s.io/client-go/tools/clientcmd" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" ) @@ -33,11 +33,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 +188,21 @@ 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 +} + +// GetPriorityPointer reads a priority value. If priority is nil, return a pointer to the default value (1000). +// Otherwise, return the pointer itself. +func GetPriorityPointer(priority *int32) *int32 { + if priority == nil { + return ptr.To(int32(defaultPriority)) + } + return priority +} From a6fe66a0970c62f4b6270959f60acb79092c809b Mon Sep 17 00:00:00 2001 From: Andreas Karis Date: Wed, 22 Oct 2025 13:03:26 +0200 Subject: [PATCH 02/11] 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 b3323d1f841106219048112c587009fbf5df35fc Mon Sep 17 00:00:00 2001 From: Andreas Karis Date: Thu, 23 Oct 2025 00:44:32 +0200 Subject: [PATCH 03/11] 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 ee8ce76a7..4b3d74abf 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 && helpers.GetPriority(a.Priority) == helpers.GetPriority(attachInfoState.Priority) && reflect.DeepEqual(a.ProceedOn, attachInfoState.ProceedOn) && - reflect.DeepEqual(r.getNetnsId(a.NetnsPath), newNetnsId) { + reflect.DeepEqual(r.NetNsCache.GetNetNsId(a.NetnsPath), newNetnsId) { return &i, nil } } diff --git a/controllers/bpfman-agent/cl_tcx_program.go b/controllers/bpfman-agent/cl_tcx_program.go index 8caca88d6..507d99115 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 && helpers.GetPriority(a.Priority) == helpers.GetPriority(attachInfoState.Priority) && - reflect.DeepEqual(r.getNetnsId(a.NetnsPath), newNetnsId) { + reflect.DeepEqual(r.NetNsCache.GetNetNsId(a.NetnsPath), newNetnsId) { return &i, nil } } diff --git a/controllers/bpfman-agent/cl_xdp_program.go b/controllers/bpfman-agent/cl_xdp_program.go index 4dc3e5870..4e11c6995 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 && helpers.GetPriority(a.Priority) == helpers.GetPriority(attachInfoState.Priority) && reflect.DeepEqual(a.ProceedOn, attachInfoState.ProceedOn) && - reflect.DeepEqual(r.getNetnsId(a.NetnsPath), newNetnsId) { + reflect.DeepEqual(r.NetNsCache.GetNetNsId(a.NetnsPath), newNetnsId) { return &i, nil } } diff --git a/controllers/bpfman-agent/common.go b/controllers/bpfman-agent/common.go index 601b58ceb..f510a075a 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 47cb36db0..4927c8d86 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 && helpers.GetPriority(a.Priority) == helpers.GetPriority(attachInfoState.Priority) && reflect.DeepEqual(a.ProceedOn, attachInfoState.ProceedOn) && - reflect.DeepEqual(r.getNetnsId(a.NetnsPath), newNetnsId) { + reflect.DeepEqual(r.NetNsCache.GetNetNsId(a.NetnsPath), newNetnsId) { return &i, nil } } diff --git a/controllers/bpfman-agent/ns_tcx_program.go b/controllers/bpfman-agent/ns_tcx_program.go index 94816b15d..4255087ea 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 && helpers.GetPriority(a.Priority) == helpers.GetPriority(attachInfoState.Priority) && - reflect.DeepEqual(r.getNetnsId(a.NetnsPath), newNetnsId) { + reflect.DeepEqual(r.NetNsCache.GetNetNsId(a.NetnsPath), newNetnsId) { return &i, nil } } diff --git a/controllers/bpfman-agent/ns_xdp_program.go b/controllers/bpfman-agent/ns_xdp_program.go index 6980758ad..97389303e 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 && helpers.GetPriority(a.Priority) == helpers.GetPriority(attachInfoState.Priority) && reflect.DeepEqual(a.ProceedOn, attachInfoState.ProceedOn) && - reflect.DeepEqual(r.getNetnsId(a.NetnsPath), newNetnsId) { + reflect.DeepEqual(r.NetNsCache.GetNetNsId(a.NetnsPath), newNetnsId) { return &i, nil } } 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 a8e45f023dcb36a1e0134a19804a386b888f1075 Mon Sep 17 00:00:00 2001 From: Andreas Karis Date: Thu, 23 Oct 2025 02:17:15 +0200 Subject: [PATCH 04/11] 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. Signed-off-by: Andreas Karis --- .../cl_application_program_test.go | 173 ++++++++++++++++- .../ns_application_program_test.go | 174 +++++++++++++++++- 2 files changed, 345 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..b4af749cd 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,34 @@ 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: + require.NotNil(t, program.XDP.Links[0].Priority) + if *program.XDP.Links[0].Priority != helpers.GetPriority(expectedProgram.XDP.Links[0].Priority) { + continue + } + case program.TC != nil: + require.NotNil(t, program.TC.Links[0].Priority) + if *program.TC.Links[0].Priority != helpers.GetPriority(expectedProgram.TC.Links[0].Priority) { + continue + } + case program.TCX != nil: + require.NotNil(t, program.TCX.Links[0].Priority) + 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..630ecd369 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,34 @@ 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: + require.NotNil(t, program.XDP.Links[0].Priority) + if *program.XDP.Links[0].Priority != helpers.GetPriority(expectedProgram.XDP.Links[0].Priority) { + continue + } + case program.TC != nil: + require.NotNil(t, program.TC.Links[0].Priority) + if *program.TC.Links[0].Priority != helpers.GetPriority(expectedProgram.TC.Links[0].Priority) { + continue + } + case program.TCX != nil: + require.NotNil(t, program.TCX.Links[0].Priority) + 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 8d041ea374f7a78023d5bda9b4bee5ecd7127e3a Mon Sep 17 00:00:00 2001 From: Andreas Karis Date: Thu, 23 Oct 2025 14:43:09 +0200 Subject: [PATCH 05/11] 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. Signed-off-by: Andreas Karis --- test/integration/common.go | 226 +++++++++++++++++++++++++++++++ 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, 463 insertions(+), 11 deletions(-) diff --git a/test/integration/common.go b/test/integration/common.go index 64d56a6b6..eafe17cb2 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,218 @@ 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) + require.NotNil(t, l.Priority) + 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) + require.NotNil(t, l.Priority) + 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) + require.NotNil(t, l.Priority) + 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 d65b8de6baa87ad876f49bded0e0f7b7798f78d8 Mon Sep 17 00:00:00 2001 From: Andrew McDermott Date: Mon, 12 Jan 2026 12:14:10 +0000 Subject: [PATCH 06/11] Revert AttachInfoState Priority field to non-pointer type The PR #485 changed Priority from int32 to *int32 in both spec (AttachInfo) and status (AttachInfoState) types. However, the status field should remain int32 because the controller always resolves a concrete priority value after reconciliation. The field is annotated +required but was declared as *int32 with json:"priority,omitempty" - these are contradictory: a required field should not be a pointer with omitempty, as that allows it to be absent from the serialised output. This commit reverts only the status types back to int32 whilst keeping the spec types as *int32 (which correctly allows distinguishing between "not set" and "explicitly set to 0"). Signed-off-by: Andrew McDermott --- apis/v1alpha1/cluster_tc_program_types.go | 2 +- apis/v1alpha1/cluster_tcx_program_types.go | 2 +- apis/v1alpha1/cluster_xdp_program_types.go | 2 +- apis/v1alpha1/tc_program_types.go | 2 +- apis/v1alpha1/tcx_program_types.go | 2 +- apis/v1alpha1/xdp_program_types.go | 2 +- apis/v1alpha1/zz_generated.deepcopy.go | 30 ------------------- .../bases/bpfman.io_bpfapplicationstates.yaml | 3 ++ ...bpfman.io_clusterbpfapplicationstates.yaml | 3 ++ 9 files changed, 12 insertions(+), 36 deletions(-) diff --git a/apis/v1alpha1/cluster_tc_program_types.go b/apis/v1alpha1/cluster_tc_program_types.go index 93f3d6652..7af7620dd 100644 --- a/apis/v1alpha1/cluster_tc_program_types.go +++ b/apis/v1alpha1/cluster_tc_program_types.go @@ -126,7 +126,7 @@ type ClTcAttachInfoState struct { // +required // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=1000 - Priority *int32 `json:"priority,omitempty"` + Priority int32 `json:"priority"` // proceedOn is the provisioned list of proceedOn values. proceedOn allows the // user to call other TC programs in a chain, or not call the next program in a diff --git a/apis/v1alpha1/cluster_tcx_program_types.go b/apis/v1alpha1/cluster_tcx_program_types.go index e163120bd..004fc027b 100644 --- a/apis/v1alpha1/cluster_tcx_program_types.go +++ b/apis/v1alpha1/cluster_tcx_program_types.go @@ -109,5 +109,5 @@ type ClTcxAttachInfoState struct { // +required // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=1000 - Priority *int32 `json:"priority,omitempty"` + Priority int32 `json:"priority"` } diff --git a/apis/v1alpha1/cluster_xdp_program_types.go b/apis/v1alpha1/cluster_xdp_program_types.go index dede988c2..b135f19ea 100644 --- a/apis/v1alpha1/cluster_xdp_program_types.go +++ b/apis/v1alpha1/cluster_xdp_program_types.go @@ -106,7 +106,7 @@ type ClXdpAttachInfoState struct { // +required // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=1000 - Priority *int32 `json:"priority,omitempty"` + Priority int32 `json:"priority"` // proceedOn is the provisioned list of proceedOn values. proceedOn allows the // user to call other TC programs in a chain, or not call the next program in a diff --git a/apis/v1alpha1/tc_program_types.go b/apis/v1alpha1/tc_program_types.go index fdcc6eebc..44839eac6 100644 --- a/apis/v1alpha1/tc_program_types.go +++ b/apis/v1alpha1/tc_program_types.go @@ -122,7 +122,7 @@ type TcAttachInfoState struct { // +required // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=1000 - Priority *int32 `json:"priority,omitempty"` + Priority int32 `json:"priority"` // proceedOn is the provisioned list of proceedOn values. proceedOn allows the // user to call other TC programs in a chain, or not call the next program in a diff --git a/apis/v1alpha1/tcx_program_types.go b/apis/v1alpha1/tcx_program_types.go index cf8c136a1..6f97b3157 100644 --- a/apis/v1alpha1/tcx_program_types.go +++ b/apis/v1alpha1/tcx_program_types.go @@ -108,5 +108,5 @@ type TcxAttachInfoState struct { // +required // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=1000 - Priority *int32 `json:"priority,omitempty"` + Priority int32 `json:"priority"` } diff --git a/apis/v1alpha1/xdp_program_types.go b/apis/v1alpha1/xdp_program_types.go index c9cc5c8c8..f49d33d6b 100644 --- a/apis/v1alpha1/xdp_program_types.go +++ b/apis/v1alpha1/xdp_program_types.go @@ -101,7 +101,7 @@ type XdpAttachInfoState struct { // +required // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=1000 - Priority *int32 `json:"priority,omitempty"` + Priority int32 `json:"priority"` // proceedOn is the provisioned list of proceedOn values. proceedOn allows the // user to call other TC programs in a chain, or not call the next program in a diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index 6d2ea1ebc..eacb30325 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -1016,11 +1016,6 @@ func (in *ClTcAttachInfo) DeepCopy() *ClTcAttachInfo { func (in *ClTcAttachInfoState) DeepCopyInto(out *ClTcAttachInfoState) { *out = *in in.AttachInfoStateCommon.DeepCopyInto(&out.AttachInfoStateCommon) - 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)) @@ -1112,11 +1107,6 @@ func (in *ClTcxAttachInfo) DeepCopy() *ClTcxAttachInfo { func (in *ClTcxAttachInfoState) DeepCopyInto(out *ClTcxAttachInfoState) { *out = *in in.AttachInfoStateCommon.DeepCopyInto(&out.AttachInfoStateCommon) - 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 ClTcxAttachInfoState. @@ -1376,11 +1366,6 @@ func (in *ClXdpAttachInfo) DeepCopy() *ClXdpAttachInfo { func (in *ClXdpAttachInfoState) DeepCopyInto(out *ClXdpAttachInfoState) { *out = *in in.AttachInfoStateCommon.DeepCopyInto(&out.AttachInfoStateCommon) - 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)) @@ -1815,11 +1800,6 @@ func (in *TcAttachInfo) DeepCopy() *TcAttachInfo { func (in *TcAttachInfoState) DeepCopyInto(out *TcAttachInfoState) { *out = *in in.AttachInfoStateCommon.DeepCopyInto(&out.AttachInfoStateCommon) - 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)) @@ -1907,11 +1887,6 @@ func (in *TcxAttachInfo) DeepCopy() *TcxAttachInfo { func (in *TcxAttachInfoState) DeepCopyInto(out *TcxAttachInfoState) { *out = *in in.AttachInfoStateCommon.DeepCopyInto(&out.AttachInfoStateCommon) - 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 TcxAttachInfoState. @@ -2085,11 +2060,6 @@ func (in *XdpAttachInfo) DeepCopy() *XdpAttachInfo { func (in *XdpAttachInfoState) DeepCopyInto(out *XdpAttachInfoState) { *out = *in in.AttachInfoStateCommon.DeepCopyInto(&out.AttachInfoStateCommon) - 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/config/crd/bases/bpfman.io_bpfapplicationstates.yaml b/config/crd/bases/bpfman.io_bpfapplicationstates.yaml index 49f03768a..40ae123e9 100644 --- a/config/crd/bases/bpfman.io_bpfapplicationstates.yaml +++ b/config/crd/bases/bpfman.io_bpfapplicationstates.yaml @@ -275,6 +275,7 @@ spec: - interfaceName - linkStatus - netnsPath + - priority - proceedOn - shouldAttach - uuid @@ -345,6 +346,7 @@ spec: - interfaceName - linkStatus - netnsPath + - priority - shouldAttach - uuid type: object @@ -596,6 +598,7 @@ spec: - interfaceName - linkStatus - netnsPath + - priority - proceedOn - shouldAttach - uuid diff --git a/config/crd/bases/bpfman.io_clusterbpfapplicationstates.yaml b/config/crd/bases/bpfman.io_clusterbpfapplicationstates.yaml index 610f3b094..c097dbfbd 100644 --- a/config/crd/bases/bpfman.io_clusterbpfapplicationstates.yaml +++ b/config/crd/bases/bpfman.io_clusterbpfapplicationstates.yaml @@ -478,6 +478,7 @@ spec: - direction - interfaceName - linkStatus + - priority - proceedOn - shouldAttach - uuid @@ -547,6 +548,7 @@ spec: - direction - interfaceName - linkStatus + - priority - shouldAttach - uuid type: object @@ -870,6 +872,7 @@ spec: required: - interfaceName - linkStatus + - priority - proceedOn - shouldAttach - uuid From 59d5fc772f42ec1020bc7ac77409a4457ac65e1f Mon Sep 17 00:00:00 2001 From: Andrew McDermott Date: Mon, 12 Jan 2026 12:14:26 +0000 Subject: [PATCH 07/11] Update reconcilers for int32 status Priority field With the AttachInfoState Priority field reverted to int32, update the reconcilers to: - Use the Priority value directly when populating status (no longer need GetPriorityPointer) - Compare Priority values directly in findLink functions (no longer need GetPriority wrapper for int32 comparisons) - Use GetPriority only when reading from spec (which remains *int32) Signed-off-by: Andrew McDermott --- controllers/bpfman-agent/cl_tc_program.go | 6 +++--- controllers/bpfman-agent/cl_tcx_program.go | 6 +++--- controllers/bpfman-agent/cl_xdp_program.go | 6 +++--- controllers/bpfman-agent/ns_tc_program.go | 6 +++--- controllers/bpfman-agent/ns_tcx_program.go | 6 +++--- controllers/bpfman-agent/ns_xdp_program.go | 6 +++--- 6 files changed, 18 insertions(+), 18 deletions(-) diff --git a/controllers/bpfman-agent/cl_tc_program.go b/controllers/bpfman-agent/cl_tc_program.go index 4b3d74abf..7f93577e2 100644 --- a/controllers/bpfman-agent/cl_tc_program.go +++ b/controllers/bpfman-agent/cl_tc_program.go @@ -132,7 +132,7 @@ func (r *ClTcProgramReconciler) getAttachRequest() *gobpfman.AttachRequest { } attachInfo := &gobpfman.TCAttachInfo{ - Priority: helpers.GetPriority(r.currentLink.Priority), + Priority: r.currentLink.Priority, Iface: r.currentLink.InterfaceName, Direction: directionToStr(r.currentLink.Direction), ProceedOn: tcProceedOnToInt(r.currentLink.ProceedOn), @@ -223,7 +223,7 @@ func (r *ClTcProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.ClTcAt // same: InterfaceName, Direction, Priority, ProceedOn, and network namespace. if a.InterfaceName == attachInfoState.InterfaceName && a.Direction == attachInfoState.Direction && - helpers.GetPriority(a.Priority) == helpers.GetPriority(attachInfoState.Priority) && + a.Priority == attachInfoState.Priority && reflect.DeepEqual(a.ProceedOn, attachInfoState.ProceedOn) && reflect.DeepEqual(r.NetNsCache.GetNetNsId(a.NetnsPath), newNetnsId) { return &i, nil @@ -307,7 +307,7 @@ func (r *ClTcProgramReconciler) getExpectedLinks(ctx context.Context, attachInfo }, InterfaceName: interfaceName, NetnsPath: netnsPath, - Priority: helpers.GetPriorityPointer(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 507d99115..00ac8febf 100644 --- a/controllers/bpfman-agent/cl_tcx_program.go +++ b/controllers/bpfman-agent/cl_tcx_program.go @@ -98,7 +98,7 @@ func (r *ClTcxProgramReconciler) getAttachRequest() *gobpfman.AttachRequest { } attachInfo := &gobpfman.TCXAttachInfo{ - Priority: helpers.GetPriority(r.currentLink.Priority), + Priority: r.currentLink.Priority, Iface: r.currentLink.InterfaceName, Direction: directionToStr(r.currentLink.Direction), Metadata: map[string]string{internal.UuidMetadataKey: string(r.currentLink.UUID)}, @@ -188,7 +188,7 @@ func (r *ClTcxProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.ClTcx // same: InterfaceName, Direction, Priority, and network namespace. if a.InterfaceName == attachInfoState.InterfaceName && a.Direction == attachInfoState.Direction && - helpers.GetPriority(a.Priority) == helpers.GetPriority(attachInfoState.Priority) && + a.Priority == attachInfoState.Priority && reflect.DeepEqual(r.NetNsCache.GetNetNsId(a.NetnsPath), newNetnsId) { return &i, nil } @@ -271,7 +271,7 @@ func (r *ClTcxProgramReconciler) getExpectedLinks(ctx context.Context, attachInf }, InterfaceName: interfaceName, NetnsPath: netnsPath, - Priority: helpers.GetPriorityPointer(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 4e11c6995..3f6dd2b60 100644 --- a/controllers/bpfman-agent/cl_xdp_program.go +++ b/controllers/bpfman-agent/cl_xdp_program.go @@ -122,7 +122,7 @@ func (r *ClXdpProgramReconciler) getAttachRequest() *gobpfman.AttachRequest { } attachInfo := &gobpfman.XDPAttachInfo{ - Priority: helpers.GetPriority(r.currentLink.Priority), + Priority: r.currentLink.Priority, Iface: r.currentLink.InterfaceName, ProceedOn: xdpProceedOnToInt(r.currentLink.ProceedOn), Metadata: map[string]string{internal.UuidMetadataKey: string(r.currentLink.UUID)}, @@ -211,7 +211,7 @@ func (r *ClXdpProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.ClXdp // attachInfoState is the same as a if the the following fields are the // same: InterfaceName, Priority, ProceedOn, and network namespace. if a.InterfaceName == attachInfoState.InterfaceName && - helpers.GetPriority(a.Priority) == helpers.GetPriority(attachInfoState.Priority) && + a.Priority == attachInfoState.Priority && reflect.DeepEqual(a.ProceedOn, attachInfoState.ProceedOn) && reflect.DeepEqual(r.NetNsCache.GetNetNsId(a.NetnsPath), newNetnsId) { return &i, nil @@ -295,7 +295,7 @@ func (r *ClXdpProgramReconciler) getExpectedLinks(ctx context.Context, attachInf }, InterfaceName: interfaceName, NetnsPath: netnsPath, - Priority: helpers.GetPriorityPointer(attachInfo.Priority), + Priority: helpers.GetPriority(attachInfo.Priority), ProceedOn: attachInfo.ProceedOn, } } diff --git a/controllers/bpfman-agent/ns_tc_program.go b/controllers/bpfman-agent/ns_tc_program.go index 4927c8d86..a91f0ee4f 100644 --- a/controllers/bpfman-agent/ns_tc_program.go +++ b/controllers/bpfman-agent/ns_tc_program.go @@ -97,7 +97,7 @@ func (r *NsTcProgramReconciler) getNamespace() string { func (r *NsTcProgramReconciler) getAttachRequest() *gobpfman.AttachRequest { attachInfo := &gobpfman.TCAttachInfo{ - Priority: helpers.GetPriority(r.currentLink.Priority), + Priority: r.currentLink.Priority, Iface: r.currentLink.InterfaceName, Direction: directionToStr(r.currentLink.Direction), ProceedOn: tcProceedOnToInt(r.currentLink.ProceedOn), @@ -187,7 +187,7 @@ func (r *NsTcProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.TcAtta // same: InterfaceName, Direction, Priority, ProceedOn, and network namespace. if a.InterfaceName == attachInfoState.InterfaceName && a.Direction == attachInfoState.Direction && - helpers.GetPriority(a.Priority) == helpers.GetPriority(attachInfoState.Priority) && + a.Priority == attachInfoState.Priority && reflect.DeepEqual(a.ProceedOn, attachInfoState.ProceedOn) && reflect.DeepEqual(r.NetNsCache.GetNetNsId(a.NetnsPath), newNetnsId) { return &i, nil @@ -295,7 +295,7 @@ func (r *NsTcProgramReconciler) getExpectedLinks(ctx context.Context, attachInfo }, InterfaceName: iface, NetnsPath: netnsPath, - Priority: helpers.GetPriorityPointer(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 4255087ea..58d51621f 100644 --- a/controllers/bpfman-agent/ns_tcx_program.go +++ b/controllers/bpfman-agent/ns_tcx_program.go @@ -97,7 +97,7 @@ func (r *NsTcxProgramReconciler) getNamespace() string { func (r *NsTcxProgramReconciler) getAttachRequest() *gobpfman.AttachRequest { attachInfo := &gobpfman.TCXAttachInfo{ - Priority: helpers.GetPriority(r.currentLink.Priority), + Priority: r.currentLink.Priority, Iface: r.currentLink.InterfaceName, Direction: directionToStr(r.currentLink.Direction), Metadata: map[string]string{internal.UuidMetadataKey: string(r.currentLink.UUID)}, @@ -186,7 +186,7 @@ func (r *NsTcxProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.TcxAt // same: InterfaceName, Direction, Priority, and network namespace. if a.InterfaceName == attachInfoState.InterfaceName && a.Direction == attachInfoState.Direction && - helpers.GetPriority(a.Priority) == helpers.GetPriority(attachInfoState.Priority) && + a.Priority == attachInfoState.Priority && reflect.DeepEqual(r.NetNsCache.GetNetNsId(a.NetnsPath), newNetnsId) { return &i, nil } @@ -294,7 +294,7 @@ func (r *NsTcxProgramReconciler) getExpectedLinks(ctx context.Context, attachInf }, InterfaceName: iface, NetnsPath: netnsPath, - Priority: helpers.GetPriorityPointer(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 97389303e..69b5c43da 100644 --- a/controllers/bpfman-agent/ns_xdp_program.go +++ b/controllers/bpfman-agent/ns_xdp_program.go @@ -97,7 +97,7 @@ func (r *NsXdpProgramReconciler) getNamespace() string { func (r *NsXdpProgramReconciler) getAttachRequest() *gobpfman.AttachRequest { attachInfo := &gobpfman.XDPAttachInfo{ - Priority: helpers.GetPriority(r.currentLink.Priority), + Priority: r.currentLink.Priority, Iface: r.currentLink.InterfaceName, ProceedOn: xdpProceedOnToInt(r.currentLink.ProceedOn), Metadata: map[string]string{internal.UuidMetadataKey: string(r.currentLink.UUID)}, @@ -184,7 +184,7 @@ func (r *NsXdpProgramReconciler) findLink(attachInfoState bpfmaniov1alpha1.XdpAt // attachInfoState is the same as a if the the following fields are the // same: InterfaceName, Priority, ProceedOn, and network namespace. if a.InterfaceName == attachInfoState.InterfaceName && - helpers.GetPriority(a.Priority) == helpers.GetPriority(attachInfoState.Priority) && + a.Priority == attachInfoState.Priority && reflect.DeepEqual(a.ProceedOn, attachInfoState.ProceedOn) && reflect.DeepEqual(r.NetNsCache.GetNetNsId(a.NetnsPath), newNetnsId) { return &i, nil @@ -293,7 +293,7 @@ func (r *NsXdpProgramReconciler) getExpectedLinks(ctx context.Context, attachInf }, InterfaceName: iface, NetnsPath: netnsPath, - Priority: helpers.GetPriorityPointer(attachInfo.Priority), + Priority: helpers.GetPriority(attachInfo.Priority), ProceedOn: attachInfo.ProceedOn, } nodeLinks = append(nodeLinks, link) From c46aa1fa16d25aec062f001c97f2e414bc1e41cc Mon Sep 17 00:00:00 2001 From: Andrew McDermott Date: Mon, 12 Jan 2026 12:14:40 +0000 Subject: [PATCH 08/11] Remove unused GetPriorityPointer helper function With the status Priority field now int32, the GetPriorityPointer helper is no longer needed. Remove it along with its unused ptr import. Signed-off-by: Andrew McDermott --- pkg/helpers/helpers.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/pkg/helpers/helpers.go b/pkg/helpers/helpers.go index 2e7e38e6d..711141850 100644 --- a/pkg/helpers/helpers.go +++ b/pkg/helpers/helpers.go @@ -25,7 +25,6 @@ import ( "k8s.io/client-go/discovery" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" - "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" ) @@ -197,12 +196,3 @@ func GetPriority(priority *int32) int32 { } return *priority } - -// GetPriorityPointer reads a priority value. If priority is nil, return a pointer to the default value (1000). -// Otherwise, return the pointer itself. -func GetPriorityPointer(priority *int32) *int32 { - if priority == nil { - return ptr.To(int32(defaultPriority)) - } - return priority -} From d493a80109badb5dbe5da454dd1f1e35509bec8e Mon Sep 17 00:00:00 2001 From: Andrew McDermott Date: Mon, 12 Jan 2026 12:45:20 +0000 Subject: [PATCH 09/11] Update tests for int32 status Priority field Remove require.NotNil checks and pointer dereferences for status Priority fields which are now int32. The GetPriority helper is still needed when comparing against spec types which remain *int32. Signed-off-by: Andrew McDermott --- controllers/bpfman-agent/cl_application_program_test.go | 9 +++------ controllers/bpfman-agent/ns_application_program_test.go | 9 +++------ test/integration/common.go | 9 +++------ 3 files changed, 9 insertions(+), 18 deletions(-) diff --git a/controllers/bpfman-agent/cl_application_program_test.go b/controllers/bpfman-agent/cl_application_program_test.go index b4af749cd..112b01ebd 100644 --- a/controllers/bpfman-agent/cl_application_program_test.go +++ b/controllers/bpfman-agent/cl_application_program_test.go @@ -481,18 +481,15 @@ func verifyClusterBpfProgramState(t *testing.T, bpfAppState *bpfmaniov1alpha1.Cl } switch { case program.XDP != nil: - require.NotNil(t, program.XDP.Links[0].Priority) - if *program.XDP.Links[0].Priority != helpers.GetPriority(expectedProgram.XDP.Links[0].Priority) { + if program.XDP.Links[0].Priority != helpers.GetPriority(expectedProgram.XDP.Links[0].Priority) { continue } case program.TC != nil: - require.NotNil(t, program.TC.Links[0].Priority) - if *program.TC.Links[0].Priority != helpers.GetPriority(expectedProgram.TC.Links[0].Priority) { + if program.TC.Links[0].Priority != helpers.GetPriority(expectedProgram.TC.Links[0].Priority) { continue } case program.TCX != nil: - require.NotNil(t, program.TCX.Links[0].Priority) - if *program.TCX.Links[0].Priority != helpers.GetPriority(expectedProgram.TCX.Links[0].Priority) { + if program.TCX.Links[0].Priority != helpers.GetPriority(expectedProgram.TCX.Links[0].Priority) { continue } } diff --git a/controllers/bpfman-agent/ns_application_program_test.go b/controllers/bpfman-agent/ns_application_program_test.go index 630ecd369..8db81f50a 100644 --- a/controllers/bpfman-agent/ns_application_program_test.go +++ b/controllers/bpfman-agent/ns_application_program_test.go @@ -453,18 +453,15 @@ func verifyNamespaceBpfProgramState(t *testing.T, bpfAppState *bpfmaniov1alpha1. } switch { case program.XDP != nil: - require.NotNil(t, program.XDP.Links[0].Priority) - if *program.XDP.Links[0].Priority != helpers.GetPriority(expectedProgram.XDP.Links[0].Priority) { + if program.XDP.Links[0].Priority != helpers.GetPriority(expectedProgram.XDP.Links[0].Priority) { continue } case program.TC != nil: - require.NotNil(t, program.TC.Links[0].Priority) - if *program.TC.Links[0].Priority != helpers.GetPriority(expectedProgram.TC.Links[0].Priority) { + if program.TC.Links[0].Priority != helpers.GetPriority(expectedProgram.TC.Links[0].Priority) { continue } case program.TCX != nil: - require.NotNil(t, program.TCX.Links[0].Priority) - if *program.TCX.Links[0].Priority != helpers.GetPriority(expectedProgram.TCX.Links[0].Priority) { + if program.TCX.Links[0].Priority != helpers.GetPriority(expectedProgram.TCX.Links[0].Priority) { continue } } diff --git a/test/integration/common.go b/test/integration/common.go index eafe17cb2..7f1db93e9 100644 --- a/test/integration/common.go +++ b/test/integration/common.go @@ -287,36 +287,33 @@ func getClusterBpfApplicationStateLinks(t *testing.T, appState v1alpha1.ClusterB // Extract XDP program links. for _, l := range program.XDP.Links { require.NotNil(t, l.LinkId) - require.NotNil(t, l.Priority) links = append(links, link{ linkId: *l.LinkId, interfaceName: l.InterfaceName, netnsPath: l.NetnsPath, - priority: *l.Priority, + priority: l.Priority, }) } case v1alpha1.ProgTypeTC: // Extract TC program links. for _, l := range program.TC.Links { require.NotNil(t, l.LinkId) - require.NotNil(t, l.Priority) links = append(links, link{ linkId: *l.LinkId, interfaceName: l.InterfaceName, netnsPath: l.NetnsPath, - priority: *l.Priority, + priority: l.Priority, }) } case v1alpha1.ProgTypeTCX: // Extract TCX program links. for _, l := range program.TCX.Links { require.NotNil(t, l.LinkId) - require.NotNil(t, l.Priority) links = append(links, link{ linkId: *l.LinkId, interfaceName: l.InterfaceName, netnsPath: l.NetnsPath, - priority: *l.Priority, + priority: l.Priority, }) } } From d8ae57cf3f55ac21ff18f751c9c4849e0bd318fc Mon Sep 17 00:00:00 2001 From: Andrew McDermott Date: Mon, 12 Jan 2026 12:45:50 +0000 Subject: [PATCH 10/11] 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 } From 336d155700a072752c8b8c9115083d0ff5d2aa55 Mon Sep 17 00:00:00 2001 From: Andrew McDermott Date: Mon, 12 Jan 2026 12:52:33 +0000 Subject: [PATCH 11/11] Regenerate OLM bundle manifests Update the bundle CRD manifests to reflect the status Priority field being required (int32) rather than optional (*int32). Signed-off-by: Andrew McDermott --- bundle/manifests/bpfman.io_bpfapplicationstates.yaml | 3 +++ bundle/manifests/bpfman.io_clusterbpfapplicationstates.yaml | 3 +++ 2 files changed, 6 insertions(+) diff --git a/bundle/manifests/bpfman.io_bpfapplicationstates.yaml b/bundle/manifests/bpfman.io_bpfapplicationstates.yaml index 20b87adaa..4423ddd49 100644 --- a/bundle/manifests/bpfman.io_bpfapplicationstates.yaml +++ b/bundle/manifests/bpfman.io_bpfapplicationstates.yaml @@ -275,6 +275,7 @@ spec: - interfaceName - linkStatus - netnsPath + - priority - proceedOn - shouldAttach - uuid @@ -345,6 +346,7 @@ spec: - interfaceName - linkStatus - netnsPath + - priority - shouldAttach - uuid type: object @@ -596,6 +598,7 @@ spec: - interfaceName - linkStatus - netnsPath + - priority - proceedOn - shouldAttach - uuid diff --git a/bundle/manifests/bpfman.io_clusterbpfapplicationstates.yaml b/bundle/manifests/bpfman.io_clusterbpfapplicationstates.yaml index aebe008b7..5ee01d9da 100644 --- a/bundle/manifests/bpfman.io_clusterbpfapplicationstates.yaml +++ b/bundle/manifests/bpfman.io_clusterbpfapplicationstates.yaml @@ -478,6 +478,7 @@ spec: - direction - interfaceName - linkStatus + - priority - proceedOn - shouldAttach - uuid @@ -547,6 +548,7 @@ spec: - direction - interfaceName - linkStatus + - priority - shouldAttach - uuid type: object @@ -870,6 +872,7 @@ spec: required: - interfaceName - linkStatus + - priority - proceedOn - shouldAttach - uuid