Skip to content

Commit

Permalink
shared/cmd: Allow a cmd asker to be created with a logger instance (#…
Browse files Browse the repository at this point in the history
…13859)

If a command asker is with a logger, the question and answer (even if it
is invalid) are systematically logged to the logger destination. This
will greatly help debugability in integration test suites like in
MicroCloud which have a lot of interactions in the command line.
  • Loading branch information
tomponline authored Aug 27, 2024
2 parents e4715fe + 128f9de commit a894583
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 23 deletions.
21 changes: 13 additions & 8 deletions lxc/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package main

import (
"bufio"
"errors"
"fmt"
"os"
"os/user"
Expand Down Expand Up @@ -89,7 +90,7 @@ For help with any of those, simply call them with --help.`))
app.CompletionOptions = cobra.CompletionOptions{DisableDefaultCmd: true}

// Global flags
globalCmd := cmdGlobal{cmd: app, asker: cli.NewAsker(bufio.NewReader(os.Stdin))}
globalCmd := cmdGlobal{cmd: app, asker: cli.NewAsker(bufio.NewReader(os.Stdin), nil)}
app.PersistentFlags().BoolVar(&globalCmd.flagVersion, "version", false, i18n.G("Print version number"))
app.PersistentFlags().BoolVarP(&globalCmd.flagHelp, "help", "h", false, i18n.G("Print help"))
app.PersistentFlags().BoolVar(&globalCmd.flagForceLocal, "force-local", false, i18n.G("Force using the local unix socket"))
Expand Down Expand Up @@ -290,11 +291,12 @@ For help with any of those, simply call them with --help.`))
if err != nil {
// Handle non-Linux systems
if err == config.ErrNotLinux {
fmt.Fprintf(os.Stderr, i18n.G(`This client hasn't been configured to use a remote LXD server yet.
msg := i18n.G(`This client hasn't been configured to use a remote LXD server yet.
As your platform can't run native Linux instances, you must connect to a remote LXD server.
If you already added a remote server, make it the default with "lxc remote switch NAME".
To easily setup a local LXD server in a virtual machine, consider using: https://multipass.run`)+"\n")
To easily setup a local LXD server in a virtual machine, consider using: https://multipass.run`)
fmt.Fprintln(os.Stderr, msg)
os.Exit(1)
}

Expand Down Expand Up @@ -358,7 +360,7 @@ func (c *cmdGlobal) PreRun(cmd *cobra.Command, args []string) error {

// Setup password helper
c.conf.PromptPassword = func(filename string) (string, error) {
return cli.AskPasswordOnce(fmt.Sprintf(i18n.G("Password for %s: "), filename)), nil
return c.asker.AskPasswordOnce(fmt.Sprintf(i18n.G("Password for %s: "), filename)), nil
}

// If the user is running a command that may attempt to connect to the local daemon
Expand Down Expand Up @@ -395,13 +397,15 @@ func (c *cmdGlobal) PreRun(cmd *cobra.Command, args []string) error {

flush := false
if runInit {
fmt.Fprintf(os.Stderr, i18n.G("If this is your first time running LXD on this machine, you should also run: lxd init")+"\n")
msg := i18n.G("If this is your first time running LXD on this machine, you should also run: lxd init")
fmt.Fprintln(os.Stderr, msg)
flush = true
}

if !shared.ValueInSlice(cmd.Name(), []string{"init", "launch"}) {
fmt.Fprintf(os.Stderr, i18n.G(`To start your first container, try: lxc launch ubuntu:24.04
Or for a virtual machine: lxc launch ubuntu:24.04 --vm`)+"\n")
msg := i18n.G(`To start your first container, try: lxc launch ubuntu:24.04
Or for a virtual machine: lxc launch ubuntu:24.04 --vm`)
fmt.Fprintln(os.Stderr, msg)
flush = true
}

Expand Down Expand Up @@ -495,7 +499,8 @@ func (c *cmdGlobal) CheckArgs(cmd *cobra.Command, args []string, minArgs int, ma
return true, nil
}

return true, fmt.Errorf(i18n.G("Invalid number of arguments"))
msg := i18n.G("Invalid number of arguments")
return true, errors.New(msg)
}

return false, nil
Expand Down
2 changes: 1 addition & 1 deletion lxd-migrate/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func main() {
app.Args = cobra.ArbitraryArgs

// Global flags
globalCmd := cmdGlobal{asker: cli.NewAsker(bufio.NewReader(os.Stdin))}
globalCmd := cmdGlobal{asker: cli.NewAsker(bufio.NewReader(os.Stdin), nil)}
migrateCmd.global = &globalCmd
app.PersistentFlags().BoolVar(&globalCmd.flagVersion, "version", false, "Print version number")
app.PersistentFlags().BoolVarP(&globalCmd.flagHelp, "help", "h", false, "Print help")
Expand Down
3 changes: 2 additions & 1 deletion lxd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ type cmdGlobal struct {
flagLogVerbose bool
}

// Run is the main entry point for the LXD daemon command.
func (c *cmdGlobal) Run(cmd *cobra.Command, args []string) error {
// Configure dqlite to *not* disable internal SQLite locking, since we
// use SQLite both through dqlite and through go-dqlite, potentially
Expand Down Expand Up @@ -91,7 +92,7 @@ func main() {
app.Args = cobra.ArbitraryArgs

// Global flags
globalCmd := cmdGlobal{cmd: app, asker: cli.NewAsker(bufio.NewReader(os.Stdin))}
globalCmd := cmdGlobal{cmd: app, asker: cli.NewAsker(bufio.NewReader(os.Stdin), nil)}
daemonCmd.global = &globalCmd
app.PersistentPreRunE = globalCmd.Run
app.PersistentFlags().BoolVar(&globalCmd.flagVersion, "version", false, "Print version number")
Expand Down
68 changes: 55 additions & 13 deletions shared/cmd/ask.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,30 @@ import (
"golang.org/x/term"

"github.com/canonical/lxd/shared"
"github.com/canonical/lxd/shared/logger"
)

// Asker holds a reader for reading input into CLI questions.
type Asker struct {
reader *bufio.Reader
logger logger.Logger
}

func NewAsker(reader *bufio.Reader) Asker {
return Asker{reader: reader}
// NewAsker creates a new Asker instance that reads from the given reader.
// It can also be configured with a logger to help during the debug process.
func NewAsker(reader *bufio.Reader, logger logger.Logger) Asker {
return Asker{reader: reader, logger: logger}
}

// AskBool asks a question and expect a yes/no answer.
func (a *Asker) AskBool(question string, defaultAnswer string) (bool, error) {
for {
answer, err := a.askQuestion(question, defaultAnswer)
if err != nil {
if a.logger != nil {
a.logger.Error("Failed to read answer for question", logger.Ctx{"answer": answer, "question": question, "err": err})
}

return false, err
}

Expand All @@ -35,7 +43,7 @@ func (a *Asker) AskBool(question string, defaultAnswer string) (bool, error) {
return false, nil
}

invalidInput()
a.invalidInput(question, answer)
}
}

Expand All @@ -44,14 +52,20 @@ func (a *Asker) AskChoice(question string, choices []string, defaultAnswer strin
for {
answer, err := a.askQuestion(question, defaultAnswer)
if err != nil {
if a.logger != nil {
a.logger.Error("Failed to read answer for question", logger.Ctx{"answer": answer, "question": question, "err": err})
}

return "", err
}

if shared.ValueInSlice(answer, choices) {
return answer, nil
} else if a.logger != nil {
a.logger.Error("Answer not among the available choices", logger.Ctx{"answer": answer, "choices": choices})
}

invalidInput()
a.invalidInput(question, answer)
}
}

Expand All @@ -60,23 +74,39 @@ func (a *Asker) AskInt(question string, min int64, max int64, defaultAnswer stri
for {
answer, err := a.askQuestion(question, defaultAnswer)
if err != nil {
if a.logger != nil {
a.logger.Error("Failed to read answer for question", logger.Ctx{"answer": answer, "question": question, "err": err})
}

return -1, err
}

result, err := strconv.ParseInt(answer, 10, 64)
if err != nil {
if a.logger != nil {
a.logger.Error("Invalid input for the question", logger.Ctx{"answer": answer, "question": question, "err": err})
}

fmt.Fprintf(os.Stderr, "Invalid input: %v\n\n", err)
continue
}

if !((min == -1 || result >= min) && (max == -1 || result <= max)) {
if a.logger != nil {
a.logger.Error("Invalid input (out of range) for the question", logger.Ctx{"answer": answer, "question": question})
}

fmt.Fprintf(os.Stderr, "Invalid input: out of range\n\n")
continue
}

if validate != nil {
err = validate(result)
if err != nil {
if a.logger != nil {
a.logger.Error("Invalid input for the question", logger.Ctx{"answer": answer, "question": question, "err": err})
}

fmt.Fprintf(os.Stderr, "Invalid input: %v\n\n", err)
continue
}
Expand All @@ -92,13 +122,21 @@ func (a *Asker) AskString(question string, defaultAnswer string, validate func(s
for {
answer, err := a.askQuestion(question, defaultAnswer)
if err != nil {
if a.logger != nil {
a.logger.Error("Failed to read answer for question", logger.Ctx{"answer": answer, "question": question, "err": err})
}

return "", err
}

if validate != nil {
error := validate(answer)
if error != nil {
fmt.Fprintf(os.Stderr, "Invalid input: %s\n\n", error)
err = validate(answer)
if err != nil {
if a.logger != nil {
a.logger.Error("Invalid input for the question", logger.Ctx{"answer": answer, "question": question, "err": err})
}

fmt.Fprintf(os.Stderr, "Invalid input: %v\n\n", err)
continue
}

Expand All @@ -109,12 +147,12 @@ func (a *Asker) AskString(question string, defaultAnswer string, validate func(s
return answer, err
}

invalidInput()
a.invalidInput(question, answer)
}
}

// AskPassword asks the user to enter a password.
func AskPassword(question string) string {
func (a *Asker) AskPassword(question string) string {
for {
fmt.Print(question)

Expand All @@ -134,14 +172,14 @@ func AskPassword(question string) string {
return inFirst
}

invalidInput()
a.invalidInput(question, "*****")
}
}

// AskPasswordOnce asks the user to enter a password.
//
// It's the same as AskPassword, but it won't ask to enter it again.
func AskPasswordOnce(question string) string {
func (a *Asker) AskPasswordOnce(question string) string {
for {
fmt.Print(question)
pwd, _ := term.ReadPassword(0)
Expand All @@ -153,7 +191,7 @@ func AskPasswordOnce(question string) string {
return spwd
}

invalidInput()
a.invalidInput(question, "*****")
}
}

Expand All @@ -176,6 +214,10 @@ func (a *Asker) readAnswer(defaultAnswer string) (string, error) {
}

// Print an invalid input message on the error stream.
func invalidInput() {
func (a *Asker) invalidInput(question string, answer string) {
if a.logger != nil {
a.logger.Error("Invalid input for the question", logger.Ctx{"answer": answer, "question": question})
}

fmt.Fprintf(os.Stderr, "Invalid input, try again.\n\n")
}

0 comments on commit a894583

Please sign in to comment.