From fcca3155cb5a161437913671321bbe0663c105a3 Mon Sep 17 00:00:00 2001
From: Michael Beaumont <mjboamail@gmail.com>
Date: Mon, 29 Jun 2020 10:24:38 +0200
Subject: [PATCH] Fix multi process kubeconfig access support

---
 pkg/utils/kubeconfig/kubeconfig.go      | 40 ++++++++++++-------------
 pkg/utils/kubeconfig/kubeconfig_test.go | 27 ++++++++++-------
 2 files changed, 36 insertions(+), 31 deletions(-)

diff --git a/pkg/utils/kubeconfig/kubeconfig.go b/pkg/utils/kubeconfig/kubeconfig.go
index 88657fd33a..72bc0a69a7 100644
--- a/pkg/utils/kubeconfig/kubeconfig.go
+++ b/pkg/utils/kubeconfig/kubeconfig.go
@@ -180,6 +180,10 @@ func AppendAuthenticator(config *clientcmdapi.Config, clusterMeta *api.ClusterMe
 	}
 }
 
+func lockFileName(filePath string) string {
+	return filePath + ".eksctl.lock"
+}
+
 // ensureDirectory should probably be handled in flock
 func ensureDirectory(filePath string) error {
 	dir := filepath.Dir(filePath)
@@ -191,29 +195,25 @@ func ensureDirectory(filePath string) error {
 	return nil
 }
 
-func lockConfigFile(filePath string) error {
+func lockConfigFile(filePath string) (*flock.Flock, error) {
+	lockFileName := lockFileName(filePath)
 	// Make sure the directory exists, otherwise flock fails
-	if err := ensureDirectory(filePath); err != nil {
-		return err
+	if err := ensureDirectory(lockFileName); err != nil {
+		return nil, err
 	}
-	flock := flock.New(filePath)
+	flock := flock.New(lockFileName)
 	err := flock.Lock()
 	if err != nil {
-		return errors.Wrap(err, "flock: failed to obtain exclusive lock on kubeconfig file")
+		return nil, errors.Wrap(err, "failed to obtain exclusive lock on kubeconfig lockfile")
 	}
 
-	return nil
+	return flock, nil
 }
 
-func unlockConfigFile(filePath string) error {
-	// Make sure the directory exists, otherwise flock fails
-	if err := ensureDirectory(filePath); err != nil {
-		return err
-	}
-	flock := flock.New(filePath)
-	err := flock.Unlock()
+func unlockConfigFile(fl *flock.Flock) error {
+	err := fl.Unlock()
 	if err != nil {
-		return errors.Wrap(err, "flock: failed to release exclusive lock on kubeconfig file")
+		return errors.Wrap(err, "failed to release exclusive lock on kubeconfig lockfile")
 	}
 
 	return nil
@@ -226,13 +226,13 @@ func unlockConfigFile(filePath string) error {
 func Write(path string, newConfig clientcmdapi.Config, setContext bool) (string, error) {
 	configAccess := getConfigAccess(path)
 	configFileName := configAccess.GetDefaultFilename()
-	err := lockConfigFile(configFileName)
+	fl, err := lockConfigFile(configFileName)
 	if err != nil {
 		return "", err
 	}
 
 	defer func() {
-		if err := unlockConfigFile(configFileName); err != nil {
+		if err := unlockConfigFile(fl); err != nil {
 			logger.Critical(err.Error())
 		}
 	}()
@@ -314,13 +314,13 @@ func MaybeDeleteConfig(meta *api.ClusterMeta) {
 	p := AutoPath(meta.Name)
 
 	if file.Exists(p) {
-		err := lockConfigFile(p)
+		fl, err := lockConfigFile(p)
 		if err != nil {
 			logger.Critical(err.Error())
 		}
 
 		defer func() {
-			if err := unlockConfigFile(p); err != nil {
+			if err := unlockConfigFile(fl); err != nil {
 				logger.Critical(err.Error())
 			}
 		}()
@@ -337,13 +337,13 @@ func MaybeDeleteConfig(meta *api.ClusterMeta) {
 
 	configAccess := getConfigAccess(DefaultPath())
 	defaultFilename := configAccess.GetDefaultFilename()
-	err := lockConfigFile(defaultFilename)
+	fl, err := lockConfigFile(defaultFilename)
 	if err != nil {
 		logger.Critical(err.Error())
 	}
 
 	defer func() {
-		if err := unlockConfigFile(defaultFilename); err != nil {
+		if err := unlockConfigFile(fl); err != nil {
 			logger.Critical(err.Error())
 		}
 	}()
diff --git a/pkg/utils/kubeconfig/kubeconfig_test.go b/pkg/utils/kubeconfig/kubeconfig_test.go
index c8d50c0a37..b46e3e17ed 100644
--- a/pkg/utils/kubeconfig/kubeconfig_test.go
+++ b/pkg/utils/kubeconfig/kubeconfig_test.go
@@ -312,35 +312,40 @@ var _ = Describe("Kubeconfig", func() {
 
 	It("safely handles concurrent read-modify-write operations", func() {
 		var (
-			oneClusterAsBytes  []byte
-			twoClustersAsBytes []byte
+			oneCluster  *api.Config
+			twoClusters *api.Config
 		)
 		ChangeKubeconfig()
 
 		var err error
+		tmp, err := ioutil.TempFile("", "")
+		Expect(err).To(BeNil())
 
-		if oneClusterAsBytes, err = ioutil.ReadFile("testdata/one_cluster.golden"); err != nil {
-			GinkgoT().Fatalf("failed reading .golden: %v", err)
-		}
+		{
+			if oneCluster, err = clientcmd.LoadFromFile("testdata/one_cluster.golden"); err != nil {
+				GinkgoT().Fatalf("failed reading .golden: %v", err)
+			}
 
-		if twoClustersAsBytes, err = ioutil.ReadFile("testdata/two_clusters.golden"); err != nil {
-			GinkgoT().Fatalf("failed reading .golden: %v", err)
+			if twoClusters, err = clientcmd.LoadFromFile("testdata/two_clusters.golden"); err != nil {
+				GinkgoT().Fatalf("failed reading .golden: %v", err)
+			}
 		}
 
 		var wg sync.WaitGroup
 		multiplier := 3
-		iters := 100
+		iters := 10
 		for i := 0; i < multiplier; i++ {
 			for k := 0; k < iters; k++ {
-				wg.Add(2)
+				wg.Add(1)
 				go func() {
 					defer wg.Done()
-					_, err := configFile.Write(oneClusterAsBytes)
+					_, err := kubeconfig.Write(tmp.Name(), *oneCluster, false)
 					Expect(err).To(BeNil())
 				}()
+				wg.Add(1)
 				go func() {
 					defer wg.Done()
-					_, err := configFile.Write(twoClustersAsBytes)
+					_, err := kubeconfig.Write(tmp.Name(), *twoClusters, false)
 					Expect(err).To(BeNil())
 				}()
 			}