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 <[email protected]>

Author: Luap99

Diff for: 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

Diff for: 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
 }

Diff for: 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)

Diff for: 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

Diff for: 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 {

Diff for: 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{}

Diff for: 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)

Diff for: 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":

Diff for: 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)

Diff for: 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() {

Diff for: 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{}

Diff for: 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{}

Diff for: 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")

Diff for: 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()

Diff for: 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)

Diff for: 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{}