Usability improvements to incus-migrate

cmd/incus-migrate/main.go

@@ -20,7 +20,7 @@ type cmdGlobal struct {
 func main() {
 	// migrate command (main)
 	migrateCmd := cmdMigrate{}
-	app := migrateCmd.Command()
+	app := migrateCmd.command()
 	app.SilenceUsage = true
 	app.CompletionOptions = cobra.CompletionOptions{DisableDefaultCmd: true}
 
@@ -39,7 +39,7 @@ func main() {
 
 	// netcat sub-command
 	netcatCmd := cmdNetcat{global: &globalCmd}
-	app.AddCommand(netcatCmd.Command())
+	app.AddCommand(netcatCmd.command())
 
 	// Run the main command and handle errors
 	err := app.Execute()

cmd/incus-migrate/main_migrate.go

@@ -37,7 +37,7 @@ type cmdMigrate struct {
 	flagRsyncArgs string
 }
 
-func (c *cmdMigrate) Command() *cobra.Command {
+func (c *cmdMigrate) command() *cobra.Command {
 	cmd := &cobra.Command{}
 	cmd.Use = "incus-migrate"
 	cmd.Short = "Physical to instance migration tool"
@@ -53,8 +53,8 @@ func (c *cmdMigrate) Command() *cobra.Command {
 
   The same set of options as ` + "`incus launch`" + ` are also supported.
 `
-	cmd.RunE = c.Run
-	cmd.Flags().StringVar(&c.flagRsyncArgs, "rsync-args", "", "Extra arguments to pass to rsync"+"``")
+	cmd.RunE = c.run
+	cmd.Flags().StringVar(&c.flagRsyncArgs, "rsync-args", "", "Extra arguments to pass to rsync (for file transfers)"+"``")
 
 	return cmd
 }
@@ -67,7 +67,7 @@ type cmdMigrateData struct {
 	Project      string
 }
 
-func (c *cmdMigrateData) Render() string {
+func (c *cmdMigrateData) render() string {
 	data := struct {
 		Name         string            `yaml:"Name"`
 		Project      string            `yaml:"Project"`
@@ -222,7 +222,8 @@ func (c *cmdMigrate) askServer() (incus.InstanceServer, string, error) {
 	var keyPath string
 	var token string
 
-	if authMethod == authMethodTLSCertificate {
+	switch authMethod {
+	case authMethodTLSCertificate:
 		certPath, err = c.global.asker.AskString("Please provide the certificate path: ", "", func(path string) error {
 			if !util.PathExists(path) {
 				return errors.New("File does not exist")
@@ -244,7 +245,8 @@ func (c *cmdMigrate) askServer() (incus.InstanceServer, string, error) {
 		if err != nil {
 			return nil, "", err
 		}
-	} else if authMethod == authMethodTLSCertificateToken {
+
+	case authMethodTLSCertificateToken:
 		token, err = c.global.asker.AskString("Please provide the certificate token: ", "", func(token string) error {
 			_, err := localtls.CertificateTokenDecode(token)
 			if err != nil {
@@ -268,7 +270,7 @@ func (c *cmdMigrate) askServer() (incus.InstanceServer, string, error) {
 	return c.connectTarget(serverURL, certPath, keyPath, authType, token)
 }
 
-func (c *cmdMigrate) RunInteractive(server incus.InstanceServer) (cmdMigrateData, error) {
+func (c *cmdMigrate) runInteractive(server incus.InstanceServer) (cmdMigrateData, error) {
 	var err error
 
 	config := cmdMigrateData{}
@@ -284,14 +286,16 @@ func (c *cmdMigrate) RunInteractive(server incus.InstanceServer) (cmdMigrateData
 	config.InstanceArgs.Devices = map[string]map[string]string{}
 
 	// Provide instance type
-	instanceType, err := c.global.asker.AskInt("Would you like to create a container (1) or virtual-machine (2)?: ", 1, 2, "1", nil)
+	instanceType, err := c.global.asker.AskInt("Would you like to create a container (1) or virtual-machine (2)?: ", 1, 2, "", nil)
 	if err != nil {
 		return cmdMigrateData{}, err
 	}
 
-	if instanceType == 1 {
+	switch instanceType {
+	case 1:
 		config.InstanceArgs.Type = api.InstanceTypeContainer
-	} else if instanceType == 2 {
+
+	case 2:
 		config.InstanceArgs.Type = api.InstanceTypeVM
 	}
 
@@ -439,7 +443,7 @@ func (c *cmdMigrate) RunInteractive(server incus.InstanceServer) (cmdMigrateData
 	for {
 		fmt.Println("\nInstance to be created:")
 
-		scanner := bufio.NewScanner(strings.NewReader(config.Render()))
+		scanner := bufio.NewScanner(strings.NewReader(config.render()))
 		for scanner.Scan() {
 			fmt.Printf("  %s\n", scanner.Text())
 		}
@@ -478,15 +482,15 @@ Additional overrides can be applied at this stage:
 	}
 }
 
-func (c *cmdMigrate) Run(cmd *cobra.Command, args []string) error {
+func (c *cmdMigrate) run(cmd *cobra.Command, args []string) error {
 	// Quick checks.
 	if os.Geteuid() != 0 {
-		return fmt.Errorf("This tool must be run as root")
+		return errors.New("This tool must be run as root")
 	}
 
 	_, err := exec.LookPath("rsync")
 	if err != nil {
-		return err
+		return errors.New("Unable to find required command \"rsync\"")
 	}
 
 	// Server
@@ -518,7 +522,7 @@ func (c *cmdMigrate) Run(cmd *cobra.Command, args []string) error {
 		defer func() { _ = server.DeleteCertificate(clientFingerprint) }()
 	}
 
-	config, err := c.RunInteractive(server)
+	config, err := c.runInteractive(server)
 	if err != nil {
 		return err
 	}
@@ -587,6 +591,12 @@ func (c *cmdMigrate) Run(cmd *cobra.Command, args []string) error {
 	} else {
 		_, ext, convCmd, _ := archive.DetectCompression(config.SourcePath)
 		if ext == ".qcow2" || ext == ".vmdk" {
+			// COnfirm the command is available.
+			_, err := exec.LookPath(convCmd[0])
+			if err != nil {
+				return fmt.Errorf("Unable to find required command %q", convCmd[0])
+			}
+
 			destImg := filepath.Join(path, "converted-raw-image.img")
 
 			cmd := []string{
@@ -625,7 +635,7 @@ func (c *cmdMigrate) Run(cmd *cobra.Command, args []string) error {
 		fullPath = path
 		target := filepath.Join(path, "root.img")
 
-		err := os.WriteFile(target, nil, 0o644)
+		err = os.WriteFile(target, nil, 0o644)
 		if err != nil {
 			return fmt.Errorf("Failed to create %q: %w", target, err)
 		}
@@ -651,16 +661,16 @@ func (c *cmdMigrate) Run(cmd *cobra.Command, args []string) error {
 
 	config.InstanceArgs.Architecture = architectureName
 
-	revert := revert.New()
-	defer revert.Fail()
+	reverter := revert.New()
+	defer reverter.Fail()
 
 	// Create the instance
 	op, err := server.CreateInstance(config.InstanceArgs)
 	if err != nil {
 		return err
 	}
 
-	revert.Add(func() {
+	reverter.Add(func() {
 		_, _ = server.DeleteInstance(config.InstanceArgs.Name)
 	})
 
@@ -677,7 +687,7 @@ func (c *cmdMigrate) Run(cmd *cobra.Command, args []string) error {
 	}
 
 	progress.Done(fmt.Sprintf("Instance %s successfully created", config.InstanceArgs.Name))
-	revert.Success()
+	reverter.Success()
 
 	return nil
 }

cmd/incus-migrate/main_netcat.go

@@ -16,18 +16,18 @@ type cmdNetcat struct {
 	global *cmdGlobal
 }
 
-func (c *cmdNetcat) Command() *cobra.Command {
+func (c *cmdNetcat) command() *cobra.Command {
 	cmd := &cobra.Command{}
 
 	cmd.Use = "netcat <address>"
 	cmd.Short = "Sends stdin data to a unix socket"
-	cmd.RunE = c.Run
+	cmd.RunE = c.run
 	cmd.Hidden = true
 
 	return cmd
 }
 
-func (c *cmdNetcat) Run(cmd *cobra.Command, args []string) error {
+func (c *cmdNetcat) run(cmd *cobra.Command, args []string) error {
 	// Help and usage
 	if len(args) == 0 {
 		_ = cmd.Help()

cmd/incus-migrate/transfer.go

@@ -131,12 +131,12 @@ func rsyncSendSetup(ctx context.Context, path string, rsyncArgs string, instance
 	return cmd, conn, stderr, nil
 }
 
-func protoSendError(ws *websocket.Conn, err error) {
-	migration.ProtoSendControl(ws, err)
+func protoSendError(conn *websocket.Conn, err error) {
+	migration.ProtoSendControl(conn, err)
 
 	if err != nil {
 		closeMsg := websocket.FormatCloseMessage(websocket.CloseNormalClosure, "")
-		_ = ws.WriteMessage(websocket.CloseMessage, closeMsg)
-		_ = ws.Close()
+		_ = conn.WriteMessage(websocket.CloseMessage, closeMsg)
+		_ = conn.Close()
 	}
 }

cmd/incus-migrate/utils.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"crypto/x509"
 	"encoding/pem"
+	"errors"
 	"fmt"
 	"io"
 	"net/url"
@@ -137,7 +138,7 @@ func transferRootfs(ctx context.Context, dst incus.InstanceServer, op incus.Oper
 	}
 
 	if !msg.GetSuccess() {
-		return fmt.Errorf(msg.GetMessage())
+		return errors.New(msg.GetMessage())
 	}
 
 	return nil
@@ -150,7 +151,7 @@ func (m *cmdMigrate) connectLocal() (incus.InstanceServer, error) {
 	return incus.ConnectIncusUnix("", &args)
 }
 
-func (m *cmdMigrate) connectTarget(url string, certPath string, keyPath string, authType string, token string) (incus.InstanceServer, string, error) {
+func (m *cmdMigrate) connectTarget(uri string, certPath string, keyPath string, authType string, token string) (incus.InstanceServer, string, error) {
 	args := incus.ConnectionArgs{
 		AuthType: authType,
 	}
@@ -199,12 +200,12 @@ func (m *cmdMigrate) connectTarget(url string, certPath string, keyPath string,
 
 	// Attempt to connect using the system CA
 	args.UserAgent = fmt.Sprintf("LXC-MIGRATE %s", version.Version)
-	c, err := incus.ConnectIncus(url, &args)
+	c, err := incus.ConnectIncus(uri, &args)
 
 	var certificate *x509.Certificate
 	if err != nil {
 		// Failed to connect using the system CA, so retrieve the remote certificate
-		certificate, err = localtls.GetRemoteCertificate(url, args.UserAgent)
+		certificate, err = localtls.GetRemoteCertificate(uri, args.UserAgent)
 		if err != nil {
 			return nil, "", err
 		}
@@ -216,7 +217,7 @@ func (m *cmdMigrate) connectTarget(url string, certPath string, keyPath string,
 		args.TLSServerCert = string(serverCrt)
 
 		// Setup a new connection, this time with the remote certificate
-		c, err = incus.ConnectIncus(url, &args)
+		c, err = incus.ConnectIncus(uri, &args)
 		if err != nil {
 			return nil, "", err
 		}
@@ -304,23 +305,23 @@ func setupSource(path string, mounts []string) error {
 }
 
 func parseURL(URL string) (string, error) {
-	u, err := url.Parse(URL)
+	uri, err := url.Parse(URL)
 	if err != nil {
 		return "", err
 	}
 
 	// Create a URL with scheme and hostname since it wasn't provided
-	if u.Scheme == "" && u.Host == "" && u.Path != "" {
-		u, err = url.Parse(fmt.Sprintf("https://%s", u.Path))
+	if uri.Scheme == "" && uri.Host == "" && uri.Path != "" {
+		uri, err = url.Parse(fmt.Sprintf("https://%s", uri.Path))
 		if err != nil {
 			return "", err
 		}
 	}
 
 	// If no port was provided, use default port
-	if u.Port() == "" {
-		u.Host = fmt.Sprintf("%s:%d", u.Hostname(), ports.HTTPSDefaultPort)
+	if uri.Port() == "" {
+		uri.Host = fmt.Sprintf("%s:%d", uri.Hostname(), ports.HTTPSDefaultPort)
 	}
 
-	return u.String(), nil
+	return uri.String(), nil
 }

shared/ask/ask.go

@@ -69,7 +69,7 @@ func (a *Asker) AskInt(question string, minValue int64, maxValue int64, defaultA
 			continue
 		}
 
-		if (minValue > -1 && result >= minValue) || (maxValue > -1 && result <= maxValue) {
+		if (minValue > -1 && result < minValue) || (maxValue > -1 && result > maxValue) {
 			fmt.Fprintf(os.Stderr, "Invalid input: out of range\n\n")
 			continue
 		}