Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lock all possible kubecfg files at the beginning of ModifyConfig. #28232

Merged
merged 1 commit into from
Jun 30, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 69 additions & 16 deletions pkg/client/unversioned/clientcmd/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"path"
"path/filepath"
"reflect"
"sort"

"github.com/golang/glog"

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
14 changes: 8 additions & 6 deletions pkg/client/unversioned/clientcmd/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
Expand Down