From 51bb04379ad71b731ebe8eeb263bb879ca2bde73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Thu, 19 Oct 2023 13:00:27 +0200 Subject: [PATCH 01/10] Landscape mock: small readability improvement --- .../landscapemockservice/landscape_mock_service.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/mocks/landscape/landscapemockservice/landscape_mock_service.go b/mocks/landscape/landscapemockservice/landscape_mock_service.go index f1a07a4ad..1baf6c219 100644 --- a/mocks/landscape/landscapemockservice/landscape_mock_service.go +++ b/mocks/landscape/landscapemockservice/landscape_mock_service.go @@ -135,7 +135,7 @@ func (s *Service) Connect(stream landscapeapi.LandscapeHostAgent_ConnectServer) } func (s *Service) firstContact(ctx context.Context, cancel func(), hostInfo HostInfo, stream landscapeapi.LandscapeHostAgent_ConnectServer) (uid string, onDisconect func(), err error) { - if other, ok := s.hosts[hostInfo.UID]; ok && other.connected != nil && *other.connected { + if s.isConnected(hostInfo.UID) { return "", nil, fmt.Errorf("UID collision: %q", hostInfo.UID) } @@ -186,6 +186,10 @@ func (s *Service) IsConnected(uid string) bool { s.mu.RLock() defer s.mu.RUnlock() + return s.isConnected(uid) +} + +func (s *Service) isConnected(uid string) bool { host, ok := s.hosts[uid] return ok && host.connected != nil && *host.connected } From a5f774f93ae528b11521cb1e164d81582a29a4e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Thu, 19 Oct 2023 13:00:41 +0200 Subject: [PATCH 02/10] Landscape mock: introduce logging --- .../landscapemockservice/landscape_mock_service.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/mocks/landscape/landscapemockservice/landscape_mock_service.go b/mocks/landscape/landscapemockservice/landscape_mock_service.go index 1baf6c219..3caf11736 100644 --- a/mocks/landscape/landscapemockservice/landscape_mock_service.go +++ b/mocks/landscape/landscapemockservice/landscape_mock_service.go @@ -5,6 +5,7 @@ package landscapemockservice import ( "context" "fmt" + "log/slog" "math/rand" "sync" @@ -108,6 +109,7 @@ func (s *Service) Connect(stream landscapeapi.LandscapeHostAgent_ConnectServer) select { case hostInfo = <-ch: case <-ctx.Done(): + slog.Info(fmt.Sprintf("Landscape: %s: terminated connection: %v", hostInfo.Hostname, ctx.Err())) return nil } @@ -116,14 +118,19 @@ func (s *Service) Connect(stream landscapeapi.LandscapeHostAgent_ConnectServer) s.recvLog = append(s.recvLog, hostInfo) if firstContact { + slog.Info(fmt.Sprintf("Landscape: %s: New connection", hostInfo.Hostname)) + firstContact = false uid, onDisconnect, err := s.firstContact(ctx, cancel, hostInfo, stream) if err != nil { s.mu.Unlock() + slog.Info(fmt.Sprintf("Landscape: %s: terminated connection: %v", hostInfo.Hostname, err)) return err } defer onDisconnect() hostInfo.UID = uid + } else { + slog.Info(fmt.Sprintf("Landscape: %s: Received update: %+v", hostInfo.Hostname, hostInfo)) } h := s.hosts[hostInfo.UID] @@ -204,6 +211,8 @@ func (s *Service) SendCommand(ctx context.Context, uid string, command *landscap return fmt.Errorf("UID %q not connected", uid) } + slog.Info(fmt.Sprintf("Landscape: %s: sending command %T: %v", conn.info.Hostname, command.GetCmd(), command.GetCmd())) + return conn.send(command) } @@ -239,6 +248,7 @@ func (s *Service) Disconnect(uid string) error { return fmt.Errorf("UID %q not registered", uid) } + slog.Info(fmt.Sprintf("Landscape: %s: requested disconnection", host.info.Hostname)) host.stop() return nil } From 034067204f3dfb6012e646b6196a912882966cc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 31 Oct 2023 10:29:09 +0100 Subject: [PATCH 03/10] Landscape mock: reorganize main function Now that the mock Landscape service has loging, we need a way of setting it from the command line. To avoid parsing even more arguments manually, I switched to Cobra with a very similar structure to that of the mock contract server. --- mocks/landscape/landscaperepl/main.go | 125 ++++++++++++++++++-------- 1 file changed, 90 insertions(+), 35 deletions(-) diff --git a/mocks/landscape/landscaperepl/main.go b/mocks/landscape/landscaperepl/main.go index 34c2c1267..47c9bf2e4 100644 --- a/mocks/landscape/landscaperepl/main.go +++ b/mocks/landscape/landscaperepl/main.go @@ -7,64 +7,120 @@ import ( "context" "errors" "fmt" - "log" + "log/slog" "net" "os" + "path/filepath" + "strconv" "strings" landscapeapi "github.com/canonical/landscape-hostagent-api" "github.com/canonical/ubuntu-pro-for-windows/mocks/landscape/landscapemockservice" + "github.com/spf13/cobra" "google.golang.org/grpc" ) func main() { - ctx := context.Background() + rootCmd := rootCmd() - if len(os.Args) != 2 || os.Args[1] == "--help" { - log.Fatalf("Usage: %s ADDRESS", os.Args[0]) + rootCmd.PersistentFlags().CountP("verbosity", "v", "WARNING (-v) INFO (-vv), DEBUG (-vvv)") + rootCmd.Flags().StringP("address", "a", "localhost:8000", "Overrides the address where the server will be hosted") + + if err := rootCmd.Execute(); err != nil { + slog.Error(fmt.Sprintf("Error executing: %v", err)) + os.Exit(1) } - addr := os.Args[1] +} - populateCommands() +func rootCmd() *cobra.Command { + return &cobra.Command{ + Use: executableName(), + Short: "A mock server for Landscape hostagent testing", + Long: `Landscape mock REPL mocks a Landscape hostagent server +on your command line. Hosted at the specified address.`, + PersistentPreRunE: func(cmd *cobra.Command, args []string) error { + // Force a visit of the local flags so persistent flags for all parents are merged. + cmd.LocalFlags() + + // command parsing has been successful. Returns to not print usage anymore. + cmd.SilenceUsage = true + + v := cmd.Flag("verbosity").Value.String() + n, err := strconv.Atoi(v) + if err != nil { + return fmt.Errorf("could not parse verbosity: %v", err) + } + + setVerboseMode(n) + return nil + }, + Run: func(cmd *cobra.Command, args []string) { + ctx := context.Background() + + addr := cmd.Flag("address").Value.String() + fmt.Printf("Hosting on %s\n", addr) + + populateCommands() + fmt.Println("Write `help` to see a list of available commands") + + var cfg net.ListenConfig + lis, err := cfg.Listen(ctx, "tcp", addr) + if err != nil { + slog.Error(fmt.Sprintf("Can't listen: %v", err)) + return + } + defer lis.Close() + + server := grpc.NewServer() + service := landscapemockservice.New() + landscapeapi.RegisterLandscapeHostAgentServer(server, service) + + go func() { + err := server.Serve(lis) + if err != nil { + slog.Error(fmt.Sprintf("Server exited with an error: %v", err)) + } + }() + defer server.Stop() + + if err := run(ctx, service); err != nil { + slog.Error(err.Error()) + return + } + }, + } +} - var cfg net.ListenConfig - lis, err := cfg.Listen(ctx, "tcp", addr) +func executableName() string { + exe, err := os.Executable() if err != nil { - log.Fatalf("Can't listen: %v", err) + return "landscaperepl" } - defer lis.Close() - - server := grpc.NewServer() - service := landscapemockservice.New() - landscapeapi.RegisterLandscapeHostAgentServer(server, service) - - go func() { - err := server.Serve(lis) - if err != nil { - log.Fatalf("Server exited with an error: %v", err) - } - }() - defer server.Stop() + return filepath.Base(exe) +} - if err := run(ctx, service); err != nil { - log.Fatalf("%v", err) +// setVerboseMode changes the verbosity of the logs. +func setVerboseMode(n int) { + var level slog.Level + switch n { + case 0: + level = slog.LevelError + case 1: + level = slog.LevelWarn + case 2: + level = slog.LevelInfo + default: + level = slog.LevelDebug } + + h := slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: level}) + slog.SetDefault(slog.New(h)) } // run contains the main execution loop. func run(ctx context.Context, s *landscapemockservice.Service) error { sc := bufio.NewScanner(os.Stdin) - prefix := "$ " - - fi, _ := os.Stdin.Stat() - if (fi.Mode() & os.ModeCharDevice) == 0 { - // data is from pipe - prefix = "" - } - - fmt.Print(prefix) - // READ for sc.Scan() { line := strings.TrimSpace(sc.Text()) @@ -85,7 +141,6 @@ func run(ctx context.Context, s *landscapemockservice.Service) error { // LOOP fmt.Println() - fmt.Print(prefix) } if err := sc.Err(); err != nil { From fb31ab0f438e945e953fd695cfcbc14f52ae3095 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Thu, 19 Oct 2023 12:59:59 +0200 Subject: [PATCH 04/10] Landscape mock: fix issue were distros would receive a new UID every time --- .../landscape_mock_service.go | 43 +++++++++++++++---- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/mocks/landscape/landscapemockservice/landscape_mock_service.go b/mocks/landscape/landscapemockservice/landscape_mock_service.go index 3caf11736..2230c99c0 100644 --- a/mocks/landscape/landscapemockservice/landscape_mock_service.go +++ b/mocks/landscape/landscapemockservice/landscape_mock_service.go @@ -81,37 +81,64 @@ func New() *Service { // Connect implements the Connect API call. // Upon first contact ever, a UID is randombly assigned to the host and sent to it. // In subsequent contacts, this UID will be its unique identifier. -func (s *Service) Connect(stream landscapeapi.LandscapeHostAgent_ConnectServer) error { +func (s *Service) Connect(stream landscapeapi.LandscapeHostAgent_ConnectServer) (err error) { ctx, cancel := context.WithCancel(stream.Context()) defer cancel() firstContact := true - ch := make(chan HostInfo) + + type msg struct { + info HostInfo + err error + } + + ch := make(chan msg) defer close(ch) + var hostInfo HostInfo for { go func() { recv, err := stream.Recv() - if err != nil { - return + + var m msg + m.err = err + if err == nil { + m.info = newHostInfo(recv) } + // Avoid sending if ctx is done select { case <-ctx.Done(): - return default: } - ch <- newHostInfo(recv) + select { + case <-ctx.Done(): + case ch <- m: + } }() - var hostInfo HostInfo + var m msg + select { + case m = <-ch: + case <-ctx.Done(): + slog.Info(fmt.Sprintf("Landscape: %s: terminated connection: %v", hostInfo.Hostname, ctx.Err())) + return nil + } + + // Avoid races when ctx.Done and ch were both ready select { - case hostInfo = <-ch: case <-ctx.Done(): slog.Info(fmt.Sprintf("Landscape: %s: terminated connection: %v", hostInfo.Hostname, ctx.Err())) return nil + default: + } + + if m.err != nil { + slog.Info(fmt.Sprintf("Landscape: %s: terminated connection: %v", hostInfo.Hostname, err)) + return err } + hostInfo = m.info s.mu.Lock() From 4710da166ad3edf41a796b461308bd9f8a6d781b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Fri, 20 Oct 2023 10:07:10 +0200 Subject: [PATCH 05/10] Landscape mock: Fix a panic by refactoring the recv loop This fixes a panic (write to closed channel) in the tests. The problem was that multiple goroutines were writing to the same channel, so none of them could reliably be in charge of closing it. Now there is a single writing goroutine, which closes the channel on exit. --- .../landscape_mock_service.go | 86 +++++++++---------- 1 file changed, 43 insertions(+), 43 deletions(-) diff --git a/mocks/landscape/landscapemockservice/landscape_mock_service.go b/mocks/landscape/landscapemockservice/landscape_mock_service.go index 2230c99c0..8cddecd13 100644 --- a/mocks/landscape/landscapemockservice/landscape_mock_service.go +++ b/mocks/landscape/landscapemockservice/landscape_mock_service.go @@ -85,60 +85,26 @@ func (s *Service) Connect(stream landscapeapi.LandscapeHostAgent_ConnectServer) ctx, cancel := context.WithCancel(stream.Context()) defer cancel() - firstContact := true - - type msg struct { - info HostInfo - err error - } - - ch := make(chan msg) - defer close(ch) + recv := asyncRecv(ctx, stream) + // We keep the hostInfo outside the loop so we can log messages with the hostname. var hostInfo HostInfo - for { - go func() { - recv, err := stream.Recv() - - var m msg - m.err = err - if err == nil { - m.info = newHostInfo(recv) - } - - // Avoid sending if ctx is done - select { - case <-ctx.Done(): - default: - } - - select { - case <-ctx.Done(): - case ch <- m: - } - }() - var m msg - select { - case m = <-ch: - case <-ctx.Done(): - slog.Info(fmt.Sprintf("Landscape: %s: terminated connection: %v", hostInfo.Hostname, ctx.Err())) - return nil - } - - // Avoid races when ctx.Done and ch were both ready + firstContact := true + for { + var msg recvMsg select { + case msg = <-recv: case <-ctx.Done(): slog.Info(fmt.Sprintf("Landscape: %s: terminated connection: %v", hostInfo.Hostname, ctx.Err())) return nil - default: } - if m.err != nil { - slog.Info(fmt.Sprintf("Landscape: %s: terminated connection: %v", hostInfo.Hostname, err)) + if msg.err != nil { + slog.Info(fmt.Sprintf("Landscape: %s: terminated connection: %v", hostInfo.Hostname, msg.err)) return err } - hostInfo = m.info + hostInfo = msg.info s.mu.Lock() @@ -168,6 +134,40 @@ func (s *Service) Connect(stream landscapeapi.LandscapeHostAgent_ConnectServer) } } +// recvMsg is the sanitixed reuturn type of a GRPC Recv, used to pass by channel. +type recvMsg struct { + info HostInfo + err error +} + +// This goroutine is an asynchronous GRPC Recv. +// Usually, you cannot select between a context and a GRPC receive. This function allows you to. +// It'll keep receiving until an error is returned, or the context is cancelled. +func asyncRecv(ctx context.Context, stream landscapeapi.LandscapeHostAgent_ConnectServer) <-chan recvMsg { + ch := make(chan recvMsg) + + go func() { + defer close(ch) + + for { + var msg recvMsg + recv, err := stream.Recv() + msg.err = err + if recv != nil { + msg.info = newHostInfo(recv) + } + + select { + case <-ctx.Done(): + return + case ch <- msg: + } + } + }() + + return ch +} + func (s *Service) firstContact(ctx context.Context, cancel func(), hostInfo HostInfo, stream landscapeapi.LandscapeHostAgent_ConnectServer) (uid string, onDisconect func(), err error) { if s.isConnected(hostInfo.UID) { return "", nil, fmt.Errorf("UID collision: %q", hostInfo.UID) From 6541a881627d1d9750a722973c906971bbc28951 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Mon, 6 Nov 2023 13:16:19 +0100 Subject: [PATCH 06/10] Fixed typo --- mocks/landscape/landscapemockservice/landscape_mock_service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mocks/landscape/landscapemockservice/landscape_mock_service.go b/mocks/landscape/landscapemockservice/landscape_mock_service.go index 8cddecd13..cea18dec1 100644 --- a/mocks/landscape/landscapemockservice/landscape_mock_service.go +++ b/mocks/landscape/landscapemockservice/landscape_mock_service.go @@ -134,7 +134,7 @@ func (s *Service) Connect(stream landscapeapi.LandscapeHostAgent_ConnectServer) } } -// recvMsg is the sanitixed reuturn type of a GRPC Recv, used to pass by channel. +// recvMsg is the sanitized reuturn type of a GRPC Recv, used to pass by channel. type recvMsg struct { info HostInfo err error From 9b167c72c4d2a7f4b37339b7f21275ad146554b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 14 Nov 2023 10:39:41 +0100 Subject: [PATCH 07/10] Improve comments --- .../landscapemockservice/landscape_mock_service.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/mocks/landscape/landscapemockservice/landscape_mock_service.go b/mocks/landscape/landscapemockservice/landscape_mock_service.go index cea18dec1..6d863dc9e 100644 --- a/mocks/landscape/landscapemockservice/landscape_mock_service.go +++ b/mocks/landscape/landscapemockservice/landscape_mock_service.go @@ -134,15 +134,15 @@ func (s *Service) Connect(stream landscapeapi.LandscapeHostAgent_ConnectServer) } } -// recvMsg is the sanitized reuturn type of a GRPC Recv, used to pass by channel. +// recvMsg is the sanitized return type of a GRPC Recv, used to pass by channel. type recvMsg struct { info HostInfo err error } -// This goroutine is an asynchronous GRPC Recv. +// asyncRecv is an asynchronous GRPC Recv. // Usually, you cannot select between a context and a GRPC receive. This function allows you to. -// It'll keep receiving until an error is returned, or the context is cancelled. +// It will keep receiving until the context is cancelled. func asyncRecv(ctx context.Context, stream landscapeapi.LandscapeHostAgent_ConnectServer) <-chan recvMsg { ch := make(chan recvMsg) @@ -223,6 +223,8 @@ func (s *Service) IsConnected(uid string) bool { return s.isConnected(uid) } +// isConnected is the unsafe version of IsConnected. It checks if a client with the +// specified hostname has an active connection. func (s *Service) isConnected(uid string) bool { host, ok := s.hosts[uid] return ok && host.connected != nil && *host.connected From 5eac9901a805d389d08cbf04a36b3c7ac00a31c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 14 Nov 2023 10:57:05 +0100 Subject: [PATCH 08/10] Replace `help` with "help" It looks better in terminals --- mocks/landscape/landscaperepl/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mocks/landscape/landscaperepl/main.go b/mocks/landscape/landscaperepl/main.go index 47c9bf2e4..7ac326149 100644 --- a/mocks/landscape/landscaperepl/main.go +++ b/mocks/landscape/landscaperepl/main.go @@ -61,7 +61,7 @@ on your command line. Hosted at the specified address.`, fmt.Printf("Hosting on %s\n", addr) populateCommands() - fmt.Println("Write `help` to see a list of available commands") + fmt.Println(`Write "help" to see a list of available commands`) var cfg net.ListenConfig lis, err := cfg.Listen(ctx, "tcp", addr) From afd2927312884859063d2747550a772e5549a73c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 14 Nov 2023 10:40:49 +0100 Subject: [PATCH 09/10] Follow usual convention for Apps I changed the structure of the code so it more closely resembles the WSL-Pro-Service and Windows-Agent apps. The Landscape mock App matches the interface except that it is missing the Quit method. I did not implement it because I saw no reason to. This method is used only in signal handling, which we don't use in the Landscape mock. --- mocks/landscape/landscaperepl/app.go | 204 ++++++++++++++++++++++++++ mocks/landscape/landscaperepl/main.go | 183 ++--------------------- 2 files changed, 213 insertions(+), 174 deletions(-) create mode 100644 mocks/landscape/landscaperepl/app.go diff --git a/mocks/landscape/landscaperepl/app.go b/mocks/landscape/landscaperepl/app.go new file mode 100644 index 000000000..e28ffda31 --- /dev/null +++ b/mocks/landscape/landscaperepl/app.go @@ -0,0 +1,204 @@ +package main + +import ( + "bufio" + "context" + "errors" + "fmt" + "log/slog" + "net" + "os" + "path/filepath" + "strconv" + "strings" + + landscapeapi "github.com/canonical/landscape-hostagent-api" + "github.com/canonical/ubuntu-pro-for-windows/mocks/landscape/landscapemockservice" + "github.com/spf13/cobra" + "google.golang.org/grpc" +) + +// App encapsulate commands of the REPL. +type App struct { + rootCmd *cobra.Command +} + +// New registers commands and returns a new App. +func New() *App { + var a App + a.rootCmd = &cobra.Command{ + Use: executableName(), + Short: "A mock server for Landscape hostagent testing", + Long: `Landscape mock REPL mocks a Landscape hostagent server +on your command line. Hosted at the specified address.`, + PersistentPreRunE: func(cmd *cobra.Command, args []string) error { + // Force a visit of the local flags so persistent flags for all parents are merged. + cmd.LocalFlags() + + // command parsing has been successful. Returns to not print usage anymore. + cmd.SilenceUsage = true + + v := cmd.Flag("verbosity").Value.String() + n, err := strconv.Atoi(v) + if err != nil { + return fmt.Errorf("could not parse verbosity: %v", err) + } + + setVerboseMode(n) + return nil + }, + Run: func(cmd *cobra.Command, args []string) { + ctx := context.Background() + + addr := cmd.Flag("address").Value.String() + fmt.Printf("Hosting on %s\n", addr) + + populateCommands() + fmt.Println(`Write "help" to see a list of available commands`) + + var cfg net.ListenConfig + lis, err := cfg.Listen(ctx, "tcp", addr) + if err != nil { + slog.Error(fmt.Sprintf("Can't listen: %v", err)) + return + } + defer lis.Close() + + server := grpc.NewServer() + service := landscapemockservice.New() + landscapeapi.RegisterLandscapeHostAgentServer(server, service) + + go func() { + err := server.Serve(lis) + if err != nil { + slog.Error(fmt.Sprintf("Server exited with an error: %v", err)) + } + }() + defer server.Stop() + + if err := a.run(ctx, service); err != nil { + slog.Error(err.Error()) + return + } + }, + } + + a.rootCmd.PersistentFlags().CountP("verbosity", "v", "WARNING (-v) INFO (-vv), DEBUG (-vvv)") + a.rootCmd.Flags().StringP("address", "a", "localhost:8000", "Overrides the address where the server will be hosted") + + return &a +} + +// Run executes the command and associated process. It returns an error on syntax/usage error. +func (a *App) Run() error { + if a.rootCmd == nil { + return errors.New("root command was not populated") + } + + return a.rootCmd.Execute() +} + +// UsageError returns if the error is a command parsing or runtime one. +func (a *App) UsageError() bool { + return !a.rootCmd.SilenceUsage +} + +func executableName() string { + exe, err := os.Executable() + if err != nil { + return "landscaperepl" + } + return filepath.Base(exe) +} + +// setVerboseMode changes the verbosity of the logs. +func setVerboseMode(n int) { + var level slog.Level + switch n { + case 0: + level = slog.LevelError + case 1: + level = slog.LevelWarn + case 2: + level = slog.LevelInfo + default: + level = slog.LevelDebug + } + + h := slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: level}) + slog.SetDefault(slog.New(h)) +} + +// run contains the main execution loop. +func (a *App) run(ctx context.Context, s *landscapemockservice.Service) error { + sc := bufio.NewScanner(os.Stdin) + + // READ + for sc.Scan() { + line := strings.TrimSpace(sc.Text()) + + if len(line) == 0 { + continue + } + + if strings.HasPrefix(line, "#") { + continue + } + + // EXECUTE + PRINT + done := executeCommand(ctx, s, line) + if done { + break + } + + // LOOP + fmt.Println() + } + + if err := sc.Err(); err != nil { + return err + } + + return nil +} + +type wrongUsageError struct{} + +func (err wrongUsageError) Error() string { + return "wrong usage" +} + +type exitError struct{} + +func (exitError) Error() string { + return "exiting" +} + +func executeCommand(ctx context.Context, s *landscapemockservice.Service, command string) (exit bool) { + fields := strings.Fields(command) + + verb := fields[0] + args := fields[1:] + + cmd, ok := commands[verb] + if !ok { + fmt.Fprintf(os.Stderr, "unknown verb %q. use 'help' to see available commands.\n", verb) + return false + } + + err := cmd.callback(ctx, s, args...) + if errors.Is(err, exitError{}) { + return true + } + if errors.Is(err, wrongUsageError{}) { + fmt.Fprintln(os.Stderr, "Error: wrong usage:") + showHelp(os.Stderr, verb) + return false + } + if err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + return false + } + + return false +} diff --git a/mocks/landscape/landscaperepl/main.go b/mocks/landscape/landscaperepl/main.go index 7ac326149..6a5ee9834 100644 --- a/mocks/landscape/landscaperepl/main.go +++ b/mocks/landscape/landscaperepl/main.go @@ -3,190 +3,25 @@ package main import ( - "bufio" - "context" - "errors" "fmt" "log/slog" - "net" "os" - "path/filepath" - "strconv" - "strings" - - landscapeapi "github.com/canonical/landscape-hostagent-api" - "github.com/canonical/ubuntu-pro-for-windows/mocks/landscape/landscapemockservice" - "github.com/spf13/cobra" - "google.golang.org/grpc" ) func main() { - rootCmd := rootCmd() - - rootCmd.PersistentFlags().CountP("verbosity", "v", "WARNING (-v) INFO (-vv), DEBUG (-vvv)") - rootCmd.Flags().StringP("address", "a", "localhost:8000", "Overrides the address where the server will be hosted") - - if err := rootCmd.Execute(); err != nil { - slog.Error(fmt.Sprintf("Error executing: %v", err)) - os.Exit(1) - } -} - -func rootCmd() *cobra.Command { - return &cobra.Command{ - Use: executableName(), - Short: "A mock server for Landscape hostagent testing", - Long: `Landscape mock REPL mocks a Landscape hostagent server -on your command line. Hosted at the specified address.`, - PersistentPreRunE: func(cmd *cobra.Command, args []string) error { - // Force a visit of the local flags so persistent flags for all parents are merged. - cmd.LocalFlags() - - // command parsing has been successful. Returns to not print usage anymore. - cmd.SilenceUsage = true - - v := cmd.Flag("verbosity").Value.String() - n, err := strconv.Atoi(v) - if err != nil { - return fmt.Errorf("could not parse verbosity: %v", err) - } - - setVerboseMode(n) - return nil - }, - Run: func(cmd *cobra.Command, args []string) { - ctx := context.Background() - - addr := cmd.Flag("address").Value.String() - fmt.Printf("Hosting on %s\n", addr) - - populateCommands() - fmt.Println(`Write "help" to see a list of available commands`) - - var cfg net.ListenConfig - lis, err := cfg.Listen(ctx, "tcp", addr) - if err != nil { - slog.Error(fmt.Sprintf("Can't listen: %v", err)) - return - } - defer lis.Close() - - server := grpc.NewServer() - service := landscapemockservice.New() - landscapeapi.RegisterLandscapeHostAgentServer(server, service) - - go func() { - err := server.Serve(lis) - if err != nil { - slog.Error(fmt.Sprintf("Server exited with an error: %v", err)) - } - }() - defer server.Stop() - - if err := run(ctx, service); err != nil { - slog.Error(err.Error()) - return - } - }, - } -} - -func executableName() string { - exe, err := os.Executable() - if err != nil { - return "landscaperepl" - } - return filepath.Base(exe) -} - -// setVerboseMode changes the verbosity of the logs. -func setVerboseMode(n int) { - var level slog.Level - switch n { - case 0: - level = slog.LevelError - case 1: - level = slog.LevelWarn - case 2: - level = slog.LevelInfo - default: - level = slog.LevelDebug - } - - h := slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: level}) - slog.SetDefault(slog.New(h)) + app := New() + os.Exit(run(app)) } -// run contains the main execution loop. -func run(ctx context.Context, s *landscapemockservice.Service) error { - sc := bufio.NewScanner(os.Stdin) - - // READ - for sc.Scan() { - line := strings.TrimSpace(sc.Text()) +func run(a *App) int { + if err := a.Run(); err != nil { + slog.Error(fmt.Sprintf("Error: %v", err)) - if len(line) == 0 { - continue + if a.UsageError() { + return 2 } - - if strings.HasPrefix(line, "#") { - continue - } - - // EXECUTE + PRINT - done := executeCommand(ctx, s, line) - if done { - break - } - - // LOOP - fmt.Println() - } - - if err := sc.Err(); err != nil { - return err - } - - return nil -} - -type wrongUsageError struct{} - -func (err wrongUsageError) Error() string { - return "wrong usage" -} - -type exitError struct{} - -func (exitError) Error() string { - return "exiting" -} - -func executeCommand(ctx context.Context, s *landscapemockservice.Service, command string) (exit bool) { - fields := strings.Fields(command) - - verb := fields[0] - args := fields[1:] - - cmd, ok := commands[verb] - if !ok { - fmt.Fprintf(os.Stderr, "unknown verb %q. use 'help' to see available commands.\n", verb) - return false - } - - err := cmd.callback(ctx, s, args...) - if errors.Is(err, exitError{}) { - return true - } - if errors.Is(err, wrongUsageError{}) { - fmt.Fprintln(os.Stderr, "Error: wrong usage:") - showHelp(os.Stderr, verb) - return false - } - if err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - return false + return 1 } - return false + return 0 } From aa17113abc43a510abb80697fd54c319f4993679 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 14 Nov 2023 11:24:05 +0100 Subject: [PATCH 10/10] Improve error check in asyncRecv We used to check that the message was not nill, but forgot to check the error. This double-check (is err nil? + is message nil?) would cause a bit of ugly nesting so I moved it into newHostInfo so we can return instead of nest. This required a renaming of newHostInfo to receiveHostInfo to keep the naming accurate. --- .../landscape_mock_service.go | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/mocks/landscape/landscapemockservice/landscape_mock_service.go b/mocks/landscape/landscapemockservice/landscape_mock_service.go index 6d863dc9e..dbb3cca9c 100644 --- a/mocks/landscape/landscapemockservice/landscape_mock_service.go +++ b/mocks/landscape/landscapemockservice/landscape_mock_service.go @@ -4,6 +4,7 @@ package landscapemockservice import ( "context" + "errors" "fmt" "log/slog" "math/rand" @@ -30,8 +31,17 @@ type HostInfo struct { Instances []InstanceInfo } -// newHostInfo recursively copies the info in a landscapeapi.HostAgentInfo to a HostInfo. -func newHostInfo(src *landscapeapi.HostAgentInfo) HostInfo { +// receiveHostInfo receives a landscapeapi.HostAgentInfo and converts it to a HostInfo. +func receiveHostInfo(stream landscapeapi.LandscapeHostAgent_ConnectServer) (HostInfo, error) { + src, err := stream.Recv() + if err != nil { + return HostInfo{}, err + } + + if src == nil { + return HostInfo{}, errors.New("nil HostAgentInfo") + } + h := HostInfo{ UID: src.GetUid(), Hostname: src.GetHostname(), @@ -48,7 +58,7 @@ func newHostInfo(src *landscapeapi.HostAgentInfo) HostInfo { }) } - return h + return h, nil } type host struct { @@ -150,17 +160,12 @@ func asyncRecv(ctx context.Context, stream landscapeapi.LandscapeHostAgent_Conne defer close(ch) for { - var msg recvMsg - recv, err := stream.Recv() - msg.err = err - if recv != nil { - msg.info = newHostInfo(recv) - } + info, err := receiveHostInfo(stream) select { case <-ctx.Done(): return - case ch <- msg: + case ch <- recvMsg{info, err}: } } }()