Skip to content

Commit

Permalink
sensors: introduce SensorIface interface
Browse files Browse the repository at this point in the history
current code use a struct Sensor, which makes testing the sensor manager
code hard. This patch introduces a sensor interface.

Subsequent patches will use the interface for testing.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
  • Loading branch information
kkourt committed Jun 10, 2024
1 parent 30ebb6c commit 788cac7
Show file tree
Hide file tree
Showing 16 changed files with 82 additions and 36 deletions.
2 changes: 1 addition & 1 deletion pkg/bench/bench.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func runTetragon(ctx context.Context, configFile string, args *Arguments, summar
}

<-ctx.Done()
sensors.UnloadSensors([]*sensors.Sensor{benchSensors, baseSensors})
sensors.UnloadSensors([]sensors.SensorIface{benchSensors, baseSensors})
}

func sigHandler(ctx context.Context, cancel context.CancelFunc) {
Expand Down
12 changes: 9 additions & 3 deletions pkg/observer/observertesthelper/observer_test_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ func GetDefaultSensorsWithFile(tb testing.TB, file, lib string, opts ...TestOpti
}
}

var sens []*sensors.Sensor
var sens []sensors.SensorIface

if tp != nil {
sens, err = sensors.SensorsFromPolicy(tp, policyfilter.NoFilterID)
Expand All @@ -360,7 +360,13 @@ func GetDefaultSensorsWithFile(tb testing.TB, file, lib string, opts ...TestOpti
}

sens = append(sens, base)
return sens, nil
ret := make([]*sensors.Sensor, 0, len(sens))
for _, si := range sens {
if s, ok := si.(*sensors.Sensor); ok {
ret = append(ret, s)
}
}
return ret, nil
}

func loadExporter(tb testing.TB, ctx context.Context, obs *observer.Observer, opts *testExporterOptions, oo *testObserverOptions) error {
Expand Down Expand Up @@ -458,7 +464,7 @@ func loadObserver(tb testing.TB, ctx context.Context, base *sensors.Sensor,
return nil
}

func loadSensors(tb testing.TB, base *sensors.Sensor, sens []*sensors.Sensor) error {
func loadSensors(tb testing.TB, base sensors.SensorIface, sens []sensors.SensorIface) error {
if err := base.Load(option.Config.BpfDir); err != nil {
tb.Fatalf("Load base error: %s\n", err)
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/sensors/collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (ck *collectionKey) String() string {
// This can either be creating from a tracing policy, or by loading sensors indepenently for sensors
// that are not loaded via a tracing policy (e.g., base sensor) and testing.
type collection struct {
sensors []*Sensor
sensors []SensorIface
name string
err error
// fields below are only set for tracing policies
Expand Down Expand Up @@ -97,21 +97,21 @@ func (c *collection) load(bpfDir string) error {

var err error
for _, sensor := range c.sensors {
if sensor.Loaded {
if sensor.IsLoaded() {
// NB: For now, we don't treat a sensor already loaded as an error
// because that would complicate things.
continue
}
if err = sensor.Load(bpfDir); err != nil {
err = fmt.Errorf("sensor %s from collection %s failed to load: %s", sensor.Name, c.name, err)
err = fmt.Errorf("sensor %s from collection %s failed to load: %s", sensor.GetName(), c.name, err)
break
}
}

// if there was an error, try to unload all the sensors
if err != nil {
// NB: we could try to unload sensors going back from the one that failed, but since
// unload() checks s.Loaded, is easier to just to use unload().
// unload() checks s.IsLoaded, is easier to just to use unload().
if unloadErr := c.unload(); unloadErr != nil {
err = multierr.Append(err, fmt.Errorf("unloading after loading failure failed: %w", unloadErr))
}
Expand All @@ -124,7 +124,7 @@ func (c *collection) load(bpfDir string) error {
func (c *collection) unload() error {
var err error
for _, s := range c.sensors {
if !s.Loaded {
if !s.IsLoaded() {
continue
}
unloadErr := s.Unload()
Expand Down Expand Up @@ -173,7 +173,7 @@ func (cm *collectionMap) listPolicies() []*tetragon.TracingPolicyStatus {
}

for _, sens := range col.sensors {
pol.Sensors = append(pol.Sensors, sens.Name)
pol.Sensors = append(pol.Sensors, sens.GetName())
}

ret = append(ret, &pol)
Expand Down
16 changes: 8 additions & 8 deletions pkg/sensors/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func (h *handler) allocPolicyID() uint64 {
}

// revive:disable:exported
func SensorsFromPolicy(tp tracingpolicy.TracingPolicy, filterID policyfilter.PolicyID) ([]*Sensor, error) {
func SensorsFromPolicy(tp tracingpolicy.TracingPolicy, filterID policyfilter.PolicyID) ([]SensorIface, error) {
return sensorsFromPolicyHandlers(tp, filterID)
}

Expand Down Expand Up @@ -133,7 +133,8 @@ func (h *handler) addTracingPolicy(op *tracingPolicyAdd) error {
col.state = LoadErrorState
return err
}
col.sensors = sensors
col.sensors = make([]SensorIface, 0, len(sensors))
col.sensors = append(col.sensors, sensors...)
col.state = LoadingState

// unlock so that policyLister can access the collections (read-only) while we are loading.
Expand Down Expand Up @@ -244,7 +245,7 @@ func (h *handler) addSensor(op *sensorAdd) error {
return fmt.Errorf("sensor %s already exists", ck)
}
collections[ck] = &collection{
sensors: []*Sensor{op.sensor},
sensors: []SensorIface{op.sensor},
name: op.name,
}
return nil
Expand Down Expand Up @@ -322,8 +323,8 @@ func (h *handler) listSensors(op *sensorList) error {
colInfo := col.info()
for _, s := range col.sensors {
ret = append(ret, SensorStatus{
Name: s.Name,
Enabled: s.Loaded,
Name: s.GetName(),
Enabled: s.IsLoaded(),
Collection: colInfo,
})
}
Expand All @@ -332,10 +333,9 @@ func (h *handler) listSensors(op *sensorList) error {
return nil
}

func sensorsFromPolicyHandlers(tp tracingpolicy.TracingPolicy, filterID policyfilter.PolicyID) ([]*Sensor, error) {
var sensors []*Sensor
func sensorsFromPolicyHandlers(tp tracingpolicy.TracingPolicy, filterID policyfilter.PolicyID) ([]SensorIface, error) {
var sensors []SensorIface
for n, s := range registeredPolicyHandlers {
var sensor *Sensor
sensor, err := s.PolicyHandler(tp, filterID)
if err != nil {
return nil, fmt.Errorf("policy handler '%s' failed loading policy '%s': %w", n, tp.TpName(), err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/sensors/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ func unloadProgram(prog *program.Program) {
log.Info("BPF prog was unloaded")
}

func UnloadSensors(sens []*Sensor) {
func UnloadSensors(sens []SensorIface) {
for i := range sens {
if err := sens[i].Unload(); err != nil {
logger.GetLogger().Warnf("Failed to unload sensor: %s", err)
Expand Down
4 changes: 2 additions & 2 deletions pkg/sensors/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ import (
)

type dummyHandler struct {
s *Sensor
s SensorIface
e error
}

func (d *dummyHandler) PolicyHandler(_ tracingpolicy.TracingPolicy, _ policyfilter.PolicyID) (*Sensor, error) {
func (d *dummyHandler) PolicyHandler(_ tracingpolicy.TracingPolicy, _ policyfilter.PolicyID) (SensorIface, error) {
return d.s, d.e
}

Expand Down
32 changes: 29 additions & 3 deletions pkg/sensors/sensors.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,23 @@ type Sensor struct {
DestroyHook SensorHook
}

// SensorIface is an interface for sensors.Sensor that allows implementing sensors for testing.
type SensorIface interface {
GetName() string
IsLoaded() bool
Load(bpfDir string) error
Unload() error
Destroy()
}

func (s *Sensor) GetName() string {
return s.Name
}

func (s *Sensor) IsLoaded() bool {
return s.Loaded
}

// SensorHook is the function signature for an optional function
// that can be called during sensor unloading and removing.
type SensorHook func() error
Expand All @@ -84,7 +101,7 @@ type policyHandler interface {
// PolicyHandler returns a Sensor for a given policy
// sensors that support policyfilter can use the filterID to implement filtering.
// sensors that do not support policyfilter need to return an error if filterID != policyfilter.NoFilterID
PolicyHandler(policy tracingpolicy.TracingPolicy, filterID policyfilter.PolicyID) (*Sensor, error)
PolicyHandler(policy tracingpolicy.TracingPolicy, filterID policyfilter.PolicyID) (SensorIface, error)
}

type probeLoader interface {
Expand Down Expand Up @@ -132,11 +149,20 @@ type LoadProbeArgs struct {
Version, Verbose int
}

func GetMergedSensorFromParserPolicy(tp tracingpolicy.TracingPolicy) (*Sensor, error) {
func GetMergedSensorFromParserPolicy(tp tracingpolicy.TracingPolicy) (SensorIface, error) {
// NB: use a filter id of 0, so no filtering will happen
sensors, err := SensorsFromPolicy(tp, policyfilter.NoFilterID)
sis, err := SensorsFromPolicy(tp, policyfilter.NoFilterID)
if err != nil {
return nil, err
}
sensors := make([]*Sensor, 0, len(sis))
for _, si := range sis {
s, ok := si.(*Sensor)
if !ok {
return nil, fmt.Errorf("cannot merge sensor of type %T", si)
}
sensors = append(sensors, s)
}

return SensorCombine(tp.TpName(), sensors...), nil
}
2 changes: 1 addition & 1 deletion pkg/sensors/tracing/enforcer.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func (kp *enforcerPolicy) enforcerDel(name string) bool {
func (kp *enforcerPolicy) PolicyHandler(
policy tracingpolicy.TracingPolicy,
_ policyfilter.PolicyID,
) (*sensors.Sensor, error) {
) (sensors.SensorIface, error) {

spec := policy.TpSpec()

Expand Down
2 changes: 1 addition & 1 deletion pkg/sensors/tracing/genericuprobe.go
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ func createUprobeSensorFromEntry(uprobeEntry *genericUprobe,
func (k *observerUprobeSensor) PolicyHandler(
p tracingpolicy.TracingPolicy,
fid policyfilter.PolicyID,
) (*sensors.Sensor, error) {
) (sensors.SensorIface, error) {
spec := p.TpSpec()

if len(spec.UProbes) == 0 {
Expand Down
10 changes: 6 additions & 4 deletions pkg/sensors/tracing/kprobe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4314,22 +4314,24 @@ spec:
type: "size_t"
`

var sens []*sensors.Sensor
var err error

readConfigHook := []byte(readHook)
err = os.WriteFile(testConfigFile, readConfigHook, 0644)
if err != nil {
t.Fatalf("writeFile(%s): err %s", testConfigFile, err)
}
sens, err = observertesthelper.GetDefaultSensorsWithFile(t, testConfigFile, tus.Conf().TetragonLib, observertesthelper.WithMyPid())
sens, err := observertesthelper.GetDefaultSensorsWithFile(t, testConfigFile, tus.Conf().TetragonLib, observertesthelper.WithMyPid())
if err != nil {
t.Fatalf("GetDefaultObserverWithFile error: %s", err)
}

tus.CheckSensorLoad(sens, sensorMaps, sensorProgs, t)

sensors.UnloadSensors(sens)
sensi := make([]sensors.SensorIface, 0, len(sens))
for _, s := range sens {
sensi = append(sensi, s)
}
sensors.UnloadSensors(sensi)
}

func TestFakeSyscallError(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/sensors/tracing/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func hasLoaderEvents() bool {
return bpf.HasBuildId() && kernels.MinKernelVersion("5.19.0")
}

func (k *loaderSensor) PolicyHandler(p tracingpolicy.TracingPolicy, fid policyfilter.PolicyID) (*sensors.Sensor, error) {
func (k *loaderSensor) PolicyHandler(p tracingpolicy.TracingPolicy, fid policyfilter.PolicyID) (sensors.SensorIface, error) {
spec := p.TpSpec()
// NB: no loader section is the spec, so nothing to do
if !spec.Loader {
Expand Down
2 changes: 1 addition & 1 deletion pkg/sensors/tracing/policyhandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func init() {
func (h policyHandler) PolicyHandler(
policy tracingpolicy.TracingPolicy,
policyID policyfilter.PolicyID,
) (*sensors.Sensor, error) {
) (sensors.SensorIface, error) {

policyName := policy.TpName()
spec := policy.TpSpec()
Expand Down
2 changes: 1 addition & 1 deletion pkg/sensors/tracing/selectors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func loadGenericSensorTest(t *testing.T, spec *v1alpha1.TracingPolicySpec) *sens
tus.LoadSensor(t, base.GetInitialSensor())
tus.LoadSensor(t, testsensor.GetTestSensor())
tus.LoadSensor(t, tpSensor)
return tpSensor
return tpSensor.(*sensors.Sensor)
}

// lseekTestOps retruns a function to perform test lseek operations using the
Expand Down
6 changes: 5 additions & 1 deletion pkg/sensors/tracing/tracepoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,11 @@ spec:

tus.CheckSensorLoad(sens, sensorMaps, sensorProgs, t)

sensors.UnloadSensors(sens)
sensi := make([]sensors.SensorIface, 0, len(sens))
for _, s := range sens {
sensi = append(sensi, s)
}
sensors.UnloadSensors(sensi)
}

func TestTracepointCloneThreads(t *testing.T) {
Expand Down
6 changes: 5 additions & 1 deletion pkg/sensors/tracing/uprobe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,11 @@ spec:

tus.CheckSensorLoad(sens, sensorMaps, sensorProgs, t)

sensors.UnloadSensors(sens)
sensi := make([]sensors.SensorIface, 0, len(sens))
for _, s := range sens {
sensi = append(sensi, s)
}
sensors.UnloadSensors(sensi)
}

func TestUprobeGeneric(t *testing.T) {
Expand Down
6 changes: 5 additions & 1 deletion pkg/testutils/sensors/sensors.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ import (
)

// LoadSensor is a helper for loading a sensor in tests
func LoadSensor(t *testing.T, sensor *sensors.Sensor) {
func LoadSensor(t *testing.T, sensori sensors.SensorIface) {
sensor, ok := sensori.(*sensors.Sensor)
if !ok {
t.Fatalf("Cannot call LoadSensor on type %T", sensori)
}

if err := sensor.FindPrograms(); err != nil {
t.Fatalf("ObserverFindProgs error: %s", err)
Expand Down

0 comments on commit 788cac7

Please sign in to comment.