finch icon indicating copy to clipboard operation
finch copied to clipboard

Cannot see finch VM status

Open shaonm opened this issue 2 years ago • 7 comments

What is the problem you're trying to solve?. I want to check the status of the VM that finch has started.

Describe the feature you'd like finch vm should have a status command to see the vm status.

Usage:
  finch vm [command]

Available Commands:
  init        Initialize the virtual machine
  remove      Remove the virtual machine instance
  start       Start the virtual machine
  stop        Stop the virtual machine

Add status command, status Status of the virtual machine

shaonm avatar Nov 22 '22 20:11 shaonm

Workaround:

$ LIMA_HOME=/Applications/Finch/lima/data /Applications/Finch/lima/bin/limactl ls
NAME     STATUS     SSH                ARCH      CPUS    MEMORY    DISK      DIR
finch    Running    127.0.0.1:56835    x86_64    2       8GiB      100GiB    /Applications/Finch/lima/data/finch

AkihiroSuda avatar Nov 22 '22 20:11 AkihiroSuda

Hey, I would like to help out with this one and started implementing it over in a fork.

One thing that I found which should be handled in a different way than with printing the output via sva.logger.Infof("%s", logs) is the output of lima's ls command. Because this way it is formatted in a wrong way as you can see below:

INFO[0000] NAME     STATUS     SSH                ARCH       CPUS    MEMORY    DISK      DIR
finch    Running    127.0.0.1:51977    aarch64    2       4GiB      100GiB    /Users/nm/Code/oss/finch/_output/lima/data/finch

Is there a custom logger available to print it in a right way? Or just use the regular fmt.Printf?

Since I'm pretty new to Golang I could also need a little help with testing the new command. So I am really open for help and advice.

niklasmtj avatar Nov 27 '22 20:11 niklasmtj

@niklasmtj Thanks! There is an existing internal method to get the lima VM status. Maybe the new command can wrap this internal method and do the proper translation.

You can add new e2e tests here and run "make test-e2e" to test it.

Assigned the issue to you and I could provide you the help if needed. Feel free to use this issue or #finch channel in CNCF server to raise the questions.

ningziwen avatar Nov 28 '22 20:11 ningziwen

Thanks for assigning me. It seems that I did not understand correctly at the beginning of this issue. Since I thought this status command should log all the information about the vm, more on this at the end.

Anyway, I have now used the internal lima.GetVMStatus method to translate the different vmStatus appropriately. As follows:

func (sva *statusVMAction) run() error {
	status, err := lima.GetVMStatus(sva.creator, sva.logger, limaInstanceName)
	if err != nil {
		return err
	}
	switch status {
	case lima.Running:
		sva.logger.Infof("the instance %q is running", limaInstanceName)
		return nil
	case lima.Nonexistent:
		return fmt.Errorf("the instance %q does not exist", limaInstanceName)
	case lima.Stopped:
		return fmt.Errorf("the instance %q is stopped. run `finch %s start` to start the instance", limaInstanceName, virtualMachineRootCmd)
	case lima.Unknown:
		return fmt.Errorf("the instance status of %q is unknown", limaInstanceName)
	default:
		return fmt.Errorf("the instance %q gave a not defined status", limaInstanceName)
	}
}

Find more context here.

Before adding (e2e) and tests in general I would like to get the method right. Does this seem like the wrapper command you though of?

In the course of this issue, I would also add the output of lima's ls in general for a subsequent feature to get the information about the vm such as CPU, memory etc. mentioned in the output in my previous comment.

niklasmtj avatar Nov 29 '22 20:11 niklasmtj

My understanding is that VM status (running/stopped/nonexistent) is the major request of this issue. We could add more information like CPU/memory later. @shaonm can correct me if I'm wrong.

About the translation, I think the command "finch vm status", could just objectively return whatever the status is as "Running"/"Stopped"/"Nonexistent", instead of returning Error when the vm is not running. Returning Error when the vm is not running implies it has assumption that the vm should be running, which is not an assumption for the "finch vm status".

ningziwen avatar Nov 30 '22 17:11 ningziwen

Yeah you're right about the assumptions. This shouldn't be the case when one just want to check if it's available or not. Will update it with only the logging info. Thanks for the feedback!

niklasmtj avatar Dec 01 '22 14:12 niklasmtj

About "logging info", I feel the status should be printed in stdout instead of stderr as it is the actual result instead of diagnostic information. (Difference between them)

Logger prints to stderr by default and normally prints the diagnostic information.

We can potentially use the Stdout() in stdlib.go. There are multiple ways of using it. We can discuss the details in PR.

ningziwen avatar Dec 01 '22 20:12 ningziwen