Skip to content

Commit

Permalink
More concise naming scheme for BpfPrograms
Browse files Browse the repository at this point in the history 
Create a shorter unique name for BpfProgram objects. For *Programs, the
name is <*Program name>-<random string>.  For BpfApplications, the
unique name is <BpfApplication name>-<bpf program type>-<random string>.

Then, the identifying info for BpfProgram that used to be in the name
is stored in the BpfProgram object.

Fixes: #28

Signed-off-by: Andre Fredette <[email protected]>
  • Loading branch information
@anfredette
anfredette committed Jul 8, 2024
1 parent a90c8b7 commit 2bc7d6c
Show file tree
Hide file tree
Showing 27 changed files with 272 additions and 159 deletions.
39 changes: 23 additions & 16 deletions controllers/bpfman-agent/application-program.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,20 +67,21 @@ func (r *BpfApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Reque
var err error
var complete bool

namePrefix := func(
progName := func(
app bpfmaniov1alpha1.BpfApplication,
prog bpfmaniov1alpha1.BpfApplicationProgram) string {
return app.Name + "-" + strings.ToLower(string(prog.Type)) + "-"
return app.Name + "-" + strings.ToLower(string(prog.Type))
}

for i, a := range appPrograms.Items {
for j, p := range a.Spec.Programs {
switch p.Type {
case bpfmaniov1alpha1.ProgTypeFentry:
appProgramId := fmt.Sprintf("%s-%s", strings.ToLower(string(p.Type)), sanitize(p.Fentry.FunctionName))
fentryProgram := bpfmaniov1alpha1.FentryProgram{
ObjectMeta: metav1.ObjectMeta{
Name: namePrefix(a, p) + sanitize(p.Fentry.FunctionName),
},
Name: progName(a, p),
Labels: map[string]string{internal.AppProgramId: appProgramId}},
Spec: bpfmaniov1alpha1.FentryProgramSpec{
FentryProgramInfo: *p.Fentry,
BpfAppCommon: a.Spec.BpfAppCommon,
Expand All @@ -97,10 +98,11 @@ func (r *BpfApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Reque
complete, res, err = r.reconcileCommon(ctx, rec, fentryObjects)

case bpfmaniov1alpha1.ProgTypeFexit:
appProgramId := fmt.Sprintf("%s-%s", strings.ToLower(string(p.Type)), sanitize(p.Fexit.FunctionName))

Check warning on line 101 in controllers/bpfman-agent/application-program.go

View check run for this annotation

Codecov / codecov/patch

controllers/bpfman-agent/application-program.go#L101 

Added line #L101 was not covered by tests
fexitProgram := bpfmaniov1alpha1.FexitProgram{
ObjectMeta: metav1.ObjectMeta{
Name: namePrefix(a, p) + sanitize(p.Fexit.FunctionName),
},
Name: progName(a, p),
Labels: map[string]string{internal.AppProgramId: appProgramId}},

Check warning on line 105 in controllers/bpfman-agent/application-program.go

View check run for this annotation

Codecov / codecov/patch

controllers/bpfman-agent/application-program.go#L104-L105 

Added lines #L104 - L105 were not covered by tests
Spec: bpfmaniov1alpha1.FexitProgramSpec{
FexitProgramInfo: *p.Fexit,
BpfAppCommon: a.Spec.BpfAppCommon,
Expand All @@ -118,10 +120,11 @@ func (r *BpfApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Reque

case bpfmaniov1alpha1.ProgTypeKprobe,
bpfmaniov1alpha1.ProgTypeKretprobe:
appProgramId := fmt.Sprintf("%s-%s", strings.ToLower(string(p.Type)), sanitize(p.Kprobe.FunctionName))
kprobeProgram := bpfmaniov1alpha1.KprobeProgram{
ObjectMeta: metav1.ObjectMeta{
Name: namePrefix(a, p) + sanitize(p.Kprobe.FunctionName),
},
Name: progName(a, p),
Labels: map[string]string{internal.AppProgramId: appProgramId}},
Spec: bpfmaniov1alpha1.KprobeProgramSpec{
KprobeProgramInfo: *p.Kprobe,
BpfAppCommon: a.Spec.BpfAppCommon,
Expand All @@ -139,10 +142,11 @@ func (r *BpfApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Reque

case bpfmaniov1alpha1.ProgTypeUprobe,
bpfmaniov1alpha1.ProgTypeUretprobe:
appProgramId := fmt.Sprintf("%s-%s", strings.ToLower(string(p.Type)), sanitize(p.Uprobe.FunctionName))

Check warning on line 145 in controllers/bpfman-agent/application-program.go

View check run for this annotation

Codecov / codecov/patch

controllers/bpfman-agent/application-program.go#L145 

Added line #L145 was not covered by tests
uprobeProgram := bpfmaniov1alpha1.UprobeProgram{
ObjectMeta: metav1.ObjectMeta{
Name: namePrefix(a, p) + sanitize(p.Uprobe.FunctionName),
},
Name: progName(a, p),
Labels: map[string]string{internal.AppProgramId: appProgramId}},

Check warning on line 149 in controllers/bpfman-agent/application-program.go

View check run for this annotation

Codecov / codecov/patch

controllers/bpfman-agent/application-program.go#L148-L149 

Added lines #L148 - L149 were not covered by tests
Spec: bpfmaniov1alpha1.UprobeProgramSpec{
UprobeProgramInfo: *p.Uprobe,
BpfAppCommon: a.Spec.BpfAppCommon,
Expand All @@ -159,10 +163,11 @@ func (r *BpfApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Reque
complete, res, err = r.reconcileCommon(ctx, rec, uprobeObjects)

case bpfmaniov1alpha1.ProgTypeTracepoint:
appProgramId := fmt.Sprintf("%s-%s", strings.ToLower(string(p.Type)), sanitize(p.Tracepoint.Names[0]))

Check warning on line 166 in controllers/bpfman-agent/application-program.go

View check run for this annotation

Codecov / codecov/patch

controllers/bpfman-agent/application-program.go#L166 

Added line #L166 was not covered by tests
tracepointProgram := bpfmaniov1alpha1.TracepointProgram{
ObjectMeta: metav1.ObjectMeta{
Name: namePrefix(a, p) + sanitize(p.Tracepoint.Names[0]),
},
Name: progName(a, p),
Labels: map[string]string{internal.AppProgramId: appProgramId}},

Check warning on line 170 in controllers/bpfman-agent/application-program.go

View check run for this annotation

Codecov / codecov/patch

controllers/bpfman-agent/application-program.go#L169-L170 

Added lines #L169 - L170 were not covered by tests
Spec: bpfmaniov1alpha1.TracepointProgramSpec{
TracepointProgramInfo: *p.Tracepoint,
BpfAppCommon: a.Spec.BpfAppCommon,
Expand All @@ -186,10 +191,11 @@ func (r *BpfApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Reque
"app program name", a.Name, "program index", j)
continue
}
appProgramId := fmt.Sprintf("%s-%s-%s", strings.ToLower(string(p.Type)), p.TC.Direction, interfaces[0])

Check warning on line 194 in controllers/bpfman-agent/application-program.go

View check run for this annotation

Codecov / codecov/patch

controllers/bpfman-agent/application-program.go#L194 

Added line #L194 was not covered by tests
tcProgram := bpfmaniov1alpha1.TcProgram{
ObjectMeta: metav1.ObjectMeta{
Name: namePrefix(a, p) + p.TC.Direction + "-" + interfaces[0],
},
Name: progName(a, p),
Labels: map[string]string{internal.AppProgramId: appProgramId}},

Check warning on line 198 in controllers/bpfman-agent/application-program.go

View check run for this annotation

Codecov / codecov/patch

controllers/bpfman-agent/application-program.go#L197-L198 

Added lines #L197 - L198 were not covered by tests
Spec: bpfmaniov1alpha1.TcProgramSpec{
TcProgramInfo: *p.TC,
BpfAppCommon: a.Spec.BpfAppCommon,
Expand All @@ -212,10 +218,11 @@ func (r *BpfApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Reque
"app program name", a.Name, "program index", j)
continue
}
appProgramId := fmt.Sprintf("%s-%s", strings.ToLower(string(p.Type)), interfaces[0])

Check warning on line 221 in controllers/bpfman-agent/application-program.go

View check run for this annotation

Codecov / codecov/patch

controllers/bpfman-agent/application-program.go#L221 

Added line #L221 was not covered by tests
xdpProgram := bpfmaniov1alpha1.XdpProgram{
ObjectMeta: metav1.ObjectMeta{
Name: namePrefix(a, p) + interfaces[0],
},
Name: progName(a, p),
Labels: map[string]string{internal.AppProgramId: appProgramId}},

Check warning on line 225 in controllers/bpfman-agent/application-program.go

View check run for this annotation

Codecov / codecov/patch

controllers/bpfman-agent/application-program.go#L224-L225 

Added lines #L224 - L225 were not covered by tests
Spec: bpfmaniov1alpha1.XdpProgramSpec{
XdpProgramInfo: *p.XDP,
BpfAppCommon: a.Spec.BpfAppCommon,
Expand Down
23 changes: 12 additions & 11 deletions controllers/bpfman-agent/application-program_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,15 @@ func TestBpfApplicationControllerCreate(t *testing.T) {
// fentry program config
bpfFentryFunctionName = "fentry_test"
fentryFunctionName = "do_unlinkat"
fentryBpfProgName = fmt.Sprintf("%s-%s-%s-%s-%s", name, "fentry", "do-unlinkat", fakeNode.Name, "do-unlinkat")
fentryAppProgramId = fmt.Sprintf("%s-%s", "fentry", sanitize(fentryFunctionName))
fentryAttachPoint = sanitize(fentryFunctionName)
fentryBpfProg = &bpfmaniov1alpha1.BpfProgram{}
fentryFakeUID = "ef71d42c-aa21-48e8-a697-82391d801a81"
// kprobe program config
bpfKprobeFunctionName = "kprobe_test"
kprobeFunctionName = "try_to_wake_up"
kprobeBpfProgName = fmt.Sprintf("%s-%s-%s-%s-%s", name, "kprobe", "try-to-wake-up", fakeNode.Name, "try-to-wake-up")
kprobeAppProgramId = fmt.Sprintf("%s-%s", "kprobe", sanitize(kprobeFunctionName))
kprobeAttachPoint = sanitize(kprobeFunctionName)
kprobeBpfProg = &bpfmaniov1alpha1.BpfProgram{}
kprobeFakeUID = "ef71d42c-aa21-48e8-a697-82391d801a82"
kprobeOffset = 0
Expand Down Expand Up @@ -133,14 +135,14 @@ func TestBpfApplicationControllerCreate(t *testing.T) {
}

// Check the BpfProgram Object was created successfully
err = cl.Get(ctx, types.NamespacedName{Name: fentryBpfProgName, Namespace: metav1.NamespaceAll}, fentryBpfProg)
err = rc.getBpfProgram(ctx, name, fentryAppProgramId, fentryAttachPoint, fentryBpfProg)
require.NoError(t, err)

require.NotEmpty(t, fentryBpfProg)
// Finalizer is written
require.Equal(t, internal.BpfApplicationControllerFinalizer, fentryBpfProg.Finalizers[0])
// owningConfig Label was correctly set
require.Equal(t, name, fentryBpfProg.Labels[internal.BpfProgramOwnerLabel])
require.Equal(t, name, fentryBpfProg.Labels[internal.BpfProgramOwner])
// node Label was correctly set
require.Equal(t, fakeNode.Name, fentryBpfProg.Labels[internal.K8sHostLabel])
// fentry function Annotation was correctly set
Expand Down Expand Up @@ -184,7 +186,7 @@ func TestBpfApplicationControllerCreate(t *testing.T) {
}

// Check that the bpfProgram's programs was correctly updated
err = cl.Get(ctx, types.NamespacedName{Name: fentryBpfProgName, Namespace: metav1.NamespaceAll}, fentryBpfProg)
err = rc.getBpfProgram(ctx, name, fentryAppProgramId, fentryAttachPoint, fentryBpfProg)
require.NoError(t, err)

// prog ID should already have been set
Expand All @@ -208,7 +210,7 @@ func TestBpfApplicationControllerCreate(t *testing.T) {
require.False(t, res.Requeue)

// Check that the bpfProgram's status was correctly updated
err = cl.Get(ctx, types.NamespacedName{Name: fentryBpfProgName, Namespace: metav1.NamespaceAll}, fentryBpfProg)
err = rc.getBpfProgram(ctx, name, fentryAppProgramId, fentryAttachPoint, fentryBpfProg)
require.NoError(t, err)

require.Equal(t, string(bpfmaniov1alpha1.BpfProgCondLoaded), fentryBpfProg.Status.Conditions[0].Type)
Expand All @@ -220,14 +222,14 @@ func TestBpfApplicationControllerCreate(t *testing.T) {
t.Fatalf("reconcile: (%v)", err)
}

err = cl.Get(ctx, types.NamespacedName{Name: kprobeBpfProgName, Namespace: metav1.NamespaceAll}, kprobeBpfProg)
err = rc.getBpfProgram(ctx, name, kprobeAppProgramId, kprobeAttachPoint, kprobeBpfProg)
require.NoError(t, err)

require.NotEmpty(t, kprobeBpfProg)
// Finalizer is written
require.Equal(t, internal.BpfApplicationControllerFinalizer, kprobeBpfProg.Finalizers[0])
// owningConfig Label was correctly set
require.Equal(t, name, kprobeBpfProg.Labels[internal.BpfProgramOwnerLabel])
require.Equal(t, name, kprobeBpfProg.Labels[internal.BpfProgramOwner])
// node Label was correctly set
require.Equal(t, fakeNode.Name, kprobeBpfProg.Labels[internal.K8sHostLabel])
// fentry function Annotation was correctly set
Expand Down Expand Up @@ -273,7 +275,7 @@ func TestBpfApplicationControllerCreate(t *testing.T) {
}

// Check that the bpfProgram's programs was correctly updated
err = cl.Get(ctx, types.NamespacedName{Name: kprobeBpfProgName, Namespace: metav1.NamespaceAll}, kprobeBpfProg)
err = rc.getBpfProgram(ctx, name, kprobeAppProgramId, kprobeAttachPoint, kprobeBpfProg)
require.NoError(t, err)

// prog ID should already have been set
Expand All @@ -297,9 +299,8 @@ func TestBpfApplicationControllerCreate(t *testing.T) {
require.False(t, res.Requeue)

// Check that the bpfProgram's status was correctly updated
err = cl.Get(ctx, types.NamespacedName{Name: kprobeBpfProgName, Namespace: metav1.NamespaceAll}, kprobeBpfProg)
err = rc.getBpfProgram(ctx, name, kprobeAppProgramId, kprobeAttachPoint, kprobeBpfProg)
require.NoError(t, err)

require.Equal(t, string(bpfmaniov1alpha1.BpfProgCondLoaded), kprobeBpfProg.Status.Conditions[0].Type)

}
105 changes: 89 additions & 16 deletions controllers/bpfman-agent/common.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (
"github.com/bpfman/bpfman-operator/internal"
gobpfman "github.com/bpfman/bpfman/clients/gobpfman/v1"
"github.com/go-logr/logr"
"github.com/google/uuid"
"google.golang.org/grpc"
)

Expand Down Expand Up @@ -124,6 +125,9 @@ type bpfmanReconciler interface {
getNodeSelector() *metav1.LabelSelector
// getBpfGlobalData returns the Bpf program global variables.
getBpfGlobalData() map[string][]byte
// getAppProgramId() returns the program qualifier for the current
// program. If there is no qualifier, it should return an empty string.
getAppProgramId() string
}

// reconcileCommon is the common reconciler loop called by each bpfman
Expand Down Expand Up @@ -498,17 +502,22 @@ func (r *ReconcilerCommon) updateStatus(ctx context.Context, bpfProgram *bpfmani
return true
}

type bpfProgKey struct {
appProgId string
attachPoint string
}

func (r *ReconcilerCommon) getExistingBpfPrograms(ctx context.Context,
rec bpfmanReconciler) (map[string]bpfmaniov1alpha1.BpfProgram, error) {
rec bpfmanReconciler) (map[bpfProgKey]bpfmaniov1alpha1.BpfProgram, error) {

bpfProgramList := &bpfmaniov1alpha1.BpfProgramList{}

// Only list bpfPrograms for this *Program and the controller's node
opts := []client.ListOption{
client.MatchingLabels{
internal.BpfProgramOwnerLabel: rec.getOwner().GetName(),
internal.BpfParentProgram: rec.getName(),
internal.K8sHostLabel: r.NodeName,
internal.BpfProgramOwner: rec.getOwner().GetName(),
internal.AppProgramId: rec.getAppProgramId(),
internal.K8sHostLabel: r.NodeName,
},
}

Expand All @@ -517,32 +526,47 @@ func (r *ReconcilerCommon) getExistingBpfPrograms(ctx context.Context,
return nil, err
}

existingBpfPrograms := map[string]bpfmaniov1alpha1.BpfProgram{}
existingBpfPrograms := map[bpfProgKey]bpfmaniov1alpha1.BpfProgram{}
for _, bpfProg := range bpfProgramList.Items {
existingBpfPrograms[bpfProg.GetName()] = bpfProg
key := bpfProgKey{
appProgId: bpfProg.GetLabels()[internal.AppProgramId],
attachPoint: bpfProg.GetAnnotations()[internal.BpfProgramAttachPoint],
}
existingBpfPrograms[key] = bpfProg
}

return existingBpfPrograms, nil
}

func generateUniqueName(baseName string) string {
uuid := uuid.New().String()
return fmt.Sprintf("%s-%s", baseName, uuid[:8])
}

// createBpfProgram moves some shared logic for building bpfProgram objects
// into a central location.
func (r *ReconcilerCommon) createBpfProgram(
bpfProgramName string,
attachPoint string,
rec bpfmanReconciler,
annotations map[string]string) (*bpfmaniov1alpha1.BpfProgram, error) {

r.Logger.V(1).Info("createBpfProgram()", "Name", bpfProgramName,
r.Logger.V(1).Info("createBpfProgram()", "Name", attachPoint,
"Owner", rec.getOwner().GetName(), "OwnerType", rec.getRecType(), "Name", rec.getName())

if annotations == nil {
annotations = make(map[string]string)
}

Check warning on line 558 in controllers/bpfman-agent/common.go

View check run for this annotation

Codecov / codecov/patch

controllers/bpfman-agent/common.go#L557-L558 

Added lines #L557 - L558 were not covered by tests
annotations[internal.BpfProgramAttachPoint] = attachPoint

bpfProg := &bpfmaniov1alpha1.BpfProgram{
ObjectMeta: metav1.ObjectMeta{
Name: bpfProgramName,
Name: generateUniqueName(rec.getName()),
Finalizers: []string{rec.getFinalizer()},
Labels: map[string]string{
internal.BpfProgramOwnerLabel: rec.getOwner().GetName(),
internal.BpfParentProgram: rec.getName(),
internal.K8sHostLabel: r.NodeName},
internal.BpfProgramOwner: rec.getOwner().GetName(),
internal.AppProgramId: rec.getAppProgramId(),
internal.K8sHostLabel: r.NodeName,
},
Annotations: annotations,
},
Spec: bpfmaniov1alpha1.BpfProgramSpec{
Expand Down Expand Up @@ -579,7 +603,7 @@ func (r *ReconcilerCommon) createBpfProgram(
func (r *ReconcilerCommon) handleProgDelete(
ctx context.Context,
rec bpfmanReconciler,
existingBpfPrograms map[string]bpfmaniov1alpha1.BpfProgram,
existingBpfPrograms map[bpfProgKey]bpfmaniov1alpha1.BpfProgram,
loadedBpfPrograms map[string]*gobpfman.ListResponse_ListResult,
isNodeSelected bool,
isBeingDeleted bool,
Expand Down Expand Up @@ -640,7 +664,7 @@ func (r *ReconcilerCommon) handleProgDelete(
func (r *ReconcilerCommon) handleProgCreateOrUpdate(
ctx context.Context,
rec bpfmanReconciler,
existingBpfPrograms map[string]bpfmaniov1alpha1.BpfProgram,
existingBpfPrograms map[bpfProgKey]bpfmaniov1alpha1.BpfProgram,
expectedBpfPrograms *bpfmaniov1alpha1.BpfProgramList,
loadedBpfPrograms map[string]*gobpfman.ListResponse_ListResult,
isNodeSelected bool,
Expand All @@ -653,11 +677,15 @@ func (r *ReconcilerCommon) handleProgCreateOrUpdate(
// even if the node isn't selected
for _, expectedBpfProgram := range expectedBpfPrograms.Items {
r.Logger.V(1).Info("Creating or Updating", "Name", expectedBpfProgram.Name)
existingBpfProgram, exists := existingBpfPrograms[expectedBpfProgram.Name]
key := bpfProgKey{
appProgId: expectedBpfProgram.GetLabels()[internal.AppProgramId],
attachPoint: expectedBpfProgram.GetAnnotations()[internal.BpfProgramAttachPoint],
}
existingBpfProgram, exists := existingBpfPrograms[key]
if exists {
// Remove the bpfProgram from the existingPrograms map so we know
// not to delete it below.
delete(existingBpfPrograms, expectedBpfProgram.Name)
delete(existingBpfPrograms, key)
} else {
// Create a new bpfProgram Object for this program.
opts := client.CreateOptions{}
Expand Down Expand Up @@ -899,3 +927,48 @@ func sanitize(name string) string {
name = strings.Replace(strings.Replace(name, "/", "-", -1), "_", "-", -1)
return strings.ToLower(name)
}

func appProgramId(labels map[string]string) string {
id, ok := labels[internal.AppProgramId]
if ok {
return id
}
return ""
}

// getBpfProgram returns a BpfProgram object in the bpfProgram parameter based
// on the given owner, appProgId, and attachPoint. If the BpfProgram is not
// found, an error is returned.
func (r *ReconcilerCommon) getBpfProgram(
ctx context.Context,
owner string,
appProgId string,
attachPoint string,
bpfProgram *bpfmaniov1alpha1.BpfProgram) error {

bpfProgramList := &bpfmaniov1alpha1.BpfProgramList{}

// Only list bpfPrograms for this *Program and the controller's node
opts := []client.ListOption{
client.MatchingLabels{
internal.BpfProgramOwner: owner,
internal.AppProgramId: appProgId,
internal.K8sHostLabel: r.NodeName,
},
}

err := r.List(ctx, bpfProgramList, opts...)
if err != nil {
return err
}

Check warning on line 963 in controllers/bpfman-agent/common.go

View check run for this annotation

Codecov / codecov/patch

controllers/bpfman-agent/common.go#L962-L963 

Added lines #L962 - L963 were not covered by tests

for _, bpfProg := range bpfProgramList.Items {
if appProgId == bpfProg.GetLabels()[internal.AppProgramId] &&
attachPoint == bpfProg.GetAnnotations()[internal.BpfProgramAttachPoint] {
*bpfProgram = bpfProg
return nil
}
}

return fmt.Errorf("bpfProgram not found")

Check warning on line 973 in controllers/bpfman-agent/common.go

View check run for this annotation

Codecov / codecov/patch

controllers/bpfman-agent/common.go#L973 

Added line #L973 was not covered by tests
}
Loading

0 comments on commit 2bc7d6c

Please sign in to comment.