From 00576278b520ee1c1fb5e86d4ba14501956fd633 Mon Sep 17 00:00:00 2001 From: CJ Cullen Date: Wed, 29 Jun 2016 11:29:33 -0700 Subject: [PATCH] Lock all possible kubecfg files at the beginning of ModifyConfig. --- pkg/client/unversioned/clientcmd/config.go | 85 ++++++++++++++++++---- pkg/client/unversioned/clientcmd/loader.go | 14 ++-- 2 files changed, 77 insertions(+), 22 deletions(-) diff --git a/pkg/client/unversioned/clientcmd/config.go b/pkg/client/unversioned/clientcmd/config.go index 049fc39213c32..ec5948609f194 100644 --- a/pkg/client/unversioned/clientcmd/config.go +++ b/pkg/client/unversioned/clientcmd/config.go @@ -22,6 +22,7 @@ import ( "path" "path/filepath" "reflect" + "sort" "github.com/golang/glog" @@ -153,6 +154,17 @@ func NewDefaultPathOptions() *PathOptions { // that means that this code will only write into a single file. If you want to relativizePaths, you must provide a fully qualified path in any // modified element. func ModifyConfig(configAccess ConfigAccess, newConfig clientcmdapi.Config, relativizePaths bool) error { + possibleSources := configAccess.GetLoadingPrecedence() + // sort the possible kubeconfig files so we always "lock" in the same order + // to avoid deadlock (note: this can fail w/ symlinks, but... come on). + sort.Strings(possibleSources) + for _, filename := range possibleSources { + if err := lockFile(filename); err != nil { + return err + } + defer unlockFile(filename) + } + startingConfig, err := configAccess.GetStartingConfig() if err != nil { return err @@ -186,7 +198,10 @@ func ModifyConfig(configAccess ConfigAccess, newConfig clientcmdapi.Config, rela destinationFile = configAccess.GetDefaultFilename() } - configToWrite := GetConfigFromFileOrDie(destinationFile) + configToWrite, err := getConfigFromFile(destinationFile) + if err != nil { + return err + } t := *cluster configToWrite.Clusters[key] = &t @@ -211,7 +226,10 @@ func ModifyConfig(configAccess ConfigAccess, newConfig clientcmdapi.Config, rela destinationFile = configAccess.GetDefaultFilename() } - configToWrite := GetConfigFromFileOrDie(destinationFile) + configToWrite, err := getConfigFromFile(destinationFile) + if err != nil { + return err + } configToWrite.Contexts[key] = context if err := WriteToFile(*configToWrite, destinationFile); err != nil { @@ -228,7 +246,10 @@ func ModifyConfig(configAccess ConfigAccess, newConfig clientcmdapi.Config, rela destinationFile = configAccess.GetDefaultFilename() } - configToWrite := GetConfigFromFileOrDie(destinationFile) + configToWrite, err := getConfigFromFile(destinationFile) + if err != nil { + return err + } t := *authInfo configToWrite.AuthInfos[key] = &t configToWrite.AuthInfos[key].LocationOfOrigin = destinationFile @@ -251,7 +272,10 @@ func ModifyConfig(configAccess ConfigAccess, newConfig clientcmdapi.Config, rela destinationFile = configAccess.GetDefaultFilename() } - configToWrite := GetConfigFromFileOrDie(destinationFile) + configToWrite, err := getConfigFromFile(destinationFile) + if err != nil { + return err + } delete(configToWrite.Clusters, key) if err := WriteToFile(*configToWrite, destinationFile); err != nil { @@ -267,7 +291,10 @@ func ModifyConfig(configAccess ConfigAccess, newConfig clientcmdapi.Config, rela destinationFile = configAccess.GetDefaultFilename() } - configToWrite := GetConfigFromFileOrDie(destinationFile) + configToWrite, err := getConfigFromFile(destinationFile) + if err != nil { + return err + } delete(configToWrite.Contexts, key) if err := WriteToFile(*configToWrite, destinationFile); err != nil { @@ -283,7 +310,10 @@ func ModifyConfig(configAccess ConfigAccess, newConfig clientcmdapi.Config, rela destinationFile = configAccess.GetDefaultFilename() } - configToWrite := GetConfigFromFileOrDie(destinationFile) + configToWrite, err := getConfigFromFile(destinationFile) + if err != nil { + return err + } delete(configToWrite.AuthInfos, key) if err := WriteToFile(*configToWrite, destinationFile); err != nil { @@ -330,7 +360,10 @@ func writeCurrentContext(configAccess ConfigAccess, newCurrentContext string) er if configAccess.IsExplicitFile() { file := configAccess.GetExplicitFile() - currConfig := GetConfigFromFileOrDie(file) + currConfig, err := getConfigFromFile(file) + if err != nil { + return err + } currConfig.CurrentContext = newCurrentContext if err := WriteToFile(*currConfig, file); err != nil { return err @@ -341,7 +374,10 @@ func writeCurrentContext(configAccess ConfigAccess, newCurrentContext string) er if len(newCurrentContext) > 0 { destinationFile := configAccess.GetDefaultFilename() - config := GetConfigFromFileOrDie(destinationFile) + config, err := getConfigFromFile(destinationFile) + if err != nil { + return err + } config.CurrentContext = newCurrentContext if err := WriteToFile(*config, destinationFile); err != nil { @@ -354,7 +390,10 @@ func writeCurrentContext(configAccess ConfigAccess, newCurrentContext string) er // we're supposed to be clearing the current context. We need to find the first spot in the chain that is setting it and clear it for _, file := range configAccess.GetLoadingPrecedence() { if _, err := os.Stat(file); err == nil { - currConfig := GetConfigFromFileOrDie(file) + currConfig, err := getConfigFromFile(file) + if err != nil { + return err + } if len(currConfig.CurrentContext) > 0 { currConfig.CurrentContext = newCurrentContext @@ -379,7 +418,10 @@ func writePreferences(configAccess ConfigAccess, newPrefs clientcmdapi.Preferenc if configAccess.IsExplicitFile() { file := configAccess.GetExplicitFile() - currConfig := GetConfigFromFileOrDie(file) + currConfig, err := getConfigFromFile(file) + if err != nil { + return err + } currConfig.Preferences = newPrefs if err := WriteToFile(*currConfig, file); err != nil { return err @@ -389,7 +431,10 @@ func writePreferences(configAccess ConfigAccess, newPrefs clientcmdapi.Preferenc } for _, file := range configAccess.GetLoadingPrecedence() { - currConfig := GetConfigFromFileOrDie(file) + currConfig, err := getConfigFromFile(file) + if err != nil { + return err + } if !reflect.DeepEqual(currConfig.Preferences, newPrefs) { currConfig.Preferences = newPrefs @@ -404,15 +449,23 @@ func writePreferences(configAccess ConfigAccess, newPrefs clientcmdapi.Preferenc return errors.New("no config found to write preferences") } -// GetConfigFromFileOrDie tries to read a kubeconfig file and if it can't, it calls exit. One exception, missing files result in empty configs, not an exit -func GetConfigFromFileOrDie(filename string) *clientcmdapi.Config { +// getConfigFromFile tries to read a kubeconfig file and if it can't, returns an error. One exception, missing files result in empty configs, not an error. +func getConfigFromFile(filename string) (*clientcmdapi.Config, error) { config, err := LoadFromFile(filename) if err != nil && !os.IsNotExist(err) { - glog.FatalDepth(1, err) + return nil, err } - if config == nil { - return clientcmdapi.NewConfig() + config = clientcmdapi.NewConfig() + } + return config, nil +} + +// GetConfigFromFileOrDie tries to read a kubeconfig file and if it can't, it calls exit. One exception, missing files result in empty configs, not an exit +func GetConfigFromFileOrDie(filename string) *clientcmdapi.Config { + config, err := getConfigFromFile(filename) + if err != nil { + glog.FatalDepth(1, err) } return config diff --git a/pkg/client/unversioned/clientcmd/loader.go b/pkg/client/unversioned/clientcmd/loader.go index 3d2df1f7da3a4..8988355de65bc 100644 --- a/pkg/client/unversioned/clientcmd/loader.go +++ b/pkg/client/unversioned/clientcmd/loader.go @@ -382,12 +382,6 @@ func WriteToFile(config clientcmdapi.Config, filename string) error { } } - err = lockFile(filename) - if err != nil { - return err - } - defer unlockFile(filename) - if err := ioutil.WriteFile(filename, content, 0600); err != nil { return err } @@ -397,6 +391,14 @@ func WriteToFile(config clientcmdapi.Config, filename string) error { func lockFile(filename string) error { // TODO: find a way to do this with actual file locks. Will // probably need seperate solution for windows and linux. + + // Make sure the dir exists before we try to create a lock file. + dir := filepath.Dir(filename) + if _, err := os.Stat(dir); os.IsNotExist(err) { + if err = os.MkdirAll(dir, 0755); err != nil { + return err + } + } f, err := os.OpenFile(lockName(filename), os.O_CREATE|os.O_EXCL, 0) if err != nil { return err