From 5b3801776b733a322ec6290d08a0d37a6de18efd Mon Sep 17 00:00:00 2001 From: Brent Baude Date: Tue, 19 Sep 2023 09:09:53 -0500 Subject: [PATCH] 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 --- go.mod | 2 +- go.sum | 4 +- pkg/machine/applehv/machine.go | 2 +- pkg/machine/e2e/README.md | 2 +- pkg/machine/e2e/basic_test.go | 9 ++-- pkg/machine/e2e/config_linux_test.go | 7 +++ pkg/machine/e2e/config_windows_test.go | 20 +++++++++ pkg/machine/e2e/machine_test.go | 3 ++ pkg/machine/errors.go | 32 +++++++++++++- pkg/machine/hyperv/config.go | 19 ++++---- pkg/machine/hyperv/machine.go | 43 ++++++++++++++----- pkg/machine/qemu/machine.go | 2 +- pkg/machine/wsl/machine.go | 10 ++--- .../containers/libhvee/pkg/hypervctl/vm.go | 23 +++++++--- .../libhvee/pkg/hypervctl/vm_config.go | 2 +- vendor/modules.txt | 2 +- 16 files changed, 139 insertions(+), 43 deletions(-) diff --git a/go.mod b/go.mod index 179c3bcfe0..b99dc13139 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index f8d410a52f..18c1c99b1c 100644 --- a/go.sum +++ b/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= diff --git a/pkg/machine/applehv/machine.go b/pkg/machine/applehv/machine.go index 9c7c020008..d69b4a3ef5 100644 --- a/pkg/machine/applehv/machine.go +++ b/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 diff --git a/pkg/machine/e2e/README.md b/pkg/machine/e2e/README.md index 4b4ddf6567..1f90accf2b 100644 --- a/pkg/machine/e2e/README.md +++ b/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 diff --git a/pkg/machine/e2e/basic_test.go b/pkg/machine/e2e/basic_test.go index 0993811736..a1cfb71ef5 100644 --- a/pkg/machine/e2e/basic_test.go +++ b/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")) }) }) diff --git a/pkg/machine/e2e/config_linux_test.go b/pkg/machine/e2e/config_linux_test.go index f07121a90f..644c7bff65 100644 --- a/pkg/machine/e2e/config_linux_test.go +++ b/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 +} diff --git a/pkg/machine/e2e/config_windows_test.go b/pkg/machine/e2e/config_windows_test.go index 5c39507140..9fa63cd884 100644 --- a/pkg/machine/e2e/config_windows_test.go +++ b/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 +} diff --git a/pkg/machine/e2e/machine_test.go b/pkg/machine/e2e/machine_test.go index af20d2f23e..23ef4b6d27 100644 --- a/pkg/machine/e2e/machine_test.go +++ b/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 } diff --git a/pkg/machine/errors.go b/pkg/machine/errors.go index ca01f8325e..7a3724aefc 100644 --- a/pkg/machine/errors.go +++ b/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) +} diff --git a/pkg/machine/hyperv/config.go b/pkg/machine/hyperv/config.go index 0a096ba345..0de74ef8a5 100644 --- a/pkg/machine/hyperv/config.go +++ b/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 diff --git a/pkg/machine/hyperv/machine.go b/pkg/machine/hyperv/machine.go index 6e11967c3a..ecbc6f0aac 100644 --- a/pkg/machine/hyperv/machine.go +++ b/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 +} diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index 76e7c69c9f..5b1045823c 100644 --- a/pkg/machine/qemu/machine.go +++ b/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 { diff --git a/pkg/machine/wsl/machine.go b/pkg/machine/wsl/machine.go index 05be2b55a8..4ded87d7ab 100644 --- a/pkg/machine/wsl/machine.go +++ b/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 { diff --git a/vendor/github.com/containers/libhvee/pkg/hypervctl/vm.go b/vendor/github.com/containers/libhvee/pkg/hypervctl/vm.go index 4d60f551c6..944ae2b91c 100644 --- a/vendor/github.com/containers/libhvee/pkg/hypervctl/vm.go +++ b/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 } diff --git a/vendor/github.com/containers/libhvee/pkg/hypervctl/vm_config.go b/vendor/github.com/containers/libhvee/pkg/hypervctl/vm_config.go index 6ee525ebd9..82740c578e 100644 --- a/vendor/github.com/containers/libhvee/pkg/hypervctl/vm_config.go +++ b/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 diff --git a/vendor/modules.txt b/vendor/modules.txt index ae1f10da6b..ab2b68647a 100644 --- a/vendor/modules.txt +++ b/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