Repository: microsoft/retina
File: pkg/managers/pluginmanager/pluginmanager.go (Go)

Code before:
func (p *PluginManager) Start(ctx context.Context) error {
	counter := p.tel.StartPerf("start-plugin-manager")
	p.l.Info("Starting plugin manager ...")
	var err error

	if p.cfg == nil {
		return ErrNilCfg
	}

	if p.cfg.MetricsInterval == 0 {
		return ErrZeroInterval
	}

	if p.cfg.EnablePodLevel {
		p.l.Info("starting watchers")

		// Start watcher manager
		if err := p.watcherManager.Start(ctx); err != nil {
			return errors.Wrap(err, "failed to start watcher manager")
		}
	}

	g, ctx := errgroup.WithContext(ctx)

	// run conntrack GC only if packetparser is enabled
	if p.isPluginEnabled("packetparser") {
		ct, err := conntrack.New()
		if err != nil {
			return errors.Wrap(err, "failed to get conntrack instance")
		}
		g.Go(func() error {
			return errors.Wrapf(ct.Run(ctx), "failed to run conntrack GC")
		})
	} else {
		p.l.Info("Skipping Conntrack GC loop as packetparser plugin is not enabled")
	}

	// start all plugins
	for _, plugin := range p.plugins {
		plug := plugin

		reconcilectx, cancel := context.WithTimeout(ctx, time.Duration(MAX_RECONCILE_TIME))
		defer cancel()
		err = p.Reconcile(reconcilectx, plug)
		if err != nil {
			// Update control plane metrics counter
			metrics.PluginManagerFailedToReconcileCounter.WithLabelValues(plugin.Name()).Inc()
			return errors.Wrapf(err, "failed to reconcile plugin %s", plugin.Name())
		}

		g.Go(func() error {

Review comment (refactor):
Thanks for refactoring this PR @manojks1999 

To improve efficiency, we can use `plugins` map  of `PluginManager` to perform an O(1) lookup. For example:

```go
// to ignore the first return value, which is the value associated with the key we can use _
// isPacketParserEnabled is a boolean that will be true if the key name exists in the map 
// and false otherwise.
_, isPacketParserEnabled := p.plugins["packetparser"]
```

For reference only, the plugin map gets populated here:
https://github.com/microsoft/retina/blob/e4427126cb97813d5267a0f4618931447606aeb7/pkg/managers/pluginmanager/pluginmanager.go#L61-L67

Code after:
func (p *PluginManager) Start(ctx context.Context) error {
	counter := p.tel.StartPerf("start-plugin-manager")
	p.l.Info("Starting plugin manager ...")
	var err error

	if p.cfg == nil {
		return ErrNilCfg
	}

	if p.cfg.MetricsInterval == 0 {
		return ErrZeroInterval
	}

	if p.cfg.EnablePodLevel {
		p.l.Info("starting watchers")

		// Start watcher manager
		if err := p.watcherManager.Start(ctx); err != nil {
			return errors.Wrap(err, "failed to start watcher manager")
		}
	}

	g, ctx := errgroup.WithContext(ctx)
	_, isPacketParserEnabled := p.plugins["packetparser"]
	// run conntrack GC only if packetparser is enabled
	if isPacketParserEnabled {
		ct, connErr := conntrack.New()
		if connErr != nil {
			return errors.Wrap(connErr, "failed to get conntrack instance")
		}
		g.Go(func() error {
			return errors.Wrapf(ct.Run(ctx), "failed to run conntrack GC")
		})
	} else {
		p.l.Info("Skipping Conntrack GC loop as packetparser plugin is not enabled")
	}

	// start all plugins
	for _, plugin := range p.plugins {
		plug := plugin

		reconcilectx, cancel := context.WithTimeout(ctx, time.Duration(MAX_RECONCILE_TIME))
		defer cancel()
		err = p.Reconcile(reconcilectx, plug)
		if err != nil {
			// Update control plane metrics counter
			metrics.PluginManagerFailedToReconcileCounter.WithLabelValues(plugin.Name()).Inc()
			return errors.Wrapf(err, "failed to reconcile plugin %s", plugin.Name())
		}

		g.Go(func() error {