diff --git a/pkg/machine/define/errors.go b/pkg/machine/define/errors.go index 1a32bc5165..5b241c7566 100644 --- a/pkg/machine/define/errors.go +++ b/pkg/machine/define/errors.go @@ -8,12 +8,9 @@ import ( ) var ( - ErrNoSuchVM = errors.New("VM does not exist") - ErrWrongState = errors.New("VM in wrong state to perform action") - ErrVMAlreadyExists = errors.New("VM already exists") - ErrVMAlreadyRunning = errors.New("VM already running or starting") - ErrMultipleActiveVM = errors.New("only one VM can be active at a time") - ErrNotImplemented = errors.New("functionality not implemented") + ErrWrongState = errors.New("VM in wrong state to perform action") + ErrVMAlreadyExists = errors.New("VM already exists") + ErrNotImplemented = errors.New("functionality not implemented") ) type ErrVMRunningCannotDestroyed struct { @@ -49,3 +46,16 @@ type ErrIncompatibleMachineConfig struct { func (err *ErrIncompatibleMachineConfig) Error() string { return fmt.Sprintf("incompatible machine config %q (%s) for this version of Podman", err.Path, err.Name) } + +type ErrMultipleActiveVM struct { + Name string + Provider string +} + +func (err *ErrMultipleActiveVM) Error() string { + msg := "" + if err.Provider != "" { + msg = " on the " + err.Provider + " provider" + } + return fmt.Sprintf("%s already starting or running%s: only one VM can be active at a time", err.Name, msg) +} diff --git a/pkg/machine/e2e/start_test.go b/pkg/machine/e2e/start_test.go index 1ee43206dd..73a25b5eb4 100644 --- a/pkg/machine/e2e/start_test.go +++ b/pkg/machine/e2e/start_test.go @@ -61,8 +61,10 @@ var _ = Describe("podman machine start", func() { }) It("start machine already started", func() { + name := randomString() i := new(initMachine) - session, err := mb.setCmd(i.withImage(mb.imagePath)).run() + machineTestBuilderInit := mb.setName(name).setCmd(i.withImage(mb.imagePath)) + session, err := machineTestBuilderInit.run() Expect(err).ToNot(HaveOccurred()) Expect(session).To(Exit(0)) s := new(startMachine) @@ -78,7 +80,7 @@ var _ = Describe("podman machine start", func() { startSession, err = mb.setCmd(s).run() Expect(err).ToNot(HaveOccurred()) Expect(startSession).To(Exit(125)) - Expect(startSession.errorToString()).To(ContainSubstring("VM already running or starting")) + Expect(startSession.errorToString()).To(ContainSubstring(fmt.Sprintf("Error: unable to start %q: already running", machineTestBuilderInit.name))) }) It("start machine with conflict on SSH port", func() { @@ -210,10 +212,10 @@ var _ = Describe("podman machine start", func() { Expect(startSession1).To(Or(Exit(0), Exit(125)), "start command should succeed or fail with 125") if startSession1.ExitCode() == 0 { Expect(startSession2).To(Exit(125), "first start worked, second start must fail") - Expect(startSession2.errorToString()).To(ContainSubstring("machine %s is already running: only one VM can be active at a time", machine1)) + Expect(startSession2.errorToString()).To(ContainSubstring("%s already starting or running: only one VM can be active at a time", machine1)) } else { Expect(startSession2).To(Exit(0), "first start failed, second start succeed") - Expect(startSession1.errorToString()).To(ContainSubstring("machine %s is already running: only one VM can be active at a time", machine2)) + Expect(startSession1.errorToString()).To(ContainSubstring("%s already starting or running: only one VM can be active at a time", machine2)) } }) }) diff --git a/pkg/machine/shim/host.go b/pkg/machine/shim/host.go index b0daa19ba9..1315c9adb9 100644 --- a/pkg/machine/shim/host.go +++ b/pkg/machine/shim/host.go @@ -265,7 +265,7 @@ func VMExists(name string, vmstubbers []vmconfigs.VMProvider) (*vmconfigs.Machin return nil, false, err } if mc, found := mcs[name]; found { - return mc, true, nil + return mc.MachineConfig, true, nil } // Check with the provider hypervisor for _, vmstubber := range vmstubbers { @@ -281,31 +281,46 @@ func VMExists(name string, vmstubbers []vmconfigs.VMProvider) (*vmconfigs.Machin } // checkExclusiveActiveVM checks if any of the machines are already running -func checkExclusiveActiveVM(provider vmconfigs.VMProvider, mc *vmconfigs.MachineConfig) error { +func checkExclusiveActiveVM(currentProvider vmconfigs.VMProvider, mc *vmconfigs.MachineConfig) error { + providers := provider.GetAll() // Check if any other machines are running; if so, we error - localMachines, err := getMCsOverProviders([]vmconfigs.VMProvider{provider}) + localMachines, err := getMCsOverProviders(providers) if err != nil { return err } + for name, localMachine := range localMachines { - state, err := provider.State(localMachine, false) + state, err := localMachine.Provider.State(localMachine.MachineConfig, false) if err != nil { return err } if state == machineDefine.Running || state == machineDefine.Starting { if mc.Name == name { - return fmt.Errorf("unable to start %q: machine %s: %w", mc.Name, name, machineDefine.ErrVMAlreadyRunning) + return fmt.Errorf("unable to start %q: already running", mc.Name) } - return fmt.Errorf("unable to start %q: machine %s is already running: %w", mc.Name, name, machineDefine.ErrMultipleActiveVM) + + // A machine is running in the current provider + if currentProvider.VMType() == localMachine.Provider.VMType() { + fail := machineDefine.ErrMultipleActiveVM{Name: name} + return fmt.Errorf("unable to start %q: %w", mc.Name, &fail) + } + // A machine is running in an alternate provider + fail := machineDefine.ErrMultipleActiveVM{Name: name, Provider: localMachine.Provider.VMType().String()} + return fmt.Errorf("unable to start: %w", &fail) } } return nil } +type knownMachineConfig struct { + Provider vmconfigs.VMProvider + MachineConfig *vmconfigs.MachineConfig +} + // getMCsOverProviders loads machineconfigs from a config dir derived from the "provider". it returns only what is known on // disk so things like status may be incomplete or inaccurate -func getMCsOverProviders(vmstubbers []vmconfigs.VMProvider) (map[string]*vmconfigs.MachineConfig, error) { - mcs := make(map[string]*vmconfigs.MachineConfig) +func getMCsOverProviders(vmstubbers []vmconfigs.VMProvider) (map[string]knownMachineConfig, error) { + mcs := make(map[string]knownMachineConfig) for _, stubber := range vmstubbers { dirs, err := env.GetMachineDirs(stubber.VMType()) if err != nil { @@ -320,7 +335,10 @@ func getMCsOverProviders(vmstubbers []vmconfigs.VMProvider) (map[string]*vmconfi // iterate known mcs and add the stubbers for mcName, mc := range stubberMCs { if _, ok := mcs[mcName]; !ok { - mcs[mcName] = mc + mcs[mcName] = knownMachineConfig{ + Provider: stubber, + MachineConfig: mc, + } } } } @@ -414,7 +432,7 @@ func Start(mc *vmconfigs.MachineConfig, mp vmconfigs.VMProvider, dirs *machineDe } if state == machineDefine.Running || state == machineDefine.Starting { - return fmt.Errorf("machine %s: %w", mc.Name, machineDefine.ErrVMAlreadyRunning) + return fmt.Errorf("unable to start %q: already running", mc.Name) } }