sts-sponsors team mailing list archive

[Anton Troyanov <mp+441702@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> diff --git a/src/maasagent/cmd/netmon/main.go b/src/maasagent/cmd/netmon/main.go
> index e83655f..5acd102 100644
> --- a/src/maasagent/cmd/netmon/main.go
> +++ b/src/maasagent/cmd/netmon/main.go
> @@ -1,9 +1,80 @@
>  package main
>  
> +/*
> +	Copyright 2023 Canonical Ltd.  This software is licensed under the
> +	GNU Affero General Public License version 3 (see the file LICENSE).
> +*/
> +
>  import (
> +	"context"
> +	"encoding/json"
> +	"errors"
> +	"os"
> +
> +	"github.com/rs/zerolog"
> +	"github.com/rs/zerolog/log"
> +	"golang.org/x/sync/errgroup"
> +
>  	"launchpad.net/maas/maas/src/maasagent/internal/netmon"
>  )
>  
> +var (
> +	ErrMissingIface = errors.New("Missing interface argument")
> +)
> +
> +func Run() int {
> +	zerolog.SetGlobalLevel(zerolog.InfoLevel)
> +	log.Logger = log.Output(zerolog.ConsoleWriter{Out: os.Stderr})
> +
> +	if envLogLevel, ok := os.LookupEnv("LOG_LEVEL"); ok {
> +		if logLevel, err := zerolog.ParseLevel(envLogLevel); err != nil {
> +			log.Warn().Str("LOG_LEVEL", envLogLevel).Msg("Unknown log level, defaulting to INFO")
> +		} else {
> +			zerolog.SetGlobalLevel(logLevel)
> +		}
> +	}
> +
> +	if len(os.Args) < 2 {
> +		log.Error().Err(ErrMissingIface).Msg("Please provide an interface to monitor")
> +		return 2
> +	}
> +	iface := os.Args[1]
> +
> +	ctx, cancel := context.WithCancel(context.Background())
> +
> +	sigC := make(chan os.Signal)
> +	resultC := make(chan netmon.Result)
> +
> +	g, ctx := errgroup.WithContext(ctx)
> +	g.SetLimit(2)
> +
> +	svc := netmon.NewService(iface)
> +	g.Go(func() error {
> +		return svc.Start(ctx, resultC)
> +	})
> +	g.Go(func() error {
> +		encoder := json.NewEncoder(os.Stdout)
> +		for {
> +			select {
> +			case <-sigC:
> +				cancel()
> +				return nil
> +			case res := <-resultC:

I think this place might lead to a possible issue. 
It is because of a `resultC` channel not being checked if it is closed or not.

When we pass it to, that Start function might return with an error and close the channel. Leading us in an infinite select {} reading from the closed channel.

func (s *Service) Start(ctx context.Context, resultC chan<- Result) error

> +				err := encoder.Encode(res)
> +				if err != nil {
> +					return err
> +				}
> +			}
> +		}
> +	})
> +	log.Info().Msg("Service netmon started")
> +	if err := g.Wait(); err != nil {
> +		log.Error().Err(err).Send()
> +		return 1
> +	}
> +	return 0
> +}
> +
>  func main() {
> -	netmon.NewService()
> +	os.Exit(Run())
>  }


-- 
https://code.launchpad.net/~cgrabowski/maas/+git/maas/+merge/441702
Your team MAAS Committers is subscribed to branch maas:master.