Various updates for hyperv and machine e2e tests

This PR is a mishmash of updates needed so that the hyperv provider can
begin to passd the machine e2e tests.

Summary as follows:
* Added custom error handling for machine errors so that all providers
  can generate the same formatted error messages.  The ones implemented
  thus far are needed for the basic and init tests.  More will come as
  they are identified.
* Vendored new libhvee for better memory inspection.  The memory type
  changed from uint32 to uint64.
* Some machine e2e tests used linux-specific utilities to check various
  error conditions and messages (like pgrep).  Those were made into
  functions and implemented on an operating system level.

[NO NEW TESTS NEEDED]

Signed-off-by: Brent Baude <bbaude@redhat.com>

go.mod

@@ -17,7 +17,7 @@ require (
 	github.com/containers/conmon v2.0.20+incompatible
 	github.com/containers/gvisor-tap-vsock v0.7.1-0.20230921084021-9298405740ad
 	github.com/containers/image/v5 v5.28.0
-	github.com/containers/libhvee v0.4.1-0.20230905135638-56fb23533417
+	github.com/containers/libhvee v0.4.1-0.20230920190832-6ab399cadb68
 	github.com/containers/ocicrypt v1.1.8
 	github.com/containers/psgo v1.8.0
 	github.com/containers/storage v1.50.2

go.sum

@@ -257,8 +257,8 @@ github.com/containers/gvisor-tap-vsock v0.7.1-0.20230921084021-9298405740ad h1:b
 github.com/containers/gvisor-tap-vsock v0.7.1-0.20230921084021-9298405740ad/go.mod h1:n7xPNoTHhif+K4S9zZMUmBNUvt5tcLfkHlQHreTLomM=
 github.com/containers/image/v5 v5.28.0 h1:H4cWbdI88UA/mDb6SxMo3IxpmS1BSs/Kifvhwt9g048=
 github.com/containers/image/v5 v5.28.0/go.mod h1:9aPnNkwHNHgGl9VlQxXEshvmOJRbdRAc1rNDD6sP2eU=
-github.com/containers/libhvee v0.4.1-0.20230905135638-56fb23533417 h1:fr+j21PD+IYR6Kvlf2Zrm1x9oAjV12T2Vz3oZIGTusw=
+github.com/containers/libhvee v0.4.1-0.20230920190832-6ab399cadb68 h1:QIwOjkVpJp/onBOozw+MSr1mow9f5XQ8QG7Y8AP2Xp0=
-github.com/containers/libhvee v0.4.1-0.20230905135638-56fb23533417/go.mod h1:HiXu8GZyjzGjU834fROO00Ka/4B1IM8qxy/6q6x1f+4=
+github.com/containers/libhvee v0.4.1-0.20230920190832-6ab399cadb68/go.mod h1:HiXu8GZyjzGjU834fROO00Ka/4B1IM8qxy/6q6x1f+4=
 github.com/containers/libtrust v0.0.0-20230121012942-c1716e8a8d01 h1:Qzk5C6cYglewc+UyGf6lc8Mj2UaPTHy/iF2De0/77CA=
 github.com/containers/libtrust v0.0.0-20230121012942-c1716e8a8d01/go.mod h1:9rfv8iPl1ZP7aqh9YA68wnZv2NUDbXdcdPHVz0pFbPY=
 github.com/containers/luksy v0.0.0-20230808154129-d2d74a56682f h1:/HjLNYkVoUJNT4mm2dzGl63x7nD6YHxxI/k1kR0TkzA=

pkg/machine/applehv/machine.go

@@ -353,7 +353,7 @@ func (m *MacMachine) Remove(name string, opts machine.RemoveOptions) (string, fu
 
 	if vmState == machine.Running {
 		if !opts.Force {
-			return "", nil, fmt.Errorf("invalid state: %s is running", m.Name)
+			return "", nil, &machine.ErrVMRunningCannotDestroyed{Name: m.Name}
 		}
 		if err := m.Vfkit.stop(true, true); err != nil {
 			return "", nil, err

pkg/machine/e2e/README.md

@@ -1,6 +1,6 @@
 # Working README for running the machine tests
 
-
+Note: you must not have any machines defined before running tests
 ## Linux
 
 ### QEMU

pkg/machine/e2e/basic_test.go

@@ -5,7 +5,6 @@ import (
 	"net"
 	"net/http"
 	"net/url"
-	"os/exec"
 	"time"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -69,9 +68,9 @@ var _ = Describe("run basic podman commands", func() {
 		Expect(runAlp).To(Exit(0))
 		testHTTPServer("62544", false, "podman rulez")
 
-		out, err := exec.Command("pgrep", "gvproxy").Output()
+		out, err := pgrep("gvproxy")
 		Expect(err).ToNot(HaveOccurred())
-		Expect(string(out)).ToNot(BeEmpty())
+		Expect(out).ToNot(BeEmpty())
 
 		rmCon, err := mb.setCmd(bm.withPodmanCommand([]string{"rm", "-af"})).run()
 		Expect(err).ToNot(HaveOccurred())
@@ -84,8 +83,8 @@ var _ = Describe("run basic podman commands", func() {
 		Expect(stopSession).To(Exit(0))
 
 		// gxproxy should exit after machine is stopped
-		_, err = exec.Command("pgrep", "gvproxy").Output()
+		out, _ = pgrep("gvproxy")
-		Expect(err).To(HaveOccurred())
+		Expect(out).ToNot(ContainSubstring("gvproxy"))
 	})
 
 })

pkg/machine/e2e/config_linux_test.go

@@ -1,3 +1,10 @@
 package e2e_test
 
+import "os/exec"
+
 const podmanBinary = "../../../bin/podman-remote"
+
+func pgrep(n string) (string, error) {
+	out, err := exec.Command("pgrep", "gvproxy").Output()
+	return string(out), err
+}

pkg/machine/e2e/config_windows_test.go

@@ -1,6 +1,10 @@
 package e2e_test
 
 import (
+	"fmt"
+	"os/exec"
+	"strings"
+
 	"github.com/containers/podman/v4/pkg/machine"
 	"github.com/containers/podman/v4/pkg/machine/wsl"
 	. "github.com/onsi/ginkgo/v2"
@@ -15,3 +19,19 @@ func getDownloadLocation(_ machine.VirtProvider) string {
 	}
 	return fd.Get().URL.String()
 }
+
+// pgrep emulates the pgrep linux command
+func pgrep(n string) (string, error) {
+	// add filter to find the process and do no display a header
+	args := []string{"/fi", fmt.Sprintf("IMAGENAME eq %s", n), "/nh"}
+	out, err := exec.Command("tasklist.exe", args...).Output()
+	if err != nil {
+		return "", err
+	}
+	strOut := string(out)
+	// in pgrep, if no running process is found, it exits 1 and the output is zilch
+	if strings.Contains(strOut, "INFO: No tasks are running which match the specified search") {
+		return "", fmt.Errorf("no task found")
+	}
+	return strOut, nil
+}

pkg/machine/e2e/machine_test.go

@@ -130,6 +130,9 @@ func setup() (string, *machineTestBuilder) {
 	if _, err := io.Copy(n, f); err != nil {
 		Fail(fmt.Sprintf("failed to copy %ss to %s: %q", fqImageName, mb.imagePath, err))
 	}
+	if err := n.Close(); err != nil {
+		Fail(fmt.Sprintf("failed to close image copy handler: %q", err))
+	}
 	return homeDir, mb
 }
 

pkg/machine/errors.go

@@ -1,6 +1,11 @@
 package machine
 
-import "errors"
+import (
+	"errors"
+	"fmt"
+
+	"github.com/containers/podman/v4/pkg/strongunits"
+)
 
 var (
 	ErrNoSuchVM         = errors.New("VM does not exist")
@@ -10,3 +15,28 @@ var (
 	ErrMultipleActiveVM = errors.New("only one VM can be active at a time")
 	ErrNotImplemented   = errors.New("functionality not implemented")
 )
+
+type ErrVMRunningCannotDestroyed struct {
+	Name string
+}
+
+func (err *ErrVMRunningCannotDestroyed) Error() string {
+	return fmt.Sprintf("running vm %q cannot be destroyed", err.Name)
+}
+
+type ErrVMDoesNotExist struct {
+	Name string
+}
+
+func (err *ErrVMDoesNotExist) Error() string {
+	// the current error in qemu is not quoted
+	return fmt.Sprintf("%s: VM does not exist", err.Name)
+}
+
+type ErrNewDiskSizeTooSmall struct {
+	OldSize, NewSize strongunits.GiB
+}
+
+func (err *ErrNewDiskSizeTooSmall) Error() string {
+	return fmt.Sprintf("invalid disk size %d: new disk must be larger than %dGB", err.OldSize, err.NewSize)
+}

pkg/machine/hyperv/config.go

@@ -44,20 +44,23 @@ func (v HyperVVirtualization) CheckExclusiveActiveVM() (bool, string, error) {
 }
 
 func (v HyperVVirtualization) IsValidVMName(name string) (bool, error) {
-	// We check both the local filesystem and hyperv for the valid name
+	var found bool
-	mm := HyperVMachine{Name: name}
+	vms, err := v.loadFromLocalJson()
-	configDir, err := machine.GetConfDir(v.VMType())
 	if err != nil {
 		return false, err
 	}
-	if err := mm.loadHyperVMachineFromJSON(configDir); err != nil {
+	for _, vm := range vms {
-		return false, err
+		if vm.Name == name {
+			found = true
+			break
+		}
+	}
+	if !found {
+		return false, nil
 	}
-	// The name is valid for the local filesystem
 	if _, err := hypervctl.NewVirtualMachineManager().GetMachine(name); err != nil {
 		return false, err
 	}
-	// The lookup in hyperv worked, so it is also valid there
 	return true, nil
 }
 
@@ -159,7 +162,7 @@ func (v HyperVVirtualization) NewMachine(opts machine.InitOptions) (machine.VM,
 		CPUs:     uint16(opts.CPUS),
 		DiskPath: imagePath.GetPath(),
 		DiskSize: opts.DiskSize,
-		Memory:   int32(opts.Memory),
+		Memory:   opts.Memory,
 	}
 
 	// Write the json configuration file which will be loaded by

pkg/machine/hyperv/machine.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"io/fs"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"time"
 
@@ -17,9 +18,9 @@ import (
 	gvproxy "github.com/containers/gvisor-tap-vsock/pkg/types"
 	"github.com/containers/libhvee/pkg/hypervctl"
 	"github.com/containers/podman/v4/pkg/machine"
+	"github.com/containers/podman/v4/pkg/strongunits"
 	"github.com/containers/podman/v4/pkg/util"
 	"github.com/containers/podman/v4/utils"
-	"github.com/docker/go-units"
 	"github.com/sirupsen/logrus"
 )
 
@@ -253,7 +254,6 @@ func (m *HyperVMachine) Init(opts machine.InitOptions) (bool, error) {
 		return false, os.WriteFile(m.IgnitionFile.GetPath(), inputIgnition, 0644)
 	}
 
-	// Write the JSON file for the second time.  First time was in NewMachine
 	if err := m.writeConfig(); err != nil {
 		return false, err
 	}
@@ -262,6 +262,10 @@ func (m *HyperVMachine) Init(opts machine.InitOptions) (bool, error) {
 	if err := m.writeIgnitionConfigFile(opts, m.RemoteUsername, key); err != nil {
 		return false, err
 	}
+
+	if err := m.resizeDisk(strongunits.GiB(opts.DiskSize)); err != nil {
+		return false, err
+	}
 	// The ignition file has been written. We now need to
 	// read it so that we can put it into key-value pairs
 	err = m.readAndSplitIgnition()
@@ -284,16 +288,16 @@ func (m *HyperVMachine) Inspect() (*machine.InspectInfo, error) {
 		ConnectionInfo: machine.ConnectionConfig{},
 		Created:        m.Created,
 		Image: machine.ImageConfig{
-			IgnitionFile: machine.VMFile{},
+			IgnitionFile: m.IgnitionFile,
 			ImageStream:  "",
-			ImagePath:    machine.VMFile{},
+			ImagePath:    m.ImagePath,
 		},
 		LastUp: m.LastUp,
 		Name:   m.Name,
 		Resources: machine.ResourceConfig{
 			CPUs:     uint64(cfg.Hardware.CPUs),
 			DiskSize: 0,
-			Memory:   uint64(cfg.Hardware.Memory),
+			Memory:   cfg.Hardware.Memory,
 		},
 		SSHConfig: m.SSHConfig,
 		State:     vm.State().String(),
@@ -348,7 +352,7 @@ func (m *HyperVMachine) Remove(_ string, opts machine.RemoveOptions) (string, fu
 	// In hyperv, they call running 'enabled'
 	if vm.State() == hypervctl.Enabled {
 		if !opts.Force {
-			return "", nil, hypervctl.ErrMachineStateInvalid
+			return "", nil, &machine.ErrVMRunningCannotDestroyed{Name: m.Name}
 		}
 		if err := vm.Stop(); err != nil {
 			return "", nil, err
@@ -399,7 +403,10 @@ func (m *HyperVMachine) Set(name string, opts machine.SetOptions) ([]error, erro
 		}
 	}
 	if opts.DiskSize != nil && m.DiskSize != *opts.DiskSize {
-		setErrors = append(setErrors, hypervctl.ErrNotImplemented)
+		newDiskSize := strongunits.GiB(*opts.DiskSize)
+		if err := m.resizeDisk(newDiskSize); err != nil {
+			setErrors = append(setErrors, err)
+		}
 	}
 	if opts.CPUs != nil && m.CPUs != *opts.CPUs {
 		m.CPUs = *opts.CPUs
@@ -545,6 +552,9 @@ func (m *HyperVMachine) loadFromFile() (*HyperVMachine, error) {
 	mm := HyperVMachine{}
 
 	if err := mm.loadHyperVMachineFromJSON(jsonPath); err != nil {
+		if errors.Is(err, machine.ErrNoSuchVM) {
+			return nil, &machine.ErrVMDoesNotExist{Name: m.Name}
+		}
 		return nil, err
 	}
 	vmm := hypervctl.NewVirtualMachineManager()
@@ -566,8 +576,6 @@ func (m *HyperVMachine) loadFromFile() (*HyperVMachine, error) {
 	if cfg.Hardware.Memory > 0 {
 		mm.Memory = uint64(cfg.Hardware.Memory)
 	}
-
-	mm.DiskSize = cfg.Hardware.DiskSize * units.MiB
 	mm.LastUp = cfg.Status.LastUp
 
 	return &mm, nil
@@ -583,7 +591,7 @@ func (m *HyperVMachine) loadHyperVMachineFromJSON(fqConfigPath string) error {
 	b, err := os.ReadFile(fqConfigPath)
 	if err != nil {
 		if errors.Is(err, fs.ErrNotExist) {
-			return fmt.Errorf("%q: %w", fqConfigPath, machine.ErrNoSuchVM)
+			return machine.ErrNoSuchVM
 		}
 		return err
 	}
@@ -634,7 +642,7 @@ func (m *HyperVMachine) startHostNetworking() (string, machine.APIForwardingStat
 
 	c := cmd.Cmd(gvproxyBinary)
 	if err := c.Start(); err != nil {
-		return "", 0, fmt.Errorf("unable to execute: %q: %w", cmd, err)
+		return "", 0, fmt.Errorf("unable to execute: %s: %w", cmd.ToCmdline(), err)
 	}
 	return forwardSock, state, nil
 }
@@ -691,3 +699,16 @@ func (m *HyperVMachine) setRootful(rootful bool) error {
 	m.HostUser.Modified = true
 	return nil
 }
+
+func (m *HyperVMachine) resizeDisk(newSize strongunits.GiB) error {
+	if m.DiskSize > uint64(newSize) {
+		return &machine.ErrNewDiskSizeTooSmall{OldSize: strongunits.ToGiB(strongunits.B(m.DiskSize)), NewSize: newSize}
+	}
+	resize := exec.Command("powershell", []string{"-command", fmt.Sprintf("Resize-VHD %s %d", m.ImagePath.GetPath(), newSize.ToBytes())}...)
+	resize.Stdout = os.Stdout
+	resize.Stderr = os.Stderr
+	if err := resize.Run(); err != nil {
+		return fmt.Errorf("resizing image: %q", err)
+	}
+	return nil
+}

pkg/machine/qemu/machine.go

@@ -1174,7 +1174,7 @@ func (v *MachineVM) Remove(_ string, opts machine.RemoveOptions) (string, func()
 	}
 	if state == machine.Running {
 		if !opts.Force {
-			return "", nil, fmt.Errorf("running vm %q cannot be destroyed", v.Name)
+			return "", nil, &machine.ErrVMRunningCannotDestroyed{Name: v.Name}
 		}
 		err := v.stopLocked()
 		if err != nil {

pkg/machine/wsl/machine.go

@@ -1523,7 +1523,7 @@ func (v *MachineVM) Remove(name string, opts machine.RemoveOptions) (string, fun
 
 	if v.isRunning() {
 		if !opts.Force {
-			return "", nil, fmt.Errorf("running vm %q cannot be destroyed", v.Name)
+			return "", nil, &machine.ErrVMRunningCannotDestroyed{Name: v.Name}
 		}
 		if err := v.Stop(v.Name, machine.StopOptions{}); err != nil {
 			return "", nil, err
@@ -1631,14 +1631,14 @@ func (v *MachineVM) SSH(name string, opts machine.SSHOptions) error {
 	return cmd.Run()
 }
 
-func (vm *MachineVM) updateTimeStamps(updateLast bool) (time.Time, time.Time, error) {
+func (v *MachineVM) updateTimeStamps(updateLast bool) (time.Time, time.Time, error) {
 	var err error
 	if updateLast {
-		vm.LastUp = time.Now()
+		v.LastUp = time.Now()
-		err = vm.writeConfig()
+		err = v.writeConfig()
 	}
 
-	return vm.Created, vm.LastUp, err
+	return v.Created, v.LastUp, err
 }
 
 func getDiskSize(vm *MachineVM) uint64 {

vendor/github.com/containers/libhvee/pkg/hypervctl/vm.go

@@ -326,13 +326,18 @@ func (vm *VirtualMachine) GetConfig(diskPath string) (*HyperVConfig, error) {
 		return nil, err
 	}
 	diskSize = uint64(diskPathInfo.Size())
+	mem := MemorySettings{}
+	if err := vm.getMemorySettings(&mem); err != nil {
+		return nil, err
+	}
 
 	config := HyperVConfig{
 		Hardware: HardwareConfig{
+			// TODO we could implement a getProcessorSettings like we did for memory
 			CPUs:     summary.NumberOfProcessors,
 			DiskPath: diskPath,
 			DiskSize: diskSize,
-			Memory:   summary.MemoryAvailable,
+			Memory:   mem.Limit,
 		},
 		Status: Statuses{
 			Created:  vm.InstallDate,
@@ -403,8 +408,8 @@ func (vmm *VirtualMachineManager) NewVirtualMachine(name string, config *Hardwar
 
 			// The API seems to require both of these even
 			// when not using dynamic memory
-			ms.Limit = uint64(config.Memory)
+			ms.Limit = config.Memory
-			ms.VirtualQuantity = uint64(config.Memory)
+			ms.VirtualQuantity = config.Memory
 		}).
 		PrepareProcessorSettings(func(ps *ProcessorSettings) {
 			ps.VirtualQuantity = uint64(config.CPUs) // 4 cores
@@ -468,6 +473,15 @@ func (vm *VirtualMachine) fetchExistingResourceSettings(service *wmiext.Service,
 	return service.FindFirstRelatedObject(path, resourceType, resourceSettings)
 }
 
+func (vm *VirtualMachine) getMemorySettings(m *MemorySettings) error {
+	service, err := wmiext.NewLocalService(HyperVNamespace)
+	if err != nil {
+		return err
+	}
+	defer service.Close()
+	return vm.fetchExistingResourceSettings(service, "Msvm_MemorySettingData", m)
+}
+
 // Update processor and/or mem
 func (vm *VirtualMachine) UpdateProcessorMemSettings(updateProcessor func(*ProcessorSettings), updateMemory func(*MemorySettings)) error {
 	service, err := wmiext.NewLocalService(HyperVNamespace)
@@ -496,8 +510,7 @@ func (vm *VirtualMachine) UpdateProcessorMemSettings(updateProcessor func(*Proce
 	}
 
 	if updateMemory != nil {
-		err = vm.fetchExistingResourceSettings(service, "Msvm_MemorySettingData", mem)
+		if err := vm.getMemorySettings(mem); err != nil {
-		if err != nil {
 			return err
 		}
 

vendor/github.com/containers/libhvee/pkg/hypervctl/vm_config.go

@@ -97,7 +97,7 @@ type HardwareConfig struct {
 	// Disk size in gigabytes assigned to the vm
 	DiskSize uint64
 	// Memory in megabytes assigned to the vm
-	Memory int32
+	Memory uint64
 	// Network is bool to add a Network Connection to the
 	// default network switch in Microsoft HyperV
 	Network bool

vendor/modules.txt

@@ -301,7 +301,7 @@ github.com/containers/image/v5/transports
 github.com/containers/image/v5/transports/alltransports
 github.com/containers/image/v5/types
 github.com/containers/image/v5/version
-# github.com/containers/libhvee v0.4.1-0.20230905135638-56fb23533417
+# github.com/containers/libhvee v0.4.1-0.20230920190832-6ab399cadb68
 ## explicit; go 1.18
 github.com/containers/libhvee/pkg/hypervctl
 github.com/containers/libhvee/pkg/kvp/ginsu