Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clean up operator initialization code #2901

Merged
merged 5 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions cmd/common/oci.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"fmt"
"time"

log "github.com/sirupsen/logrus"
"github.com/spf13/cobra"

"github.com/inspektor-gadget/inspektor-gadget/cmd/common/frontends/console"
Expand All @@ -41,7 +42,10 @@ func NewRunCommand(rootCmd *cobra.Command, runtime runtime.Runtime, hiddenColumn
ociParams := apihelpers.ToParamDescs(ocihandler.OciHandler.InstanceParams()).ToParams()

// Add operator global flags
operatorsGlobalParamsCollection := operators.GlobalParamsCollection()
opGlobalParams := make(map[string]*params.Params)
for _, op := range operators.GetDataOperators() {
opGlobalParams[op.Name()] = apihelpers.ToParamDescs(op.GlobalParams()).ToParams()
}

// gadget parameters are only available after contacting the server
gadgetParams := make(params.Params, 0)
Expand Down Expand Up @@ -89,6 +93,12 @@ func NewRunCommand(rootCmd *cobra.Command, runtime runtime.Runtime, hiddenColumn

ops := make([]operators.DataOperator, 0)
for _, op := range operators.GetDataOperators() {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we have global parameters for layer operators? I suppose we'll need to add them later on.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not yet, but yeah, we should add them sooner or later.

// Initialize operator
err := op.Init(opGlobalParams[op.Name()])
if err != nil {
log.Warnf("error initializing operator %s: %v", op.Name(), err)
continue
}
ops = append(ops, op)
}
ops = append(ops, clioperator.CLIOperator)
Expand Down Expand Up @@ -197,7 +207,7 @@ func NewRunCommand(rootCmd *cobra.Command, runtime runtime.Runtime, hiddenColumn
AddFlags(cmd, runtimeGlobalParams, nil, runtime)
AddFlags(cmd, runtimeParams, nil, runtime)

for _, operatorParams := range operatorsGlobalParamsCollection {
for _, operatorParams := range opGlobalParams {
AddFlags(cmd, operatorParams, nil, runtime)
}

Expand Down
10 changes: 9 additions & 1 deletion cmd/ig/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
log "github.com/sirupsen/logrus"
"github.com/spf13/cobra"

"github.com/inspektor-gadget/inspektor-gadget/cmd/common"
gadgetservice "github.com/inspektor-gadget/inspektor-gadget/pkg/gadget-service"
"github.com/inspektor-gadget/inspektor-gadget/pkg/gadget-service/api"
"github.com/inspektor-gadget/inspektor-gadget/pkg/runtime"
Expand Down Expand Up @@ -63,6 +64,12 @@ func newDaemonCommand(runtime runtime.Runtime) *cobra.Command {
16384,
"The events buffer length. A low value could impact horizontal scaling.")

service := gadgetservice.NewService(log.StandardLogger())

for _, params := range service.GetOperatorMap() {
common.AddFlags(daemonCmd, params, nil, runtime)
}

daemonCmd.RunE = func(cmd *cobra.Command, args []string) error {
if os.Geteuid() != 0 {
return fmt.Errorf("%s must be run as root to be able to run eBPF programs", filepath.Base(os.Args[0]))
Expand All @@ -86,7 +93,8 @@ func newDaemonCommand(runtime runtime.Runtime) *cobra.Command {
}

log.Infof("starting Inspektor Gadget daemon at %q", socket)
service := gadgetservice.NewService(log.StandardLogger(), eventBufferLength)
service.SetEventBufferLength(eventBufferLength)

return service.Run(gadgetservice.RunConfig{
SocketType: socketType,
SocketPath: socketPath,
Expand Down
3 changes: 2 additions & 1 deletion gadget-container/gadgettracermanager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,8 @@ func main() {
if err != nil {
log.Fatalf("Parsing EVENTS_BUFFER_LENGTH %q: %v", stringBufferLength, err)
}
service := gadgetservice.NewService(log.StandardLogger(), bufferLength)
service := gadgetservice.NewService(log.StandardLogger())
service.SetEventBufferLength(bufferLength)

socketType, socketPath, err := api.ParseSocketAddress(gadgetServiceHost)
if err != nil {
Expand Down
11 changes: 1 addition & 10 deletions pkg/gadget-context/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,20 +41,11 @@ func (c *GadgetContext) initAndPrepareOperators(paramValues api.ParamValues) ([]
log.Debugf("initializing data op %q", op.Name())
opParamPrefix := fmt.Sprintf("operator.%s", op.Name())

// Lazily initialize operator
// TODO: global params should be filled out from a config file or such; maybe it's a better idea not to
// lazily initialize operators at all, but just hand over the config. The "lazy" stuff could then be done
// if the operator is instantiated and needs to do work
err := op.Init(apihelpers.ToParamDescs(op.GlobalParams()).ToParams())
if err != nil {
return nil, fmt.Errorf("initializing operator %q: %w", op.Name(), err)
}

// Get and fill params
instanceParams := op.InstanceParams().AddPrefix(opParamPrefix)
opParamValues := paramValues.ExtractPrefixedValues(opParamPrefix)

err = apihelpers.Validate(instanceParams, opParamValues)
err := apihelpers.Validate(instanceParams, opParamValues)
if err != nil {
return nil, fmt.Errorf("validating params for operator %q: %w", op.Name(), err)
}
Expand Down
19 changes: 17 additions & 2 deletions pkg/gadget-service/service-oci.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,31 @@ import (
"github.com/inspektor-gadget/inspektor-gadget/pkg/logger"
"github.com/inspektor-gadget/inspektor-gadget/pkg/operators"
"github.com/inspektor-gadget/inspektor-gadget/pkg/operators/simple"
"github.com/inspektor-gadget/inspektor-gadget/pkg/params"
)

func (s *Service) initOperators() error {
for op, globalParams := range s.operators {
err := op.Init(globalParams)
if err != nil {
return fmt.Errorf("initializing operator %s: %w", op.Name(), err)
}
}
return nil
}

func (s *Service) GetOperatorMap() map[operators.DataOperator]*params.Params {
return s.operators
}

func (s *Service) GetGadgetInfo(ctx context.Context, req *api.GetGadgetInfoRequest) (*api.GetGadgetInfoResponse, error) {
if req.Version != api.VersionGadgetInfo {
return nil, fmt.Errorf("expected version to be %d, got %d", api.VersionGadgetInfo, req.Version)
}

// Get all available operators
ops := make([]operators.DataOperator, 0)
for _, op := range operators.GetDataOperators() {
for op := range s.operators {
ops = append(ops, op)
}

Expand Down Expand Up @@ -183,7 +198,7 @@ func (s *Service) RunGadget(runGadget api.GadgetManager_RunGadgetServer) error {
)

ops := make([]operators.DataOperator, 0)
for _, op := range operators.GetDataOperators() {
for op := range s.operators {
ops = append(ops, op)
}
ops = append(ops, svc)
Expand Down
26 changes: 22 additions & 4 deletions pkg/gadget-service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,11 @@ import (
gadgetcontext "github.com/inspektor-gadget/inspektor-gadget/pkg/gadget-context"
gadgetregistry "github.com/inspektor-gadget/inspektor-gadget/pkg/gadget-registry"
"github.com/inspektor-gadget/inspektor-gadget/pkg/gadget-service/api"
apihelpers "github.com/inspektor-gadget/inspektor-gadget/pkg/gadget-service/api-helpers"
"github.com/inspektor-gadget/inspektor-gadget/pkg/gadgets"
"github.com/inspektor-gadget/inspektor-gadget/pkg/logger"
"github.com/inspektor-gadget/inspektor-gadget/pkg/operators"
"github.com/inspektor-gadget/inspektor-gadget/pkg/params"
"github.com/inspektor-gadget/inspektor-gadget/pkg/runtime"
"github.com/inspektor-gadget/inspektor-gadget/pkg/runtime/local"
"github.com/inspektor-gadget/inspektor-gadget/pkg/utils/experimental"
Expand All @@ -62,16 +64,27 @@ type Service struct {
logger logger.Logger
servers map[*grpc.Server]struct{}
eventBufferLength uint64

// operators stores all global parameters for DataOperators (non-legacy)
operators map[operators.DataOperator]*params.Params
}

func NewService(defaultLogger logger.Logger, length uint64) *Service {
func NewService(defaultLogger logger.Logger) *Service {
ops := make(map[operators.DataOperator]*params.Params)
for _, op := range operators.GetDataOperators() {
ops[op] = apihelpers.ToParamDescs(op.GlobalParams()).ToParams()
}
return &Service{
servers: map[*grpc.Server]struct{}{},
logger: defaultLogger,
eventBufferLength: length,
servers: map[*grpc.Server]struct{}{},
logger: defaultLogger,
operators: ops,
}
}

func (s *Service) SetEventBufferLength(val uint64) {
s.eventBufferLength = val
}

flyth marked this conversation as resolved.
Show resolved Hide resolved
func (s *Service) GetInfo(ctx context.Context, request *api.InfoRequest) (*api.InfoResponse, error) {
catalog, err := s.runtime.GetCatalog()
if err != nil {
Expand Down Expand Up @@ -328,6 +341,11 @@ func (s *Service) Run(runConfig RunConfig, serverOptions ...grpc.ServerOption) e

s.servers[server] = struct{}{}

err = s.initOperators()
if err != nil {
return fmt.Errorf("initializing operators: %w", err)
}

return server.Serve(s.listener)
}

Expand Down
Loading