Merge pull request #14850 from Luap99/e2e-machine

pkg/machine/e2e: do not import from cmd/podman

4 files changed, 65 insertions, 36 deletions

diff --git a/cmd/podman/machine/list.go b/cmd/podman/machine/list.go
index b1e31566f..dd4a86697 100644
--- a/cmd/podman/machine/list.go
+++ b/cmd/podman/machine/list.go
@@ -16,6 +16,7 @@ import (
 	"github.com/containers/podman/v4/cmd/podman/common"
 	"github.com/containers/podman/v4/cmd/podman/registry"
 	"github.com/containers/podman/v4/cmd/podman/validate"
+	"github.com/containers/podman/v4/pkg/domain/entities"
 	"github.com/containers/podman/v4/pkg/machine"
 	"github.com/docker/go-units"
 	"github.com/spf13/cobra"
@@ -44,23 +45,6 @@ type listFlagType struct {
 	quiet     bool
 }
 
-type ListReporter struct {
-	Name           string
-	Default        bool
-	Created        string
-	Running        bool
-	Starting       bool
-	LastUp         string
-	Stream         string
-	VMType         string
-	CPUs           uint64
-	Memory         string
-	DiskSize       string
-	Port           int
-	RemoteUsername string
-	IdentityPath   string
-}
-
 func init() {
 	registry.Commands = append(registry.Commands, registry.CliCommand{
 		Command: lsCmd,
@@ -70,7 +54,7 @@ func init() {
 	flags := lsCmd.Flags()
 	formatFlagName := "format"
 	flags.StringVar(&listFlag.format, formatFlagName, "{{.Name}}\t{{.VMType}}\t{{.Created}}\t{{.LastUp}}\t{{.CPUs}}\t{{.Memory}}\t{{.DiskSize}}\n", "Format volume output using JSON or a Go template")
-	_ = lsCmd.RegisterFlagCompletionFunc(formatFlagName, common.AutocompleteFormat(&ListReporter{}))
+	_ = lsCmd.RegisterFlagCompletionFunc(formatFlagName, common.AutocompleteFormat(&entities.ListReporter{}))
 	flags.BoolVar(&listFlag.noHeading, "noheading", false, "Do not print headers")
 	flags.BoolVarP(&listFlag.quiet, "quiet", "q", false, "Show only machine names")
 }
@@ -123,8 +107,8 @@ func list(cmd *cobra.Command, args []string) error {
 	return outputTemplate(cmd, machineReporter)
 }
 
-func outputTemplate(cmd *cobra.Command, responses []*ListReporter) error {
-	headers := report.Headers(ListReporter{}, map[string]string{
+func outputTemplate(cmd *cobra.Command, responses []*entities.ListReporter) error {
+	headers := report.Headers(entities.ListReporter{}, map[string]string{
 		"LastUp":   "LAST UP",
 		"VmType":   "VM TYPE",
 		"CPUs":     "CPUS",
@@ -183,15 +167,15 @@ func streamName(imageStream string) string {
 	return imageStream
 }
 
-func toMachineFormat(vms []*machine.ListResponse) ([]*ListReporter, error) {
+func toMachineFormat(vms []*machine.ListResponse) ([]*entities.ListReporter, error) {
 	cfg, err := config.ReadCustomConfig()
 	if err != nil {
 		return nil, err
 	}
 
-	machineResponses := make([]*ListReporter, 0, len(vms))
+	machineResponses := make([]*entities.ListReporter, 0, len(vms))
 	for _, vm := range vms {
-		response := new(ListReporter)
+		response := new(entities.ListReporter)
 		response.Default = vm.Name == cfg.Engine.ActiveService
 		response.Name = vm.Name
 		response.Running = vm.Running
@@ -211,15 +195,15 @@ func toMachineFormat(vms []*machine.ListResponse) ([]*ListReporter, error) {
 	return machineResponses, nil
 }
 
-func toHumanFormat(vms []*machine.ListResponse) ([]*ListReporter, error) {
+func toHumanFormat(vms []*machine.ListResponse) ([]*entities.ListReporter, error) {
 	cfg, err := config.ReadCustomConfig()
 	if err != nil {
 		return nil, err
 	}
 
-	humanResponses := make([]*ListReporter, 0, len(vms))
+	humanResponses := make([]*entities.ListReporter, 0, len(vms))
 	for _, vm := range vms {
-		response := new(ListReporter)
+		response := new(entities.ListReporter)
 		if vm.Name == cfg.Engine.ActiveService {
 			response.Name = vm.Name + "*"
 			response.Default = true
diff --git a/pkg/domain/entities/machine.go b/pkg/domain/entities/machine.go
new file mode 100644
index 000000000..6ba53dbd1
--- /dev/null
+++ b/pkg/domain/entities/machine.go
@@ -0,0 +1,18 @@
+package entities
+
+type ListReporter struct {
+	Name           string
+	Default        bool
+	Created        string
+	Running        bool
+	Starting       bool
+	LastUp         string
+	Stream         string
+	VMType         string
+	CPUs           uint64
+	Memory         string
+	DiskSize       string
+	Port           int
+	RemoteUsername string
+	IdentityPath   string
+}
diff --git a/pkg/machine/e2e/list_test.go b/pkg/machine/e2e/list_test.go
index e2121e7bf..9e3e9956c 100644
--- a/pkg/machine/e2e/list_test.go
+++ b/pkg/machine/e2e/list_test.go
@@ -2,9 +2,10 @@ package e2e
 
 import (
 	"strings"
+	"time"
 
 	"github.com/containers/common/pkg/util"
-	"github.com/containers/podman/v4/cmd/podman/machine"
+	"github.com/containers/podman/v4/pkg/domain/entities"
 	jsoniter "github.com/json-iterator/go"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
@@ -87,7 +88,7 @@ var _ = Describe("podman machine list", func() {
 		startSession, err := mb.setCmd(s).runWithoutWait()
 		Expect(err).To(BeNil())
 		l := new(listMachine)
-		for { // needs to be infinite because we need to check if running when inspect returns to avoid race conditions.
+		for i := 0; i < 30; i++ {
 			listSession, err := mb.setCmd(l).run()
 			Expect(listSession).To(Exit(0))
 			Expect(err).To(BeNil())
@@ -96,6 +97,7 @@ var _ = Describe("podman machine list", func() {
 			} else {
 				break
 			}
+			time.Sleep(3 * time.Second)
 		}
 		Expect(startSession).To(Exit(0))
 		listSession, err := mb.setCmd(l).run()
@@ -132,7 +134,7 @@ var _ = Describe("podman machine list", func() {
 		Expect(err).To(BeNil())
 		Expect(listSession2).To(Exit(0))
 
-		var listResponse []*machine.ListReporter
+		var listResponse []*entities.ListReporter
 		err = jsoniter.Unmarshal(listSession.Bytes(), &listResponse)
 		Expect(err).To(BeNil())
 
diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go
index 322aa3a15..6134e69e1 100644
--- a/pkg/machine/qemu/machine.go
+++ b/pkg/machine/qemu/machine.go
@@ -5,6 +5,7 @@ package qemu
 
 import (
 	"bufio"
+	"bytes"
 	"context"
 	"encoding/base64"
 	"encoding/json"
@@ -138,8 +139,10 @@ func (p *Provider) NewMachine(opts machine.InitOptions) (machine.VM, error) {
 	cmd = append(cmd, []string{
 		"-device", "virtio-serial",
 		// qemu needs to establish the long name; other connections can use the symlink'd
-		"-chardev", "socket,path=" + vm.ReadySocket.Path + ",server=on,wait=off,id=" + vm.Name + "_ready",
-		"-device", "virtserialport,chardev=" + vm.Name + "_ready" + ",name=org.fedoraproject.port.0",
+		// Note both id and chardev start with an extra "a" because qemu requires that it
+		// starts with an letter but users can also use numbers
+		"-chardev", "socket,path=" + vm.ReadySocket.Path + ",server=on,wait=off,id=a" + vm.Name + "_ready",
+		"-device", "virtserialport,chardev=a" + vm.Name + "_ready" + ",name=org.fedoraproject.port.0",
 		"-pidfile", vm.VMPidFilePath.GetPath()}...)
 	vm.CmdLine = cmd
 	return vm, nil
@@ -571,15 +574,25 @@ func (v *MachineVM) Start(name string, _ machine.StartOptions) error {
 	files := []*os.File{dnr, dnw, dnw, fd}
 	attr.Files = files
 	logrus.Debug(v.CmdLine)
-	cmd := v.CmdLine
+	cmdLine := v.CmdLine
 
 	// Disable graphic window when not in debug mode
 	// Done in start, so we're not suck with the debug level we used on init
 	if !logrus.IsLevelEnabled(logrus.DebugLevel) {
-		cmd = append(cmd, "-display", "none")
+		cmdLine = append(cmdLine, "-display", "none")
 	}
 
-	_, err = os.StartProcess(v.CmdLine[0], cmd, attr)
+	stderrBuf := &bytes.Buffer{}
+
+	cmd := &exec.Cmd{
+		Args:       cmdLine,
+		Path:       cmdLine[0],
+		Stdin:      dnr,
+		Stdout:     dnw,
+		Stderr:     stderrBuf,
+		ExtraFiles: []*os.File{fd},
+	}
+	err = cmd.Start()
 	if err != nil {
 		// check if qemu was not found
 		if !errors.Is(err, os.ErrNotExist) {
@@ -590,15 +603,17 @@ func (v *MachineVM) Start(name string, _ machine.StartOptions) error {
 		if err != nil {
 			return err
 		}
-		cmd[0], err = cfg.FindHelperBinary(QemuCommand, true)
+		cmdLine[0], err = cfg.FindHelperBinary(QemuCommand, true)
 		if err != nil {
 			return err
 		}
-		_, err = os.StartProcess(cmd[0], cmd, attr)
+		cmd.Path = cmdLine[0]
+		err = cmd.Start()
 		if err != nil {
 			return fmt.Errorf("unable to execute %q: %w", cmd, err)
 		}
 	}
+	defer cmd.Process.Release() //nolint:errcheck
 	fmt.Println("Waiting for VM ...")
 	socketPath, err := getRuntimeDir()
 	if err != nil {
@@ -613,6 +628,16 @@ func (v *MachineVM) Start(name string, _ machine.StartOptions) error {
 		if err == nil {
 			break
 		}
+		// check if qemu is still alive
+		var status syscall.WaitStatus
+		pid, err := syscall.Wait4(cmd.Process.Pid, &status, syscall.WNOHANG, nil)
+		if err != nil {
+			return fmt.Errorf("failed to read qemu process status: %w", err)
+		}
+		if pid > 0 {
+			// child exited
+			return fmt.Errorf("qemu exited unexpectedly with exit code %d, stderr: %s", status.ExitStatus(), stderrBuf.String())
+		}
 		time.Sleep(wait)
 		wait++
 	}