Move testing lock from server creation to test start (#5155)

Signed-off-by: Derek Nola <derek.nola@suse.com>

tests/integration/custometcdargs/custometcdargs_int_test.go

@@ -15,9 +15,13 @@ var customEtcdArgsServerArgs = []string{
 	"--cluster-init",
 	"--etcd-arg quota-backend-bytes=858993459",
 }
+var testLock int
+
 var _ = BeforeSuite(func() {
 	if !testutil.IsExistingServer() {
 		var err error
+		testLock, err = testutil.K3sTestLock()
+		Expect(err).ToNot(HaveOccurred())
 		customEtcdArgsServer, err = testutil.K3sStartServer(customEtcdArgsServerArgs...)
 		Expect(err).ToNot(HaveOccurred())
 	}
@@ -50,8 +54,8 @@ var _ = Describe("custom etcd args", func() {
 
 var _ = AfterSuite(func() {
 	if !testutil.IsExistingServer() {
-		Expect(testutil.K3sKillServer(customEtcdArgsServer, false)).To(Succeed())
+		Expect(testutil.K3sKillServer(customEtcdArgsServer)).To(Succeed())
-		Expect(testutil.K3sCleanup(customEtcdArgsServer, true, "")).To(Succeed())
+		Expect(testutil.K3sCleanup(testLock, "")).To(Succeed())
 	}
 })
 

tests/integration/dualstack/dualstack_int_test.go

@@ -17,9 +17,13 @@ var dualStackServerArgs = []string{
 	"--service-cidr 10.43.0.0/16,2001:cafe:42:1::/112",
 	"--disable-network-policy",
 }
+var testLock int
+
 var _ = BeforeSuite(func() {
 	if !testutil.IsExistingServer() && os.Getenv("CI") != "true" {
 		var err error
+		testLock, err = testutil.K3sTestLock()
+		Expect(err).ToNot(HaveOccurred())
 		dualStackServer, err = testutil.K3sStartServer(dualStackServerArgs...)
 		Expect(err).ToNot(HaveOccurred())
 	}
@@ -51,8 +55,8 @@ var _ = Describe("dual stack", func() {
 
 var _ = AfterSuite(func() {
 	if !testutil.IsExistingServer() && os.Getenv("CI") != "true" {
-		Expect(testutil.K3sKillServer(dualStackServer, false)).To(Succeed())
+		Expect(testutil.K3sKillServer(dualStackServer)).To(Succeed())
-		Expect(testutil.K3sCleanup(dualStackServer, true, "")).To(Succeed())
+		Expect(testutil.K3sCleanup(testLock, "")).To(Succeed())
 	}
 })
 

tests/integration/etcdrestore/etcd_restore_int_test.go

@@ -12,10 +12,13 @@ import (
 var server1, server2 *testutil.K3sServer
 var tmpdDataDir = "/tmp/restoredatadir"
 var clientCACertHash string
+var testLock int
 var restoreServerArgs = []string{"--cluster-init", "-t", "test", "-d", tmpdDataDir}
 var _ = BeforeSuite(func() {
 	if !testutil.IsExistingServer() {
 		var err error
+		testLock, err = testutil.K3sTestLock()
+		Expect(err).ToNot(HaveOccurred())
 		server1, err = testutil.K3sStartServer(restoreServerArgs...)
 		Expect(err).ToNot(HaveOccurred())
 	}
@@ -59,7 +62,7 @@ var _ = Describe("etcd snapshot restore", func() {
 			Expect(err).ToNot(HaveOccurred())
 		})
 		It("stop k3s", func() {
-			Expect(testutil.K3sKillServer(server1, true)).To(Succeed())
+			Expect(testutil.K3sKillServer(server1)).To(Succeed())
 		})
 		It("restore the snapshot", func() {
 			// get snapshot file
@@ -98,17 +101,16 @@ var _ = Describe("etcd snapshot restore", func() {
 			Expect(clientCACertHash2).To(Equal(clientCACertHash))
 		})
 		It("stop k3s", func() {
-			Expect(testutil.K3sKillServer(server2, false)).To(Succeed())
+			Expect(testutil.K3sKillServer(server2)).To(Succeed())
 		})
 	})
 })
 
 var _ = AfterSuite(func() {
 	if !testutil.IsExistingServer() {
-		Expect(testutil.K3sKillServer(server1, false)).To(Succeed())
+		Expect(testutil.K3sKillServer(server1)).To(Succeed())
-		Expect(testutil.K3sCleanup(server1, true, tmpdDataDir)).To(Succeed())
+		Expect(testutil.K3sKillServer(server2)).To(Succeed())
-		Expect(testutil.K3sKillServer(server2, false)).To(Succeed())
+		Expect(testutil.K3sCleanup(testLock, tmpdDataDir)).To(Succeed())
-		Expect(testutil.K3sCleanup(server2, true, tmpdDataDir)).To(Succeed())
 	}
 })
 

tests/integration/etcdsnapshot/etcdsnapshot_int_test.go

@@ -13,9 +13,13 @@ import (
 
 var server *testutil.K3sServer
 var serverArgs = []string{"--cluster-init"}
+var testLock int
+
 var _ = BeforeSuite(func() {
 	if !testutil.IsExistingServer() {
 		var err error
+		testLock, err = testutil.K3sTestLock()
+		Expect(err).ToNot(HaveOccurred())
 		server, err = testutil.K3sStartServer(serverArgs...)
 		Expect(err).ToNot(HaveOccurred())
 	}
@@ -111,8 +115,8 @@ var _ = Describe("etcd snapshots", func() {
 
 var _ = AfterSuite(func() {
 	if !testutil.IsExistingServer() {
-		Expect(testutil.K3sKillServer(server, false)).To(Succeed())
+		Expect(testutil.K3sKillServer(server)).To(Succeed())
-		Expect(testutil.K3sCleanup(server, true, "")).To(Succeed())
+		Expect(testutil.K3sCleanup(testLock, "")).To(Succeed())
 	}
 })
 

tests/integration/localstorage/localstorage_int_test.go

@@ -14,9 +14,13 @@ import (
 
 var localStorageServer *testutil.K3sServer
 var localStorageServerArgs = []string{"--cluster-init"}
+var testLock int
+
 var _ = BeforeSuite(func() {
 	if !testutil.IsExistingServer() {
 		var err error
+		testLock, err = testutil.K3sTestLock()
+		Expect(err).ToNot(HaveOccurred())
 		localStorageServer, err = testutil.K3sStartServer(localStorageServerArgs...)
 		Expect(err).ToNot(HaveOccurred())
 	}
@@ -80,8 +84,8 @@ var _ = Describe("local storage", func() {
 
 var _ = AfterSuite(func() {
 	if !testutil.IsExistingServer() {
-		Expect(testutil.K3sKillServer(localStorageServer, false)).To(Succeed())
+		Expect(testutil.K3sKillServer(localStorageServer)).To(Succeed())
-		Expect(testutil.K3sCleanup(localStorageServer, true, "")).To(Succeed())
+		Expect(testutil.K3sCleanup(testLock, "")).To(Succeed())
 	}
 })
 

tests/integration/secretsencryption/secretsencryption_int_test.go

@@ -2,7 +2,6 @@ package integration
 
 import (
 	"fmt"
-	"os"
 	"regexp"
 	"testing"
 	"time"
@@ -14,12 +13,14 @@ import (
 
 var secretsEncryptionServer *testutil.K3sServer
 var secretsEncryptionDataDir = "/tmp/k3sse"
-
 var secretsEncryptionServerArgs = []string{"--secrets-encryption", "-d", secretsEncryptionDataDir}
+var testLock int
+
 var _ = BeforeSuite(func() {
 	if !testutil.IsExistingServer() {
 		var err error
-		Expect(os.MkdirAll(secretsEncryptionDataDir, 0777)).To(Succeed())
+		testLock, err = testutil.K3sTestLock()
+		Expect(err).ToNot(HaveOccurred())
 		secretsEncryptionServer, err = testutil.K3sStartServer(secretsEncryptionServerArgs...)
 		Expect(err).ToNot(HaveOccurred())
 	}
@@ -61,7 +62,7 @@ var _ = Describe("secrets encryption rotation", func() {
 		})
 		It("restarts the server", func() {
 			var err error
-			Expect(testutil.K3sKillServer(secretsEncryptionServer, true)).To(Succeed())
+			Expect(testutil.K3sKillServer(secretsEncryptionServer)).To(Succeed())
 			secretsEncryptionServer, err = testutil.K3sStartServer(secretsEncryptionServerArgs...)
 			Expect(err).ToNot(HaveOccurred())
 			Eventually(func() (string, error) {
@@ -85,7 +86,7 @@ var _ = Describe("secrets encryption rotation", func() {
 		})
 		It("restarts the server", func() {
 			var err error
-			Expect(testutil.K3sKillServer(secretsEncryptionServer, true)).To(Succeed())
+			Expect(testutil.K3sKillServer(secretsEncryptionServer)).To(Succeed())
 			secretsEncryptionServer, err = testutil.K3sStartServer(secretsEncryptionServerArgs...)
 			Expect(err).ToNot(HaveOccurred())
 			Eventually(func() (string, error) {
@@ -119,7 +120,7 @@ var _ = Describe("secrets encryption rotation", func() {
 		})
 		It("restarts the server", func() {
 			var err error
-			Expect(testutil.K3sKillServer(secretsEncryptionServer, true)).To(Succeed())
+			Expect(testutil.K3sKillServer(secretsEncryptionServer)).To(Succeed())
 			secretsEncryptionServer, err = testutil.K3sStartServer(secretsEncryptionServerArgs...)
 			Expect(err).ToNot(HaveOccurred())
 			Eventually(func() (string, error) {
@@ -141,8 +142,8 @@ var _ = Describe("secrets encryption rotation", func() {
 
 var _ = AfterSuite(func() {
 	if !testutil.IsExistingServer() {
-		Expect(testutil.K3sKillServer(secretsEncryptionServer, true)).To(Succeed())
+		Expect(testutil.K3sKillServer(secretsEncryptionServer)).To(Succeed())
-		Expect(os.RemoveAll(secretsEncryptionDataDir)).To(Succeed())
+		Expect(testutil.K3sCleanup(testLock, secretsEncryptionDataDir)).To(Succeed())
 	}
 })
 

tests/util/integration.go

@@ -25,7 +25,6 @@ const lockFile = "/tmp/k3s-test.lock"
 type K3sServer struct {
 	cmd     *exec.Cmd
 	scanner *bufio.Scanner
-	lock    int
 }
 
 func findK3sExecutable() string {
@@ -130,6 +129,11 @@ func FindStringInCmdAsync(scanner *bufio.Scanner, target string) bool {
 	return false
 }
 
+func K3sTestLock() (int, error) {
+	logrus.Info("waiting to get test lock")
+	return flock.Acquire(lockFile)
+}
+
 // K3sStartServer acquires an exclusive lock on a temporary file, then launches a k3s cluster
 // with the provided arguments. Subsequent/parallel calls to this function will block until
 // the original lock is cleared using K3sKillServer
@@ -138,12 +142,6 @@ func K3sStartServer(inputArgs ...string) (*K3sServer, error) {
 		return nil, errors.New("integration tests must be run as sudo/root")
 	}
 
-	logrus.Info("waiting to get server lock")
-	k3sLock, err := flock.Acquire(lockFile)
-	if err != nil {
-		return nil, err
-	}
-
 	var cmdArgs []string
 	for _, arg := range inputArgs {
 		cmdArgs = append(cmdArgs, strings.Fields(arg)...)
@@ -155,13 +153,13 @@ func K3sStartServer(inputArgs ...string) (*K3sServer, error) {
 	cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
 	cmdOut, _ := cmd.StderrPipe()
 	cmd.Stderr = os.Stderr
-	err = cmd.Start()
+	err := cmd.Start()
-	return &K3sServer{cmd, bufio.NewScanner(cmdOut), k3sLock}, err
+	return &K3sServer{cmd, bufio.NewScanner(cmdOut)}, err
 }
 
 // K3sKillServer terminates the running K3s server and its children
 // and unlocks the file for other tests
-func K3sKillServer(server *K3sServer, releaseLock bool) error {
+func K3sKillServer(server *K3sServer) error {
 	pgid, err := syscall.Getpgid(server.cmd.Process.Pid)
 	if err != nil {
 		return err
@@ -169,17 +167,11 @@ func K3sKillServer(server *K3sServer, releaseLock bool) error {
 	if err := syscall.Kill(-pgid, syscall.SIGKILL); err != nil {
 		return err
 	}
-	if err := server.cmd.Process.Kill(); err != nil {
+	return server.cmd.Process.Kill()
-		return err
-	}
-	if releaseLock {
-		return flock.Release(server.lock)
-	}
-	return nil
 }
 
 // K3sCleanup attempts to cleanup networking and files leftover from an integration test
-func K3sCleanup(server *K3sServer, releaseLock bool, dataDir string) error {
+func K3sCleanup(k3sTestLock int, dataDir string) error {
 	if cni0Link, err := netlink.LinkByName("cni0"); err == nil {
 		links, _ := netlink.LinkList()
 		for _, link := range links {
@@ -202,10 +194,7 @@ func K3sCleanup(server *K3sServer, releaseLock bool, dataDir string) error {
 	if err := os.RemoveAll(dataDir); err != nil {
 		return err
 	}
-	if releaseLock {
+	return flock.Release(k3sTestLock)
-		return flock.Release(server.lock)
-	}
-	return nil
 }
 
 // RunCommand Runs command on the cluster accessing the cluster through kubeconfig file