Merge pull request #20074 from baude/hypervmachinee2epass1

Various updates for hyperv and machine e2e tests

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.20230905135638-56fb23533417/go.mod h1:HiXu8GZyjzGjU834fROO00Ka/4B1IM8qxy/6q6x1f+4=
+github.com/containers/libhvee v0.4.1-0.20230920190832-6ab399cadb68 h1:QIwOjkVpJp/onBOozw+MSr1mow9f5XQ8QG7Y8AP2Xp0=
+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()
-		Expect(err).To(HaveOccurred())
+		out, _ = pgrep("gvproxy")
+		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
-	mm := HyperVMachine{Name: name}
-	configDir, err := machine.GetConfDir(v.VMType())
+	var found bool
+	vms, err := v.loadFromLocalJson()
 	if err != nil {
 		return false, err
 	}
-	if err := mm.loadHyperVMachineFromJSON(configDir); err != nil {
-		return false, err
+	for _, vm := range vms {
+		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()
-		err = vm.writeConfig()
+		v.LastUp = time.Now()
+		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.VirtualQuantity = uint64(config.Memory)
+			ms.Limit = 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 != nil {
+		if err := vm.getMemorySettings(mem); 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