pkg/store: refactor YAML loader

Signed-off-by: Akihiro Suda <[email protected]>

Author: AkihiroSuda

cmd/limactl/shell.go

@@ -57,7 +57,7 @@ func shellAction(clicontext *cli.Context) error {
 	if inst.Status == store.StatusStopped {
 		return errors.Errorf("instance %q is stopped, run `limactl start %s` to start the instance", instName, instName)
 	}
-	y, _, err := store.LoadYAMLByInstanceName(instName)
+	y, err := inst.LoadYAML()
 	if err != nil {
 		return err
 	}

cmd/limactl/start.go

@@ -34,86 +34,88 @@ var startCommand = &cli.Command{
 	BashComplete: startBashComplete,
 }
 
-func loadOrCreateYAML(clicontext *cli.Context) (y *limayaml.LimaYAML, instDir string, err error) {
+func loadOrCreateInstance(clicontext *cli.Context) (*store.Instance, error) {
 	if clicontext.NArg() > 1 {
-		return nil, "", errors.Errorf("too many arguments")
+		return nil, errors.Errorf("too many arguments")
 	}
 
 	arg := clicontext.Args().First()
 	if arg == "" {
 		arg = DefaultInstanceName
 	}
 
-	yBytes := limayaml.DefaultTemplate
-	var instName string
+	var (
+		instName string
+		yBytes   = limayaml.DefaultTemplate
+		err      error
+	)
 
 	if argSeemsYAMLPath(arg) {
 		instName = instNameFromYAMLPath(arg)
 		logrus.Debugf("interpreting argument %q as a file path for instance %q", arg, instName)
 		yBytes, err = os.ReadFile(arg)
 		if err != nil {
-			return nil, "", err
+			return nil, err
 		}
 	} else {
 		instName = arg
 		logrus.Debugf("interpreting argument %q as an instance name %q", arg, instName)
-		if err = identifiers.Validate(instName); err != nil {
-			return nil, "", errors.Wrapf(err, "argument must be either an instance name or a YAML file path, got %q", instName)
+		if err := identifiers.Validate(instName); err != nil {
+			return nil, errors.Wrapf(err, "argument must be either an instance name or a YAML file path, got %q", instName)
 		}
-		y, instDir, err = store.LoadYAMLByInstanceName(instName)
-		if err == nil {
+		if inst, err := store.Inspect(instName); err == nil {
 			logrus.Infof("Using the existing instance %q", instName)
-			return y, instDir, nil
+			return inst, nil
 		} else {
 			if !errors.Is(err, os.ErrNotExist) {
-				return nil, "", err
+				return nil, err
 			}
 		}
 	}
 	// create a new instance from the template
-	instDir, err = store.InstanceDir(instName)
+	instDir, err := store.InstanceDir(instName)
 	if err != nil {
-		return nil, "", err
+		return nil, err
 	}
 
 	if _, err := os.Stat(instDir); !errors.Is(err, os.ErrNotExist) {
-		return nil, "", errors.Errorf("instance %q already exists (%q)", instName, instDir)
+		return nil, errors.Errorf("instance %q already exists (%q)", instName, instDir)
 	}
 
 	if clicontext.Bool("tty") {
 		yBytes, err = openEditor(clicontext, instName, yBytes)
 		if err != nil {
-			return nil, "", err
+			return nil, err
 		}
 		if len(yBytes) == 0 {
 			logrus.Info("Aborting, as requested by saving the file with empty content")
 			os.Exit(0)
-			return nil, "", errors.New("should not reach here")
+			return nil, errors.New("should not reach here")
 		}
 	} else {
 		logrus.Info("Terminal is not available, proceeding without opening an editor")
 	}
-	y, err = limayaml.Load(yBytes)
+	y, err := limayaml.Load(yBytes)
 	if err != nil {
-		return nil, "", err
+		return nil, err
 	}
 	if err := limayaml.Validate(*y); err != nil {
 		if !clicontext.Bool("tty") {
-			return nil, "", err
+			return nil, err
 		}
 		rejectedYAML := "lima.REJECTED.yaml"
 		if writeErr := os.WriteFile(rejectedYAML, yBytes, 0644); writeErr != nil {
-			return nil, "", errors.Wrapf(err, "the YAML is invalid, attempted to save the buffer as %q but failed: %v", rejectedYAML, writeErr)
+			return nil, errors.Wrapf(err, "the YAML is invalid, attempted to save the buffer as %q but failed: %v", rejectedYAML, writeErr)
 		}
-		return nil, "", errors.Wrapf(err, "the YAML is invalid, saved the buffer as %q", rejectedYAML)
+		return nil, errors.Wrapf(err, "the YAML is invalid, saved the buffer as %q", rejectedYAML)
 	}
 	if err := os.MkdirAll(instDir, 0700); err != nil {
-		return nil, "", err
+		return nil, err
 	}
 	if err := os.WriteFile(filepath.Join(instDir, store.YAMLFileName), yBytes, 0644); err != nil {
-		return nil, "", err
+		return nil, err
 	}
-	return y, instDir, nil
+	return store.Inspect(instName)
 }
 
 // openEditor opens an editor, and returns the content (not path) of the modified yaml.
@@ -164,16 +166,12 @@ func openEditor(clicontext *cli.Context, name string, initialContent []byte) ([]
 }
 
 func startAction(clicontext *cli.Context) error {
-	y, instDir, err := loadOrCreateYAML(clicontext)
-	if err != nil {
-		return err
-	}
-	instName, err := store.InstanceNameFromInstanceDir(instDir)
+	inst, err := loadOrCreateInstance(clicontext)
 	if err != nil {
 		return err
 	}
 	ctx := clicontext.Context
-	return start.Start(ctx, instName, instDir, y)
+	return start.Start(ctx, inst)
 }
 
 func argSeemsYAMLPath(arg string) bool {

pkg/hostagent/hostagent.go

@@ -56,22 +56,27 @@ func New(instName string, stdout, stderr io.Writer, sigintCh chan os.Signal) (*H
 		Level:     logrus.DebugLevel,
 	}
 
-	y, instDir, err := store.LoadYAMLByInstanceName(instName)
+	inst, err := store.Inspect(instName)
+	if err != nil {
+		return nil, err
+	}
+
+	y, err := inst.LoadYAML()
 	if err != nil {
 		return nil, err
 	}
 
 	qCfg := qemu.Config{
 		Name:        instName,
-		InstanceDir: instDir,
+		InstanceDir: inst.Dir,
 		LimaYAML:    y,
 	}
 	qExe, qArgs, err := qemu.Cmdline(qCfg)
 	if err != nil {
 		return nil, err
 	}
 
-	sshArgs, err := sshutil.SSHArgs(instDir)
+	sshArgs, err := sshutil.SSHArgs(inst.Dir)
 	if err != nil {
 		return nil, err
 	}
@@ -82,7 +87,7 @@ func New(instName string, stdout, stderr io.Writer, sigintCh chan os.Signal) (*H
 	a := &HostAgent{
 		l:             l,
 		y:             y,
-		instDir:       instDir,
+		instDir:       inst.Dir,
 		sshConfig:     sshConfig,
 		portForwarder: newPortForwarder(l, sshConfig, y.SSH.LocalPort),
 		qExe:          qExe,

pkg/start/start.go

@@ -12,6 +12,7 @@ import (
 	hostagentapi "github.com/AkihiroSuda/lima/pkg/hostagent/api"
 	"github.com/AkihiroSuda/lima/pkg/limayaml"
 	"github.com/AkihiroSuda/lima/pkg/qemu"
+	"github.com/AkihiroSuda/lima/pkg/store"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -33,22 +34,27 @@ func ensureDisk(ctx context.Context, instName, instDir string, y *limayaml.LimaY
 	return nil
 }
 
-func Start(ctx context.Context, instName, instDir string, y *limayaml.LimaYAML) error {
-	haPIDPath := filepath.Join(instDir, "ha.pid")
+func Start(ctx context.Context, inst *store.Instance) error {
+	haPIDPath := filepath.Join(inst.Dir, "ha.pid")
 	if _, err := os.Stat(haPIDPath); !errors.Is(err, os.ErrNotExist) {
-		return errors.Errorf("instance %q seems running (hint: remove %q if the instance is not actually running)", instName, haPIDPath)
+		return errors.Errorf("instance %q seems running (hint: remove %q if the instance is not actually running)", inst.Name, haPIDPath)
 	}
 
-	if err := ensureDisk(ctx, instName, instDir, y); err != nil {
+	y, err := inst.LoadYAML()
+	if err != nil {
+		return err
+	}
+
+	if err := ensureDisk(ctx, inst.Name, inst.Dir, y); err != nil {
 		return err
 	}
 
 	self, err := os.Executable()
 	if err != nil {
 		return err
 	}
-	haStdoutPath := filepath.Join(instDir, "ha.stdout.log")
-	haStderrPath := filepath.Join(instDir, "ha.stderr.log")
+	haStdoutPath := filepath.Join(inst.Dir, "ha.stdout.log")
+	haStderrPath := filepath.Join(inst.Dir, "ha.stderr.log")
 	if err := os.RemoveAll(haStdoutPath); err != nil {
 		return err
 	}
@@ -69,7 +75,7 @@ func Start(ctx context.Context, instName, instDir string, y *limayaml.LimaYAML)
 	haCmd := exec.CommandContext(ctx, self,
 		"hostagent",
 		"--pidfile", haPIDPath,
-		instName)
+		inst.Name)
 	haCmd.Stdout = haStdoutW
 	haCmd.Stderr = haStderrW
 
@@ -81,7 +87,7 @@ func Start(ctx context.Context, instName, instDir string, y *limayaml.LimaYAML)
 		return err
 	}
 
-	return watchHostAgentEvents(ctx, instName, haStdoutPath, haStderrPath)
+	return watchHostAgentEvents(ctx, inst.Name, haStdoutPath, haStderrPath)
 	// leave the hostagent process running
 }
 

pkg/store/instance.go

@@ -30,14 +30,28 @@ type Instance struct {
 	Errors       []error       `json:"errors,omitempty"`
 }
 
-// Inspect returns err only when the instance does not exist.
+func (inst *Instance) LoadYAML() (*limayaml.LimaYAML, error) {
+	if inst.Dir == "" {
+		return nil, errors.New("inst.Dir is empty")
+	}
+	yamlPath := filepath.Join(inst.Dir, YAMLFileName)
+	return LoadYAMLByFilePath(yamlPath)
+}
+
+// Inspect returns err only when the instance does not exist (os.ErrNotExist).
 // Other errors are returned as *Instance.Errors
 func Inspect(instName string) (*Instance, error) {
 	inst := &Instance{
 		Name:   instName,
 		Status: StatusUnknown,
 	}
-	y, instDir, err := LoadYAMLByInstanceName(instName)
+	// InstanceDir validates the instName but does not check whether the instance exists
+	instDir, err := InstanceDir(instName)
+	if err != nil {
+		return nil, err
+	}
+	yamlPath := filepath.Join(instDir, YAMLFileName)
+	y, err := LoadYAMLByFilePath(yamlPath)
 	if err != nil {
 		if errors.Is(err, os.ErrNotExist) {
 			return nil, err

pkg/store/store.go

@@ -64,14 +64,6 @@ func InstanceDir(name string) (string, error) {
 	return dir, nil
 }
 
-// InstanceNameFromInstanceDir extracts the instance name
-// from the path of the instance directory.
-// e.g. "foo" for "/Users/somebody/.lima/foo".
-func InstanceNameFromInstanceDir(s string) (string, error) {
-	base := filepath.Base(s)
-	return base, identifiers.Validate(base)
-}
-
 // LoadYAMLByFilePath loads and validates the yaml.
 func LoadYAMLByFilePath(filePath string) (*limayaml.LimaYAML, error) {
 	if _, err := os.Stat(filePath); err != nil {
@@ -90,28 +82,3 @@ func LoadYAMLByFilePath(filePath string) (*limayaml.LimaYAML, error) {
 	}
 	return y, nil
 }
-
-// YAMLFilePathByInstanceName returns the yaml file path but does not
-// check whether it exists.
-func YAMLFilePathByInstanceName(instName string) (string, string, error) {
-	instDir, err := InstanceDir(instName)
-	if err != nil {
-		return "", instDir, err
-	}
-	instYAMLPath := filepath.Join(instDir, YAMLFileName)
-	return instYAMLPath, instDir, nil
-}
-
-// LoadYAMLByInstanceName loads and validates the yaml.
-// LoadYAMLByInstanceName may return os.ErrNotExist
-func LoadYAMLByInstanceName(instName string) (*limayaml.LimaYAML, string, error) {
-	instYAMLPath, instDir, err := YAMLFilePathByInstanceName(instName)
-	if err != nil {
-		return nil, "", err
-	}
-	y, err := LoadYAMLByFilePath(instYAMLPath)
-	if err != nil {
-		return nil, "", err
-	}
-	return y, instDir, nil
-}