pkg/machine/e2e: fix broken cleanup

Currently all podman machine rm errors in AfterEach were ignored.
This means some leaked and caused issues later on, see #22844.

To fix it first rework the logic to only remove machines when needed at
the place were they are created using DeferCleanup(), however
DeferCleanup() does not work well together with AfterEach() as it always
run AfterEach() before DeferCleanup(). As AfterEach() deletes the dir
the podman machine rm call can not be done afterwards.

As such migrate all cleanup to use DeferCleanup() and while I have to
touch this fix the code to remove the per file duplciation and define
the setup/cleanup once in the global scope.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>

pkg/machine/e2e/basic_test.go

@@ -18,17 +18,6 @@ import (
 )
 
 var _ = Describe("run basic podman commands", func() {
-	var (
-		mb      *machineTestBuilder
-		testDir string
-	)
-
-	BeforeEach(func() {
-		testDir, mb = setup()
-	})
-	AfterEach(func() {
-		teardown(originalHomeDir, testDir, mb)
-	})
 
 	It("Basic ops", func() {
 		// golangci-lint has trouble with actually skipping tests marked Skip

pkg/machine/e2e/config_init_test.go

@@ -2,6 +2,11 @@ package e2e_test
 
 import (
 	"strconv"
+	"strings"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	. "github.com/onsi/gomega/gexec"
 )
 
 type initMachine struct {
@@ -71,7 +76,24 @@ func (i *initMachine) buildCmd(m *machineTestBuilder) []string {
 	if i.userModeNetworking {
 		cmd = append(cmd, "--user-mode-networking")
 	}
-	cmd = append(cmd, m.name)
+	name := m.name
+	cmd = append(cmd, name)
+
+	// when we create a new VM remove it again as cleanup
+	DeferCleanup(func() {
+		r := new(rmMachine)
+		session, err := m.setName(name).setCmd(r.withForce()).run()
+		Expect(err).ToNot(HaveOccurred(), "error occurred rm'ing machine")
+		// Some test create a invalid VM so the VM does not exists in this case we have to ignore the error.
+		// It would be much better if rm -f would behave like other commands and ignore not exists errors.
+		if session.ExitCode() == 125 {
+			if strings.Contains(session.errorToString(), "VM does not exist") {
+				return
+			}
+		}
+		Expect(session).To(Exit(0))
+	})
+
 	i.cmd = cmd
 	return cmd
 }

pkg/machine/e2e/info_test.go

@@ -11,17 +11,6 @@ import (
 )
 
 var _ = Describe("podman machine info", func() {
-	var (
-		mb      *machineTestBuilder
-		testDir string
-	)
-
-	BeforeEach(func() {
-		testDir, mb = setup()
-	})
-	AfterEach(func() {
-		teardown(originalHomeDir, testDir, mb)
-	})
 
 	It("machine info", func() {
 		info := new(infoMachine)

pkg/machine/e2e/init_test.go

@@ -20,18 +20,6 @@ import (
 )
 
 var _ = Describe("podman machine init", func() {
-	var (
-		mb      *machineTestBuilder
-		testDir string
-	)
-
-	BeforeEach(func() {
-		testDir, mb = setup()
-	})
-	AfterEach(func() {
-		teardown(originalHomeDir, testDir, mb)
-	})
-
 	cpus := runtime.NumCPU() / 2
 	if cpus == 0 {
 		cpus = 1

pkg/machine/e2e/init_windows_test.go

@@ -15,17 +15,6 @@ import (
 )
 
 var _ = Describe("podman machine init - windows only", func() {
-	var (
-		mb      *machineTestBuilder
-		testDir string
-	)
-
-	BeforeEach(func() {
-		testDir, mb = setup()
-	})
-	AfterEach(func() {
-		teardown(originalHomeDir, testDir, mb)
-	})
 
 	It("init with user mode networking", func() {
 		if testProvider.VMType() != define.WSLVirt {

pkg/machine/e2e/inspect_test.go

@@ -11,17 +11,6 @@ import (
 )
 
 var _ = Describe("podman inspect stop", func() {
-	var (
-		mb      *machineTestBuilder
-		testDir string
-	)
-
-	BeforeEach(func() {
-		testDir, mb = setup()
-	})
-	AfterEach(func() {
-		teardown(originalHomeDir, testDir, mb)
-	})
 
 	It("inspect bad name", func() {
 		i := inspectMachine{}

pkg/machine/e2e/list_test.go

@@ -14,17 +14,6 @@ import (
 )
 
 var _ = Describe("podman machine list", func() {
-	var (
-		mb      *machineTestBuilder
-		testDir string
-	)
-
-	BeforeEach(func() {
-		testDir, mb = setup()
-	})
-	AfterEach(func() {
-		teardown(originalHomeDir, testDir, mb)
-	})
 
 	It("list machine", func() {
 		list := new(listMachine)

pkg/machine/e2e/machine_test.go

@@ -131,14 +131,7 @@ func setup() (string, *machineTestBuilder) {
 	return homeDir, mb
 }
 
-func teardown(origHomeDir string, testDir string, mb *machineTestBuilder) {
-	r := new(rmMachine)
-	for _, name := range mb.names {
-		if _, err := mb.setName(name).setCmd(r.withForce()).run(); err != nil {
-			GinkgoWriter.Printf("error occurred rm'ing machine: %q\n", err)
-		}
-	}
-
+func teardown(origHomeDir string, testDir string) {
 	if err := utils.GuardedRemoveAll(testDir); err != nil {
 		Fail(fmt.Sprintf("failed to remove test dir: %q", err))
 	}
@@ -153,6 +146,18 @@ func teardown(origHomeDir string, testDir string, mb *machineTestBuilder) {
 	}
 }
 
+var (
+	mb      *machineTestBuilder
+	testDir string
+)
+
+var _ = BeforeEach(func() {
+	testDir, mb = setup()
+	DeferCleanup(func() {
+		teardown(originalHomeDir, testDir)
+	})
+})
+
 func setTmpDir(value string) (string, error) {
 	switch {
 	case runtime.GOOS != "darwin":

pkg/machine/e2e/os_test.go

@@ -7,17 +7,6 @@ package e2e_test
 // )
 
 // var _ = Describe("podman machine os apply", func() {
-// 	var (
-// 		mb      *machineTestBuilder
-// 		testDir string
-// 	)
-
-// 	BeforeEach(func() {
-// 		testDir, mb = setup()
-// 	})
-// 	AfterEach(func() {
-// 		teardown(originalHomeDir, testDir, mb)
-// 	})
 
 // 	It("apply machine", func() {
 // 		i := new(initMachine)

pkg/machine/e2e/proxy_test.go

@@ -11,17 +11,6 @@ import (
 )
 
 var _ = Describe("podman machine proxy settings propagation", func() {
-	var (
-		mb      *machineTestBuilder
-		testDir string
-	)
-
-	BeforeEach(func() {
-		testDir, mb = setup()
-	})
-	AfterEach(func() {
-		teardown(originalHomeDir, testDir, mb)
-	})
 
 	It("ssh to running machine and check proxy settings", func() {
 		defer func() {

pkg/machine/e2e/reset_test.go

@@ -7,17 +7,6 @@ import (
 )
 
 var _ = Describe("podman machine reset", func() {
-	var (
-		mb      *machineTestBuilder
-		testDir string
-	)
-
-	BeforeEach(func() {
-		testDir, mb = setup()
-	})
-	AfterEach(func() {
-		teardown(originalHomeDir, testDir, mb)
-	})
 
 	It("starting from scratch should not error", func() {
 		i := resetMachine{}

pkg/machine/e2e/rm_test.go

@@ -12,17 +12,6 @@ import (
 )
 
 var _ = Describe("podman machine rm", func() {
-	var (
-		mb      *machineTestBuilder
-		testDir string
-	)
-
-	BeforeEach(func() {
-		testDir, mb = setup()
-	})
-	AfterEach(func() {
-		teardown(originalHomeDir, testDir, mb)
-	})
 
 	It("bad init name", func() {
 		i := rmMachine{}

pkg/machine/e2e/set_test.go

@@ -13,17 +13,6 @@ import (
 )
 
 var _ = Describe("podman machine set", func() {
-	var (
-		mb      *machineTestBuilder
-		testDir string
-	)
-
-	BeforeEach(func() {
-		testDir, mb = setup()
-	})
-	AfterEach(func() {
-		teardown(originalHomeDir, testDir, mb)
-	})
 
 	It("set machine cpus, disk, memory", func() {
 		skipIfWSL("WSL cannot change set properties of disk, processor, or memory")

pkg/machine/e2e/ssh_test.go

@@ -8,17 +8,6 @@ import (
 )
 
 var _ = Describe("podman machine ssh", func() {
-	var (
-		mb      *machineTestBuilder
-		testDir string
-	)
-
-	BeforeEach(func() {
-		testDir, mb = setup()
-	})
-	AfterEach(func() {
-		teardown(originalHomeDir, testDir, mb)
-	})
 
 	It("bad machine name", func() {
 		name := randomString()

pkg/machine/e2e/start_test.go

@@ -14,16 +14,6 @@ import (
 )
 
 var _ = Describe("podman machine start", func() {
-	var (
-		mb      *machineTestBuilder
-		testDir string
-	)
-	BeforeEach(func() {
-		testDir, mb = setup()
-	})
-	AfterEach(func() {
-		teardown(originalHomeDir, testDir, mb)
-	})
 
 	It("start simple machine", func() {
 		i := new(initMachine)

pkg/machine/e2e/stop_test.go

@@ -10,17 +10,6 @@ import (
 )
 
 var _ = Describe("podman machine stop", func() {
-	var (
-		mb      *machineTestBuilder
-		testDir string
-	)
-
-	BeforeEach(func() {
-		testDir, mb = setup()
-	})
-	AfterEach(func() {
-		teardown(originalHomeDir, testDir, mb)
-	})
 
 	It("stop bad name", func() {
 		i := stopMachine{}