test: add unit tests for bpf restart and update logic#1766
test: add unit tests for bpf restart and update logic#1766Aryanburnwal05 wants to merge 1 commit into
Conversation
kmesh-bot
requested review from
Okabe-Rintarou-0,
hzxuzhonghu and
supercharge-xsy
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the reliability of the Kmesh codebase by introducing critical unit tests for BPF restart and update mechanisms, as well as XDP authentication handlers. These changes ensure that structural changes and lifecycle events are correctly managed, while also providing the necessary infrastructure for kernel-native eBPF support and updating core testing dependencies. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. The tests were missing, cold and bare, Now coverage fills the empty air. With maps and structs and diffs in line, The Kmesh code will surely shine. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several unit tests for BPF restart, update, and XDP auth handler functionality, upgrades the testify dependency, initializes prev_key to NULL in map_restore, and adds auto-generated bpf2go files. The review feedback suggests making the Kubernetes start-type inference test hermetic by mocking the client creation with gomonkey, and skipping the XDP connection reset test in unprivileged environments where BPF map creation or memlock removal is not permitted.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| ) |
There was a problem hiding this comment.
Add the necessary imports for mocking the Kubernetes client and using gomonkey to make the tests hermetic.
import (
"context"
"errors"
"testing"
"github.com/agiledragon/gomonkey/v2"
"github.com/stretchr/testify/assert"
appsv1 "k8s.io/api/apps/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/kubernetes/fake"
"kmesh.net/kmesh/pkg/kube"
)| func TestInferNextStartType(t *testing.T) { | ||
| // Since we don't have a mocked kube client or a real cluster running, | ||
| // kube.CreateKubeClient will return an error and the function will fallback to Normal start. | ||
| startType := InferNextStartType() | ||
| assert.Equal(t, Normal, startType) | ||
| } |
There was a problem hiding this comment.
This test is currently non-hermetic. If a developer runs this test locally with a valid kubeconfig pointing to a real or local Kubernetes cluster, kube.CreateKubeClient("") will succeed instead of failing. It will then attempt to make an actual API call to the cluster to fetch a DaemonSet, which can lead to unexpected test failures, hangs, or security/permission issues depending on the active kubeconfig context.
To make this test hermetic and robust, we should mock the Kubernetes client creation. Since gomonkey is already a dependency in go.mod, we can use it to mock kube.CreateKubeClient and test both the Normal fallback path and the Restart success path.
func TestInferNextStartType_Normal(t *testing.T) {
patches := gomonkey.ApplyFunc(kube.CreateKubeClient, func(string) (kubernetes.Interface, error) {
return nil, errors.New("mocked error")
})
defer patches.Reset()
startType := InferNextStartType()
assert.Equal(t, Normal, startType)
}
func TestInferNextStartType_Restart(t *testing.T) {
t.Setenv("POD_NAME", "kmesh-daemonset-12345")
t.Setenv("POD_NAMESPACE", "kmesh-system")
fakeClient := fake.NewSimpleClientset(&appsv1.DaemonSet{
ObjectMeta: metav1.ObjectMeta{
Name: "kmesh",
Namespace: "kmesh-system",
},
})
patches := gomonkey.ApplyFunc(kube.CreateKubeClient, func(string) (kubernetes.Interface, error) {
return fakeClient, nil
})
defer patches.Reset()
startType := InferNextStartType()
assert.Equal(t, Restart, startType)
}| func Test_xdpNotifyConnRst(t *testing.T) { | ||
| err := rlimit.RemoveMemlock() | ||
| require.NoError(t, err, "failed to remove memlock") | ||
|
|
||
| mapOfAuth, err := ebpf.NewMap(&ebpf.MapSpec{ | ||
| Name: "test_map_of_auth_result", | ||
| Type: ebpf.Hash, | ||
| KeySize: uint32(TUPLE_LEN), | ||
| ValueSize: uint32(unsafe.Sizeof(uint32(0))), | ||
| MaxEntries: 4096, | ||
| }) | ||
| require.NoError(t, err, "failed to create ebpf map") | ||
| defer mapOfAuth.Close() |
There was a problem hiding this comment.
If this test is run in an unprivileged environment (e.g., non-root or lacking CAP_BPF/CAP_SYS_RESOURCE), rlimit.RemoveMemlock() or ebpf.NewMap() will fail, causing the test suite to fail. To prevent CI/CD pipeline failures in unprivileged environments, we should skip the test if we don't have the required privileges.
func Test_xdpNotifyConnRst(t *testing.T) {
err := rlimit.RemoveMemlock()
if err != nil {
t.Skipf("skipping test; failed to remove memlock (requires root/CAP_SYS_RESOURCE): %v", err)
}
mapOfAuth, err := ebpf.NewMap(&ebpf.MapSpec{
Name: "test_map_of_auth_result",
Type: ebpf.Hash,
KeySize: uint32(TUPLE_LEN),
ValueSize: uint32(unsafe.Sizeof(uint32(0))),
MaxEntries: 4096,
})
if err != nil {
t.Skipf("skipping test; failed to create ebpf map (requires root/CAP_BPF): %v", err)
}
defer mapOfAuth.Close()
Aryanburnwal05
force-pushed
the
test-bpf-restart
branch
from
9e4c2eb to
f68152e
Compare
There was a problem hiding this comment.
Pull request overview
This PR primarily adds unit tests to improve coverage for BPF restart/update logic, and also includes a dependency bump plus additional unrelated generated/artifact changes.
Changes:
- Added unit tests for
pkg/bpf/restartcovering start/exit lifecycle state and struct-layout diffing / recreate decisions. - Added an auth-related unit test for
xdpNotifyConnRst. - Updated dependencies (
testify) and added generatedbpf2goartifacts, plus a small C fix in map restore logic.
Reviewed changes
Copilot reviewed 2 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/bpf/restart/bpf_update_test.go | Adds tests for unionKeys, DiffStructInfoAgainstBTF, and needsRecreate. |
| pkg/bpf/restart/bpf_restart_test.go | Adds tests for start/exit type getters/setters and InferNextStartType. |
| pkg/auth/xdp_auth_handler_test.go | Adds tests for xdpNotifyConnRst key mutation/insertion behavior. |
| go.mod | Bumps github.com/stretchr/testify to v1.11.1. |
| go.sum | Adds checksum entries for github.com/stretchr/testify v1.11.1. |
| bpf/kmesh/bpf2go/kernelnative/kmeshsockopscompat_bpfel.go | Adds generated bpf2go bindings for kernel-native sockops compat (little-endian). |
| bpf/kmesh/bpf2go/kernelnative/kmeshsockopscompat_bpfeb.go | Adds generated bpf2go bindings for kernel-native sockops compat (big-endian). |
| bpf/kmesh/bpf2go/kernelnative/kmeshsockops_bpfel.go | Adds generated bpf2go bindings for kernel-native sockops (little-endian). |
| bpf/kmesh/bpf2go/kernelnative/kmeshsockops_bpfeb.go | Adds generated bpf2go bindings for kernel-native sockops (big-endian). |
| bpf/kmesh/bpf2go/kernelnative/kmeshcgroupsockcompat_bpfel.go | Adds generated bpf2go bindings for kernel-native cgroup sock compat (little-endian). |
| bpf/kmesh/bpf2go/kernelnative/kmeshcgroupsockcompat_bpfeb.go | Adds generated bpf2go bindings for kernel-native cgroup sock compat (big-endian). |
| bpf/kmesh/bpf2go/kernelnative/kmeshcgroupsock_bpfel.go | Adds generated bpf2go bindings for kernel-native cgroup sock (little-endian). |
| bpf/kmesh/bpf2go/kernelnative/kmeshcgroupsock_bpfeb.go | Adds generated bpf2go bindings for kernel-native cgroup sock (big-endian). |
| bpf/deserialization_to_bpf_map/deserialization_to_bpf_map.c | Initializes prev_key to NULL before the first bpf_map_get_next_key call. |
Files not reviewed (8)
- bpf/kmesh/bpf2go/kernelnative/kmeshcgroupsock_bpfeb.go: Generated file
- bpf/kmesh/bpf2go/kernelnative/kmeshcgroupsock_bpfel.go: Generated file
- bpf/kmesh/bpf2go/kernelnative/kmeshcgroupsockcompat_bpfeb.go: Generated file
- bpf/kmesh/bpf2go/kernelnative/kmeshcgroupsockcompat_bpfel.go: Generated file
- bpf/kmesh/bpf2go/kernelnative/kmeshsockops_bpfeb.go: Generated file
- bpf/kmesh/bpf2go/kernelnative/kmeshsockops_bpfel.go: Generated file
- bpf/kmesh/bpf2go/kernelnative/kmeshsockopscompat_bpfeb.go: Generated file
- bpf/kmesh/bpf2go/kernelnative/kmeshsockopscompat_bpfel.go: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,36 @@ | |||
| package restart | |||
| @@ -0,0 +1,108 @@ | |||
| package restart | |||
| func TestInferNextStartType(t *testing.T) { | ||
| // Since we don't have a mocked kube client or a real cluster running, | ||
| // kube.CreateKubeClient will return an error and the function will fallback to Normal start. | ||
| startType := InferNextStartType() | ||
| assert.Equal(t, Normal, startType) | ||
| } |
| t.Run("nil map", func(t *testing.T) { | ||
| key := make([]byte, TUPLE_LEN) | ||
| err := xdpNotifyConnRst(nil, constants.MSG_TYPE_IPV4, key) | ||
| assert.Error(t, err) | ||
| assert.Equal(t, "map_of_auth_result is nil", err.Error()) | ||
| }) |
| github.com/spf13/cobra v1.8.1 | ||
| github.com/spf13/pflag v1.0.7 | ||
| github.com/stretchr/testify v1.10.0 | ||
| github.com/stretchr/testify v1.11.1 |
| // Code generated by bpf2go; DO NOT EDIT. | ||
| //go:build 386 || amd64 || arm || arm64 || loong64 || mips64le || mipsle || ppc64le || riscv64 | ||
|
|
||
| package bpf2go |
This PR adds missing unit tests for the
pkg/bpf/restartpackage to improve coverage around the bpf map lifecycle and struct diffing logic.Changes included:
bpf_restart_test.go: Added tests for tracking start/exit lifecycle states and fallback behaviors forInferNextStartType.bpf_update_test.go: Added coverage forDiffStructInfoAgainstBTFto verify how it handles structural changes (like field additions, removals, offset or type changes). Also covers map recreation scenarios (needsRecreate) and utility functions.To run the tests locally:
go test -v kmesh.net/kmesh/pkg/bpf/restart