From 4a97fec6e3d409e8ef04d1df423ad65bfd635824 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20Julia=CC=81n?= Date: Mon, 23 Dec 2024 12:40:41 +0100 Subject: [PATCH 1/8] Add sub-interfaces to modifiers --- pkg/ebpf/manager.go | 107 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 90 insertions(+), 17 deletions(-) diff --git a/pkg/ebpf/manager.go b/pkg/ebpf/manager.go index 06e790609a9ec..8a17ef63775eb 100644 --- a/pkg/ebpf/manager.go +++ b/pkg/ebpf/manager.go @@ -8,6 +8,7 @@ package ebpf import ( + "errors" "fmt" "io" "sync" @@ -51,27 +52,64 @@ func NewManagerWithDefault(mgr *manager.Manager, name string, modifiers ...Modif return NewManager(mgr, name, append(defaultModifiers, modifiers...)...) } -// Modifier is an interface that can be implemented by a package to -// add functionality to the ebpf.Manager. It exposes a name to identify the modifier, -// two functions that will be called before and after the ebpf.Manager.InitWithOptions -// call, and a function that will be called when the manager is stopped. -// Note regarding internal state of the modifier: if the modifier is added to the list of modifiers -// enabled by default (pkg/ebpf/ebpf.go:registerDefaultModifiers), all managers with those default modifiers -// will share the same instance of the modifier. On the other hand, if the modifier is added to a specific -// manager, it can have its own instance of the modifier, unless the caller explicitly uses the same modifier -// instance with different managers. In other words, if the modifier is to have any internal state specific to -// each manager, it should not be added to the list of default modifiers, and developers using it -// should be aware of this behavior. +// Modifier is an interface that can be implemented by a package to add +// functionality to the ebpf.Manager. It exposes a name to identify the +// modifier, and then any of the functions Before/AfterInit, Before/AfterStart, +// Before/AfterStop, that will be called at the corresponding stage of the +// manager lifecycle. To avoid code churn and implementing unnecessary +// functions, the Modifier interface is split into sub-interfaces, each with a +// single function. This way, the developer can implement only the functions +// they need, and the manager will call them at the right time. Note regarding +// internal state of the modifier: if the modifier is added to the list of +// modifiers enabled by default (see NewManagerWithDefault above), all managers +// with those default modifiers will share the same instance of the modifier. On +// the other hand, if the modifier is added to a specific manager, it can have +// its own instance of the modifier, unless the caller explicitly uses the same +// modifier instance with different managers. In other words, if the modifier is +// to have any internal state specific to each manager, it should not be added +// to the list of default modifiers, and developers using it should be aware of +// this behavior. type Modifier interface { fmt.Stringer +} + +// modifierBeforeInit is a sub-interface of Modifier that exposes a BeforeInit method +type modifierBeforeInit interface { + Modifier + // BeforeInit is called before the ebpf.Manager.InitWithOptions call - // names.ModuleName refers to the name associated with Manager instance. + // names.ModuleName refers to the name associated with Manager instance. An + // error returned from this function will stop the initialization process. BeforeInit(*manager.Manager, names.ModuleName, *manager.Options) error +} + +// modifierAfterInit is a sub-interface of Modifier that exposes an AfterInit method +type modifierAfterInit interface { + Modifier // AfterInit is called after the ebpf.Manager.InitWithOptions call AfterInit(*manager.Manager, names.ModuleName, *manager.Options) error } +// modifierBeforeStop is a sub-interface of Modifier that exposes a BeforeStop method +type modifierBeforeStop interface { + Modifier + + // BeforeStop is called before the ebpf.Manager.Stop call. An error returned + // from this function will not prevent the manager from stopping, but it will + // be logged. + BeforeStop(*manager.Manager, names.ModuleName, manager.MapCleanupType) error +} + +// modifierAfterStop is a sub-interface of Modifier that exposes an AfterStop method +type modifierAfterStop interface { + Modifier + + // AfterStop is called after the ebpf.Manager.Stop call. An error returned + // from this function will be logged. + AfterStop(*manager.Manager, names.ModuleName, manager.MapCleanupType) error +} + // InitWithOptions is a wrapper around ebpf-manager.Manager.InitWithOptions func (m *Manager) InitWithOptions(bytecode io.ReaderAt, opts *manager.Options) error { // we must load the ELF file before initialization, @@ -83,8 +121,10 @@ func (m *Manager) InitWithOptions(bytecode io.ReaderAt, opts *manager.Options) e for _, mod := range m.EnabledModifiers { log.Tracef("Running %s manager modifier BeforeInit", mod) - if err := mod.BeforeInit(m.Manager, m.Name, opts); err != nil { - return fmt.Errorf("error running %s manager modifier: %w", mod, err) + if as, ok := mod.(modifierBeforeInit); ok { + if err := as.BeforeInit(m.Manager, m.Name, opts); err != nil { + return fmt.Errorf("error running %s manager modifier: %w", mod, err) + } } } @@ -92,11 +132,44 @@ func (m *Manager) InitWithOptions(bytecode io.ReaderAt, opts *manager.Options) e return err } + var errs error for _, mod := range m.EnabledModifiers { log.Tracef("Running %s manager modifier AfterInit", mod) - if err := mod.AfterInit(m.Manager, m.Name, opts); err != nil { - return fmt.Errorf("error running %s manager modifier: %w", mod, err) + if as, ok := mod.(modifierAfterInit); ok { + if err := as.AfterInit(m.Manager, m.Name, opts); err != nil { + errs = errors.Join(errs, fmt.Errorf("error running %s manager modifier: %w", mod, err)) + } } } - return nil + + return errs +} + +// Stop is a wrapper around ebpf-manager.Manager.Stop +func (m *Manager) Stop(cleanupType manager.MapCleanupType) error { + var errs error + + for _, mod := range m.EnabledModifiers { + log.Tracef("Running %s manager modifier BeforeStop", mod) + if as, ok := mod.(modifierBeforeStop); ok { + if err := as.BeforeStop(m.Manager, m.Name, cleanupType); err != nil { + errs = errors.Join(errs, fmt.Errorf("error running %s manager modifier BeforeStop(): %s", mod, err)) + } + } + } + + if err := m.Manager.Stop(cleanupType); err != nil { + errs = errors.Join(errs, fmt.Errorf("failed to stop manager %w", err)) + } + + for _, mod := range m.EnabledModifiers { + log.Tracef("Running %s manager modifier AfterStop", mod) + if as, ok := mod.(modifierAfterStop); ok { + if err := as.AfterStop(m.Manager, m.Name, cleanupType); err != nil { + errs = errors.Join(errs, fmt.Errorf("error running %s manager modifier AfterStop(): %s", mod, err)) + } + } + } + + return errs } From 9668cc08358f8a27f1eaaf82c1b868128a423140 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20Julia=CC=81n?= Date: Mon, 23 Dec 2024 13:10:02 +0100 Subject: [PATCH 2/8] Add tests --- pkg/ebpf/helper_call_patcher.go | 6 +--- pkg/ebpf/manager.go | 24 ++++++------- pkg/ebpf/manager_test.go | 60 ++++++++++++++++++++++++++++----- pkg/ebpf/printk_patcher.go | 8 ++--- 4 files changed, 67 insertions(+), 31 deletions(-) diff --git a/pkg/ebpf/helper_call_patcher.go b/pkg/ebpf/helper_call_patcher.go index b274d752270a9..380bbab529aeb 100644 --- a/pkg/ebpf/helper_call_patcher.go +++ b/pkg/ebpf/helper_call_patcher.go @@ -40,7 +40,7 @@ var replaceIns = asm.Mov.Imm(asm.R0, 0) // conditionally select eBPF helpers. This should be regarded as a last resort // when the aforementioned options don't apply (prebuilt artifacts, for // example). -func NewHelperCallRemover(helpers ...asm.BuiltinFunc) Modifier { +func NewHelperCallRemover(helpers ...asm.BuiltinFunc) ModifierBeforeInit { return &helperCallRemover{ helpers: helpers, } @@ -81,10 +81,6 @@ func (h *helperCallRemover) BeforeInit(m *manager.Manager, _ names.ModuleName, _ return nil } -func (h *helperCallRemover) AfterInit(*manager.Manager, names.ModuleName, *manager.Options) error { - return nil -} - func (h *helperCallRemover) String() string { return fmt.Sprintf("HelperCallRemover[%+v]", h.helpers) } diff --git a/pkg/ebpf/manager.go b/pkg/ebpf/manager.go index 8a17ef63775eb..f2fc74a753d13 100644 --- a/pkg/ebpf/manager.go +++ b/pkg/ebpf/manager.go @@ -73,8 +73,8 @@ type Modifier interface { fmt.Stringer } -// modifierBeforeInit is a sub-interface of Modifier that exposes a BeforeInit method -type modifierBeforeInit interface { +// ModifierBeforeInit is a sub-interface of Modifier that exposes a BeforeInit method +type ModifierBeforeInit interface { Modifier // BeforeInit is called before the ebpf.Manager.InitWithOptions call @@ -83,16 +83,16 @@ type modifierBeforeInit interface { BeforeInit(*manager.Manager, names.ModuleName, *manager.Options) error } -// modifierAfterInit is a sub-interface of Modifier that exposes an AfterInit method -type modifierAfterInit interface { +// ModifierAfterInit is a sub-interface of Modifier that exposes an AfterInit method +type ModifierAfterInit interface { Modifier // AfterInit is called after the ebpf.Manager.InitWithOptions call AfterInit(*manager.Manager, names.ModuleName, *manager.Options) error } -// modifierBeforeStop is a sub-interface of Modifier that exposes a BeforeStop method -type modifierBeforeStop interface { +// ModifierBeforeStop is a sub-interface of Modifier that exposes a BeforeStop method +type ModifierBeforeStop interface { Modifier // BeforeStop is called before the ebpf.Manager.Stop call. An error returned @@ -101,8 +101,8 @@ type modifierBeforeStop interface { BeforeStop(*manager.Manager, names.ModuleName, manager.MapCleanupType) error } -// modifierAfterStop is a sub-interface of Modifier that exposes an AfterStop method -type modifierAfterStop interface { +// ModifierAfterStop is a sub-interface of Modifier that exposes an AfterStop method +type ModifierAfterStop interface { Modifier // AfterStop is called after the ebpf.Manager.Stop call. An error returned @@ -121,7 +121,7 @@ func (m *Manager) InitWithOptions(bytecode io.ReaderAt, opts *manager.Options) e for _, mod := range m.EnabledModifiers { log.Tracef("Running %s manager modifier BeforeInit", mod) - if as, ok := mod.(modifierBeforeInit); ok { + if as, ok := mod.(ModifierBeforeInit); ok { if err := as.BeforeInit(m.Manager, m.Name, opts); err != nil { return fmt.Errorf("error running %s manager modifier: %w", mod, err) } @@ -135,7 +135,7 @@ func (m *Manager) InitWithOptions(bytecode io.ReaderAt, opts *manager.Options) e var errs error for _, mod := range m.EnabledModifiers { log.Tracef("Running %s manager modifier AfterInit", mod) - if as, ok := mod.(modifierAfterInit); ok { + if as, ok := mod.(ModifierAfterInit); ok { if err := as.AfterInit(m.Manager, m.Name, opts); err != nil { errs = errors.Join(errs, fmt.Errorf("error running %s manager modifier: %w", mod, err)) } @@ -151,7 +151,7 @@ func (m *Manager) Stop(cleanupType manager.MapCleanupType) error { for _, mod := range m.EnabledModifiers { log.Tracef("Running %s manager modifier BeforeStop", mod) - if as, ok := mod.(modifierBeforeStop); ok { + if as, ok := mod.(ModifierBeforeStop); ok { if err := as.BeforeStop(m.Manager, m.Name, cleanupType); err != nil { errs = errors.Join(errs, fmt.Errorf("error running %s manager modifier BeforeStop(): %s", mod, err)) } @@ -164,7 +164,7 @@ func (m *Manager) Stop(cleanupType manager.MapCleanupType) error { for _, mod := range m.EnabledModifiers { log.Tracef("Running %s manager modifier AfterStop", mod) - if as, ok := mod.(modifierAfterStop); ok { + if as, ok := mod.(ModifierAfterStop); ok { if err := as.AfterStop(m.Manager, m.Name, cleanupType); err != nil { errs = errors.Join(errs, fmt.Errorf("error running %s manager modifier AfterStop(): %s", mod, err)) } diff --git a/pkg/ebpf/manager_test.go b/pkg/ebpf/manager_test.go index 32018e808fb0e..6500aebf3fb3e 100644 --- a/pkg/ebpf/manager_test.go +++ b/pkg/ebpf/manager_test.go @@ -9,29 +9,43 @@ package ebpf import ( "testing" - "github.com/DataDog/datadog-agent/pkg/ebpf/names" manager "github.com/DataDog/ebpf-manager" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + "github.com/DataDog/datadog-agent/pkg/ebpf/names" ) -// PrintkPatcherModifier adds an InstructionPatcher to the manager that removes the newline character from log_debug calls if needed type dummyModifier struct { + mock.Mock } const dummyModifierName = "DummyModifier" func (t *dummyModifier) String() string { - return dummyModifierName + args := t.Called() + return args.String(0) +} + +func (t *dummyModifier) BeforeInit(m *manager.Manager, name names.ModuleName, opts *manager.Options) error { + args := t.Called(m, name, opts) + return args.Error(0) } -// BeforeInit adds the patchPrintkNewline function to the manager -func (t *dummyModifier) BeforeInit(_ *manager.Manager, _ names.ModuleName, _ *manager.Options) error { - return nil +func (t *dummyModifier) AfterInit(m *manager.Manager, name names.ModuleName, opts *manager.Options) error { + args := t.Called(m, name, opts) + return args.Error(0) } -// AfterInit is a no-op for this modifier -func (t *dummyModifier) AfterInit(_ *manager.Manager, _ names.ModuleName, _ *manager.Options) error { - return nil +func (t *dummyModifier) BeforeStop(m *manager.Manager, name names.ModuleName, opts *manager.Options) error { + args := t.Called(m, name, opts) + return args.Error(0) +} + +func (t *dummyModifier) AfterStop(m *manager.Manager, name names.ModuleName, opts *manager.Options) error { + args := t.Called(m, name, opts) + return args.Error(0) } func TestNewManagerWithDefault(t *testing.T) { @@ -69,3 +83,31 @@ func TestNewManagerWithDefault(t *testing.T) { }) } } + +func TestManagerInitWithOptions(t *testing.T) { + modifier := &dummyModifier{} + modifier.On("BeforeInit").Return(nil) + modifier.On("AfterInit").Return(nil) + + mgr := NewManager(&manager.Manager{}, "test", modifier) + require.NotNil(t, mgr) + + err := mgr.InitWithOptions(nil, nil) + require.NoError(t, err) + + modifier.AssertExpectations(t) +} + +func TestManagerStop(t *testing.T) { + modifier := &dummyModifier{} + modifier.On("BeforeStop").Return(nil) + modifier.On("AfterStop").Return(nil) + + mgr := NewManager(&manager.Manager{}, "test", modifier) + require.NotNil(t, mgr) + + // The Stop call will fail because the manager is not initialized, but the modifiers should still be called + _ = mgr.Stop(manager.CleanAll) + + modifier.AssertExpectations(t) +} diff --git a/pkg/ebpf/printk_patcher.go b/pkg/ebpf/printk_patcher.go index afda0f4869c6a..b417c8b011415 100644 --- a/pkg/ebpf/printk_patcher.go +++ b/pkg/ebpf/printk_patcher.go @@ -213,6 +213,9 @@ func patchPrintkInstructions(p *ebpf.ProgramSpec) (int, error) { type PrintkPatcherModifier struct { } +// ensure PrintkPatcherModifier implements the ModifierBeforeInit interface +var _ ModifierBeforeInit = &PrintkPatcherModifier{} + func (t *PrintkPatcherModifier) String() string { return "PrintkPatcherModifier" } @@ -222,8 +225,3 @@ func (t *PrintkPatcherModifier) BeforeInit(m *manager.Manager, _ names.ModuleNam m.InstructionPatchers = append(m.InstructionPatchers, patchPrintkNewline) return nil } - -// AfterInit is a no-op for this modifier -func (t *PrintkPatcherModifier) AfterInit(_ *manager.Manager, _ names.ModuleName, _ *manager.Options) error { - return nil -} From d8c0cb374699b8bde4f56e49eb50f30fc185dae8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20Julia=CC=81n?= Date: Mon, 23 Dec 2024 14:03:45 +0100 Subject: [PATCH 3/8] Fix String method --- pkg/ebpf/manager.go | 36 ++++++++++++++++++++---------------- pkg/ebpf/manager_test.go | 4 ++-- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/pkg/ebpf/manager.go b/pkg/ebpf/manager.go index f2fc74a753d13..7574b3885ca73 100644 --- a/pkg/ebpf/manager.go +++ b/pkg/ebpf/manager.go @@ -110,6 +110,18 @@ type ModifierAfterStop interface { AfterStop(*manager.Manager, names.ModuleName, manager.MapCleanupType) error } +func runModifiersOfType[K Modifier](modifiers []Modifier, funcName string, runner func(K) error) error { + var errs error + for _, mod := range modifiers { + if as, ok := mod.(K); ok { + if err := runner(as); err != nil { + errs = errors.Join(errs, fmt.Errorf("error running %s manager modifier: %w", mod, err)) + } + } + } + return errs +} + // InitWithOptions is a wrapper around ebpf-manager.Manager.InitWithOptions func (m *Manager) InitWithOptions(bytecode io.ReaderAt, opts *manager.Options) error { // we must load the ELF file before initialization, @@ -119,28 +131,20 @@ func (m *Manager) InitWithOptions(bytecode io.ReaderAt, opts *manager.Options) e return fmt.Errorf("failed to load elf from reader: %w", err) } - for _, mod := range m.EnabledModifiers { - log.Tracef("Running %s manager modifier BeforeInit", mod) - if as, ok := mod.(ModifierBeforeInit); ok { - if err := as.BeforeInit(m.Manager, m.Name, opts); err != nil { - return fmt.Errorf("error running %s manager modifier: %w", mod, err) - } - } + err := runModifiersOfType(m.EnabledModifiers, "BeforeInit", func(mod ModifierBeforeInit) error { + return mod.BeforeInit(m.Manager, m.Name, opts) + }) + if err != nil { + return err } if err := m.Manager.InitWithOptions(nil, *opts); err != nil { return err } - var errs error - for _, mod := range m.EnabledModifiers { - log.Tracef("Running %s manager modifier AfterInit", mod) - if as, ok := mod.(ModifierAfterInit); ok { - if err := as.AfterInit(m.Manager, m.Name, opts); err != nil { - errs = errors.Join(errs, fmt.Errorf("error running %s manager modifier: %w", mod, err)) - } - } - } + err = runModifiersOfType(m.EnabledModifiers, "AfterInit", func(mod ModifierAfterInit) error { + return mod.AfterInit(m.Manager, m.Name, opts) + }) return errs } diff --git a/pkg/ebpf/manager_test.go b/pkg/ebpf/manager_test.go index 6500aebf3fb3e..4f69fd554a757 100644 --- a/pkg/ebpf/manager_test.go +++ b/pkg/ebpf/manager_test.go @@ -24,8 +24,8 @@ type dummyModifier struct { const dummyModifierName = "DummyModifier" func (t *dummyModifier) String() string { - args := t.Called() - return args.String(0) + // Do not mock this method for simplicity, to avoid having to define it always + return dummyModifierName } func (t *dummyModifier) BeforeInit(m *manager.Manager, name names.ModuleName, opts *manager.Options) error { From 88827ea9403e1bc4b20e555f8a210a8fcd486e97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20Julia=CC=81n?= Date: Mon, 23 Dec 2024 14:16:43 +0100 Subject: [PATCH 4/8] Fix return --- pkg/ebpf/manager.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/ebpf/manager.go b/pkg/ebpf/manager.go index 7574b3885ca73..9af9453a7f7e8 100644 --- a/pkg/ebpf/manager.go +++ b/pkg/ebpf/manager.go @@ -142,11 +142,9 @@ func (m *Manager) InitWithOptions(bytecode io.ReaderAt, opts *manager.Options) e return err } - err = runModifiersOfType(m.EnabledModifiers, "AfterInit", func(mod ModifierAfterInit) error { + return runModifiersOfType(m.EnabledModifiers, "AfterInit", func(mod ModifierAfterInit) error { return mod.AfterInit(m.Manager, m.Name, opts) }) - - return errs } // Stop is a wrapper around ebpf-manager.Manager.Stop From e2eeb4ae58a2e93cb3d8907f9027e28a0222c4b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20Julia=CC=81n?= Date: Mon, 23 Dec 2024 15:45:54 +0100 Subject: [PATCH 5/8] Fix unused variable --- pkg/ebpf/manager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/ebpf/manager.go b/pkg/ebpf/manager.go index 9af9453a7f7e8..e60a04382257d 100644 --- a/pkg/ebpf/manager.go +++ b/pkg/ebpf/manager.go @@ -115,7 +115,7 @@ func runModifiersOfType[K Modifier](modifiers []Modifier, funcName string, runne for _, mod := range modifiers { if as, ok := mod.(K); ok { if err := runner(as); err != nil { - errs = errors.Join(errs, fmt.Errorf("error running %s manager modifier: %w", mod, err)) + errs = errors.Join(errs, fmt.Errorf("error running %s manager modifier %s: %w", mod, funcName, err)) } } } From a892c65cd345c82629a6864392ed966aa04212a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20Julia=CC=81n?= Date: Tue, 24 Dec 2024 18:31:29 +0100 Subject: [PATCH 6/8] Use runModifiersOfType --- pkg/ebpf/manager.go | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/pkg/ebpf/manager.go b/pkg/ebpf/manager.go index e60a04382257d..0cf37a7e1891a 100644 --- a/pkg/ebpf/manager.go +++ b/pkg/ebpf/manager.go @@ -151,27 +151,20 @@ func (m *Manager) InitWithOptions(bytecode io.ReaderAt, opts *manager.Options) e func (m *Manager) Stop(cleanupType manager.MapCleanupType) error { var errs error - for _, mod := range m.EnabledModifiers { - log.Tracef("Running %s manager modifier BeforeStop", mod) - if as, ok := mod.(ModifierBeforeStop); ok { - if err := as.BeforeStop(m.Manager, m.Name, cleanupType); err != nil { - errs = errors.Join(errs, fmt.Errorf("error running %s manager modifier BeforeStop(): %s", mod, err)) - } - } + err := runModifiersOfType(m.EnabledModifiers, "BeforeStop", func(mod ModifierBeforeStop) error { + return mod.BeforeStop(m.Manager, m.Name, cleanupType) + }) + if err != nil { + errs = errors.Join(errs, err) } if err := m.Manager.Stop(cleanupType); err != nil { errs = errors.Join(errs, fmt.Errorf("failed to stop manager %w", err)) } - for _, mod := range m.EnabledModifiers { - log.Tracef("Running %s manager modifier AfterStop", mod) - if as, ok := mod.(ModifierAfterStop); ok { - if err := as.AfterStop(m.Manager, m.Name, cleanupType); err != nil { - errs = errors.Join(errs, fmt.Errorf("error running %s manager modifier AfterStop(): %s", mod, err)) - } - } - } + err = runModifiersOfType(m.EnabledModifiers, "AfterStop", func(mod ModifierAfterStop) error { + return mod.AfterStop(m.Manager, m.Name, cleanupType) + }) - return errs + return errors.Join(errs, err) } From 2da17b46007ff179059a7b0d062302bc4fd2783e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20Julia=CC=81n?= Date: Tue, 24 Dec 2024 18:35:46 +0100 Subject: [PATCH 7/8] Fix tests --- pkg/ebpf/manager_test.go | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/pkg/ebpf/manager_test.go b/pkg/ebpf/manager_test.go index 4f69fd554a757..9c33485e74460 100644 --- a/pkg/ebpf/manager_test.go +++ b/pkg/ebpf/manager_test.go @@ -14,6 +14,7 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + "github.com/DataDog/datadog-agent/pkg/ebpf/bytecode" "github.com/DataDog/datadog-agent/pkg/ebpf/names" ) @@ -38,13 +39,13 @@ func (t *dummyModifier) AfterInit(m *manager.Manager, name names.ModuleName, opt return args.Error(0) } -func (t *dummyModifier) BeforeStop(m *manager.Manager, name names.ModuleName, opts *manager.Options) error { - args := t.Called(m, name, opts) +func (t *dummyModifier) BeforeStop(m *manager.Manager, name names.ModuleName, cleanupType manager.MapCleanupType) error { + args := t.Called(m, name, cleanupType) return args.Error(0) } -func (t *dummyModifier) AfterStop(m *manager.Manager, name names.ModuleName, opts *manager.Options) error { - args := t.Called(m, name, opts) +func (t *dummyModifier) AfterStop(m *manager.Manager, name names.ModuleName, cleanupType manager.MapCleanupType) error { + args := t.Called(m, name, cleanupType) return args.Error(0) } @@ -86,13 +87,21 @@ func TestNewManagerWithDefault(t *testing.T) { func TestManagerInitWithOptions(t *testing.T) { modifier := &dummyModifier{} - modifier.On("BeforeInit").Return(nil) - modifier.On("AfterInit").Return(nil) + modifier.On("BeforeInit", mock.Anything, mock.Anything, mock.Anything).Return(nil) + modifier.On("AfterInit", mock.Anything, mock.Anything, mock.Anything).Return(nil) mgr := NewManager(&manager.Manager{}, "test", modifier) require.NotNil(t, mgr) - err := mgr.InitWithOptions(nil, nil) + // Load a simple eBPF program to test the modifiers + cfg := NewConfig() + require.NotNil(t, cfg) + + buf, err := bytecode.GetReader(cfg.BPFDir, "logdebug-test.o") + require.NoError(t, err) + t.Cleanup(func() { _ = buf.Close }) + + err = mgr.InitWithOptions(buf, nil) require.NoError(t, err) modifier.AssertExpectations(t) @@ -100,8 +109,8 @@ func TestManagerInitWithOptions(t *testing.T) { func TestManagerStop(t *testing.T) { modifier := &dummyModifier{} - modifier.On("BeforeStop").Return(nil) - modifier.On("AfterStop").Return(nil) + modifier.On("BeforeStop", mock.Anything, mock.Anything, mock.Anything).Return(nil) + modifier.On("AfterStop", mock.Anything, mock.Anything, mock.Anything).Return(nil) mgr := NewManager(&manager.Manager{}, "test", modifier) require.NotNil(t, mgr) From 8f061882700827694451608b1ad899b86ecca046 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20Julia=CC=81n?= Date: Thu, 26 Dec 2024 09:51:08 +0100 Subject: [PATCH 8/8] FIx tests --- pkg/ebpf/manager_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/ebpf/manager_test.go b/pkg/ebpf/manager_test.go index 9c33485e74460..0a03659dadfa1 100644 --- a/pkg/ebpf/manager_test.go +++ b/pkg/ebpf/manager_test.go @@ -101,7 +101,8 @@ func TestManagerInitWithOptions(t *testing.T) { require.NoError(t, err) t.Cleanup(func() { _ = buf.Close }) - err = mgr.InitWithOptions(buf, nil) + opts := manager.Options{} + err = mgr.InitWithOptions(buf, &opts) require.NoError(t, err) modifier.AssertExpectations(t)