Skip to content

limactl shell: ask whether to start the instance if not running #2763

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

limactl shell: ask whether to start the instance if not running #2763

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
35 changes: 5 additions & 30 deletions cmd/limactl/edit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,9 @@ import (
"github.com/lima-vm/lima/cmd/limactl/editflags"
"github.com/lima-vm/lima/cmd/limactl/guessarg"
"github.com/lima-vm/lima/pkg/editutil"
"github.com/lima-vm/lima/pkg/instance"
"github.com/lima-vm/lima/pkg/limayaml"
networks "github.com/lima-vm/lima/pkg/networks/reconcile"
"github.com/lima-vm/lima/pkg/store"
"github.com/lima-vm/lima/pkg/store/filenames"
"github.com/lima-vm/lima/pkg/uiutil"
"github.com/lima-vm/lima/pkg/yqutil"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -132,34 +129,12 @@ func editAction(cmd *cobra.Command, args []string) error {
}
if inst != nil {
logrus.Infof("Instance %q configuration edited", inst.Name)
if _, err = startInteractive(cmd, inst.Name, false, false); err != nil {
return err
}
}

if !tty {
// use "start" to start it
return nil
}
if inst == nil {
// edited a limayaml file directly
return nil
}
startNow, err := askWhetherToStart()
if err != nil {
return err
}
if !startNow {
return nil
}
ctx := cmd.Context()
err = networks.Reconcile(ctx, inst.Name)
if err != nil {
return err
}
return instance.Start(ctx, inst, "", false)
}

func askWhetherToStart() (bool, error) {
message := "Do you want to start the instance now? "
return uiutil.Confirm(message, true)
// inst is nil if edited a limayaml file directly
return nil
}

func editBashComplete(cmd *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) {
Expand Down
19 changes: 16 additions & 3 deletions cmd/limactl/shell.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ func newShellCommand() *cobra.Command {
}

func shellAction(cmd *cobra.Command, args []string) error {
os.Setenv("_LIMACTL_SHELL_IN_ACTION", "")
// simulate the behavior of double dash
newArg := []string{}
if len(args) >= 2 && args[1] == "--" {
Expand All @@ -71,12 +72,24 @@ func shellAction(cmd *cobra.Command, args []string) error {
inst, err := store.Inspect(instName)
if err != nil {
if errors.Is(err, os.ErrNotExist) {
return fmt.Errorf("instance %q does not exist, run `limactl create %s` to create a new instance", instName, instName)
_, err = startInteractive(cmd, instName, true, true)
if err != nil {
return err
}
inst, err = store.Inspect(instName)
}
if err != nil {
return err
}
return err
}
if inst.Status == store.StatusStopped {
return fmt.Errorf("instance %q is stopped, run `limactl start %s` to start the instance", instName, instName)
if _, err = startInteractive(cmd, instName, false, true); err != nil {
return err
}
inst, err = store.Inspect(instName)
if err != nil {
return err
}
}

// When workDir is explicitly set, the shell MUST have workDir as the cwd, or exit with an error.
Expand Down
52 changes: 52 additions & 0 deletions cmd/limactl/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -529,3 +529,55 @@ func startBashComplete(cmd *cobra.Command, _ []string, _ string) ([]string, cobr
compTmpl, _ := bashCompleteTemplateNames(cmd)
return append(compInst, compTmpl...), cobra.ShellCompDirectiveDefault
}

// startInteractive starts the instance after a confirmation prompt.
//
// If newInst is set to true, this function creates the instance too.
//
// If mandatory is set to true, and the answer to the confirmation prompt is No,
// the function returns an error that contains an instruction to start the instance manually.
func startInteractive(cmd *cobra.Command, instName string, newInst, mandatory bool) (bool, error) {
Copy link
Member

Choose a reason for hiding this comment

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

It is not clear what the bool return value represents. It also seems to be completely unused; all 3 call-sites are _, err = startInteractive(…).

flags := cmd.Flags()
tty, err := flags.GetBool("tty")
if err != nil {
return false, err
}
errS := fmt.Sprintf("instance %q is stopped, run `limactl start %s` to start the instance", instName, instName)
if newInst {
errS = fmt.Sprintf("instance %q does not exist, run `limactl create %s` to create a new instance", instName, instName)
}
if !tty {
if mandatory {
return false, errors.New(errS)
}
return false, nil
}
logrus.Error(errS)

message := fmt.Sprintf("Do you want to start the instance %q now?", instName)
if newInst {
message = fmt.Sprintf("Do you want to create and start the instance %q using the default template now?", instName)
}
ans, err := uiutil.Confirm(message, true)
if err != nil {
return false, err
}
if !ans {
if mandatory {
return ans, errors.New(errS)
}
return ans, nil
}
if newInst {
rootCmd := cmd.Root()
Copy link
Member

Choose a reason for hiding this comment

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

rootCmd could be setup outside the conditional because it is needed again later.

// The create command shows the template chooser UI, etc.
Copy link
Member

Choose a reason for hiding this comment

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

I would have expected that the template would default to the instance name, if a template existed with that name. Otherwise it should show an error and only then default to default.

$ l shell alpine
ERRO[0000] instance "alpine" does not exist, run `limactl create alpine` to create a new instance
? Do you want to create and start the instance "alpine" using the default template now?

This should say "using the alpine template".

Copy link
Member

Choose a reason for hiding this comment

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

I agree that the mandatory mechanism is too complex/confusing. It also results in the same error being displayed twice, first as ERRO and then as FATA:

$ l shell alpine
ERRO[0000] instance "alpine" does not exist, run `limactl create alpine` to create a new instance
? Do you want to create and start the instance "alpine" using the default template now? No
FATA[0002] instance "alpine" does not exist, run `limactl create alpine` to create a new instance

I think the mandatory argument can just be removed, and the function always just returns nil when the user elected not to create/start the instance.

The caller is already calling store.Inspect() afterwards, and can then simply exit if the instance isn't in running state.

rootCmd.SetArgs([]string{"create", instName})
if err := rootCmd.Execute(); err != nil {
return ans, err
}
}
rootCmd := cmd.Root()
// The start command reconciliates the networks, etc.
rootCmd.SetArgs([]string{"start", instName})
return ans, rootCmd.Execute()
}
13 changes: 10 additions & 3 deletions pkg/instance/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,10 +307,17 @@ func watchHostAgentEvents(ctx context.Context, inst *store.Instance, haStdoutPat
err = xerr
return true
}
if *inst.Config.Plain {
logrus.Infof("READY. Run `ssh -F %q %s` to open the shell.", inst.SSHConfigFile, inst.Hostname)
// _LIMACTL_SHELL_IN_ACTION is set if `limactl shell` invoked `limactl start`.
// In this case we shouldn't print "Run `lima` to open the shell",
// because the user has already executed the `lima` command.
if _, limactlShellInAction := os.LookupEnv("_LIMACTL_SHELL_IN_ACTION"); limactlShellInAction {
logrus.Infof("READY.")
} else {
logrus.Infof("READY. Run `%s` to open the shell.", LimactlShellCmd(inst.Name))
if *inst.Config.Plain {
logrus.Infof("READY. Run `ssh -F %q %s` to open the shell.", inst.SSHConfigFile, inst.Hostname)
} else {
logrus.Infof("READY. Run `%s` to open the shell.", LimactlShellCmd(inst.Name))
}
}
_ = ShowMessage(inst)
err = nil
Expand Down
Loading