From 0d9d5db68e83a58f76be717d31962c0a9475a422 Mon Sep 17 00:00:00 2001 From: juanvallejo Date: Tue, 7 Aug 2018 14:58:16 -0400 Subject: [PATCH] UPSTREAM: 67093: improve config file modification time Trades runtime complexity for spacial complexity when modifying large amounts of contexts on a kubeconfig. In cases where there are few destination filenames for a given amount of contexts, but a large amount of contexts, this patch prevents reading and writing to the same file (or small number of files) over and over again needlessly. --- .../tools/clientcmd/client_config_test.go | 48 +++++++++++++++++++ .../client-go/tools/clientcmd/config.go | 28 ++++++++--- 2 files changed, 70 insertions(+), 6 deletions(-) diff --git a/vendor/k8s.io/kubernetes/staging/src/k8s.io/client-go/tools/clientcmd/client_config_test.go b/vendor/k8s.io/kubernetes/staging/src/k8s.io/client-go/tools/clientcmd/client_config_test.go index 0a9288bcf989..a209da42f26f 100644 --- a/vendor/k8s.io/kubernetes/staging/src/k8s.io/client-go/tools/clientcmd/client_config_test.go +++ b/vendor/k8s.io/kubernetes/staging/src/k8s.io/client-go/tools/clientcmd/client_config_test.go @@ -126,6 +126,54 @@ func TestMergeContext(t *testing.T) { matchStringArg(namespace, actual, t) } +func TestModifyContext(t *testing.T) { + expectedCtx := map[string]bool{ + "updated": true, + "clean": true, + } + + tempPath, err := ioutil.TempFile("", "testclientcmd-") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + defer os.Remove(tempPath.Name()) + + pathOptions := NewDefaultPathOptions() + config := createValidTestConfig() + + pathOptions.GlobalFile = tempPath.Name() + + // define new context and assign it - our path options config + config.Contexts["updated"] = &clientcmdapi.Context{ + Cluster: "updated", + AuthInfo: "updated", + } + config.CurrentContext = "updated" + + if err := ModifyConfig(pathOptions, *config, true); err != nil { + t.Errorf("Unexpected error: %v", err) + } + + startingConfig, err := pathOptions.GetStartingConfig() + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + // make sure the current context was updated + matchStringArg("updated", startingConfig.CurrentContext, t) + + // there should now be two contexts + if len(startingConfig.Contexts) != len(expectedCtx) { + t.Fatalf("unexpected nuber of contexts, expecting %v, but found %v", len(expectedCtx), len(startingConfig.Contexts)) + } + + for key := range startingConfig.Contexts { + if !expectedCtx[key] { + t.Fatalf("expected context %q to exist", key) + } + } +} + func TestCertificateData(t *testing.T) { caData := []byte("ca-data") certData := []byte("cert-data") diff --git a/vendor/k8s.io/kubernetes/staging/src/k8s.io/client-go/tools/clientcmd/config.go b/vendor/k8s.io/kubernetes/staging/src/k8s.io/client-go/tools/clientcmd/config.go index b508c04e181d..c6b970ca0039 100644 --- a/vendor/k8s.io/kubernetes/staging/src/k8s.io/client-go/tools/clientcmd/config.go +++ b/vendor/k8s.io/kubernetes/staging/src/k8s.io/client-go/tools/clientcmd/config.go @@ -208,6 +208,9 @@ func ModifyConfig(configAccess ConfigAccess, newConfig clientcmdapi.Config, rela } } + // seenConfigs stores a map of config source filenames to computed config objects. + seenConfigs := map[string]*clientcmdapi.Config{} + for key, context := range newConfig.Contexts { startingContext, exists := startingConfig.Contexts[key] if !reflect.DeepEqual(context, startingContext) || !exists { @@ -216,15 +219,28 @@ func ModifyConfig(configAccess ConfigAccess, newConfig clientcmdapi.Config, rela destinationFile = configAccess.GetDefaultFilename() } - configToWrite, err := getConfigFromFile(destinationFile) - if err != nil { - return err + // we only obtain a fresh config object from its source file + // if we have not seen it already - this prevents us from + // reading and writing to the same number of files repeatedly + // when multiple / all contexts share the same destination file. + configToWrite, seen := seenConfigs[destinationFile] + if !seen { + var err error + configToWrite, err = getConfigFromFile(destinationFile) + if err != nil { + return err + } + seenConfigs[destinationFile] = configToWrite } + configToWrite.Contexts[key] = context + } + } - if err := WriteToFile(*configToWrite, destinationFile); err != nil { - return err - } + // actually persist config object changes + for destinationFile, configToWrite := range seenConfigs { + if err := WriteToFile(*configToWrite, destinationFile); err != nil { + return err } }