From d60664df78daeb58f38cf80a08822086d389ed65 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Fri, 9 Aug 2019 11:06:19 -0700 Subject: [PATCH] refactor fix tests --- cmd/minikube/cmd/start.go | 33 +++- pkg/minikube/bootstrapper/certs.go | 2 +- pkg/minikube/kubeconfig/context.go | 34 ++-- pkg/minikube/kubeconfig/context_test.go | 44 ++--- pkg/minikube/kubeconfig/kcs.go | 38 +++- pkg/minikube/kubeconfig/kubeconfig.go | 2 +- pkg/minikube/kubeconfig/kubeconfig_test.go | 18 +- pkg/minikube/kubeconfig/update.go | 193 --------------------- 8 files changed, 114 insertions(+), 250 deletions(-) delete mode 100644 pkg/minikube/kubeconfig/update.go diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index c99d8ef41..27f0a9f1f 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -330,12 +330,31 @@ func runStart(cmd *cobra.Command, args []string) { } -func setupKubeconfig(host *host.Host, config *cfg.Config) (*kubeconfig.KCS, error) { - addr, err := host.Driver.GetURL() +func setupKubeconfig(h *host.Host, c *cfg.Config) (*kubeconfig.KCS, error) { + addr, err := h.Driver.GetURL() if err != nil { - exit.WithError("Failed to get host URL", err) + exit.WithError("Failed to get driver URL", err) } - return kubeconfig.Setup(addr, config) + addr = strings.Replace(addr, "tcp://", "https://", -1) + addr = strings.Replace(addr, ":2376", ":"+strconv.Itoa(c.KubernetesConfig.NodePort), -1) + if c.KubernetesConfig.APIServerName != constants.APIServerName { + addr = strings.Replace(addr, c.KubernetesConfig.NodeIP, c.KubernetesConfig.APIServerName, -1) + } + + kcs := &kubeconfig.KCS{ + ClusterName: cfg.GetMachineName(), + ClusterServerAddress: addr, + ClientCertificate: constants.MakeMiniPath("client.crt"), + ClientKey: constants.MakeMiniPath("client.key"), + CertificateAuthority: constants.MakeMiniPath("ca.crt"), + KeepContext: viper.GetBool(keepContext), + EmbedCerts: viper.GetBool(embedCerts), + } + kcs.SetPath(kubeconfig.PathFromEnv()) + if err := kubeconfig.Update(kcs); err != nil { + return kcs, err + } + return kcs, nil } func handleDownloadOnly(cacheGroup *errgroup.Group, k8sVersion string) { @@ -415,9 +434,9 @@ func showVersionInfo(k8sVersion string, cr cruntime.Manager) { } } -func showKubectlConnectInfo(kubeconfig *kubeconfig.KCS) { - if kubeconfig.KeepContext { - out.T(out.Kubectl, "To connect to this cluster, use: kubectl --context={{.name}}", out.V{"name": kubeconfig.ClusterName}) +func showKubectlConnectInfo(kcs *kubeconfig.KCS) { + if kcs.KeepContext { + out.T(out.Kubectl, "To connect to this cluster, use: kubectl --context={{.name}}", out.V{"name": kcs.ClusterName}) } else { out.T(out.Ready, `Done! kubectl is now configured to use "{{.name}}"`, out.V{"name": cfg.GetMachineName()}) } diff --git a/pkg/minikube/bootstrapper/certs.go b/pkg/minikube/bootstrapper/certs.go index adaadc732..4e0fb8ce0 100644 --- a/pkg/minikube/bootstrapper/certs.go +++ b/pkg/minikube/bootstrapper/certs.go @@ -77,7 +77,7 @@ func SetupCerts(cmd command.Runner, k8s config.KubernetesConfig) error { } kubeCfg := api.NewConfig() - err := kubeconfig.Populate(kcs, kubeCfg) + err := kubeconfig.PopulateFromSetup(kcs, kubeCfg) if err != nil { return errors.Wrap(err, "populating kubeconfig") } diff --git a/pkg/minikube/kubeconfig/context.go b/pkg/minikube/kubeconfig/context.go index f62e45def..f082acdc4 100644 --- a/pkg/minikube/kubeconfig/context.go +++ b/pkg/minikube/kubeconfig/context.go @@ -23,16 +23,20 @@ import ( ) // UnsetCurrentContext unsets the current-context from minikube to "" on minikube stop -func UnsetCurrentContext(filename, machineName string) error { - confg, err := readOrNew(filename) +func UnsetCurrentContext(machineName string, configPath ...string) error { + fPath := PathFromEnv() + if configPath != nil { + fPath = configPath[0] + } + kCfg, err := readOrNew(fPath) if err != nil { return errors.Wrap(err, "Error getting kubeconfig status") } // Unset current-context only if profile is the current-context - if confg.CurrentContext == machineName { - confg.CurrentContext = "" - if err := writeToFile(confg, filename); err != nil { + if kCfg.CurrentContext == machineName { + kCfg.CurrentContext = "" + if err := writeToFile(kCfg, fPath); err != nil { return errors.Wrap(err, "writing kubeconfig") } return nil @@ -42,21 +46,29 @@ func UnsetCurrentContext(filename, machineName string) error { } // SetCurrentContext sets the kubectl's current-context -func SetCurrentContext(kubeCfgPath, name string) error { - kcfg, err := readOrNew(kubeCfgPath) +func SetCurrentContext(name string, configPath ...string) error { + fPath := PathFromEnv() + if configPath != nil { + fPath = configPath[0] + } + kcfg, err := readOrNew(fPath) if err != nil { return errors.Wrap(err, "Error getting kubeconfig status") } kcfg.CurrentContext = name - if err := writeToFile(kcfg, kubeCfgPath); err != nil { + if err := writeToFile(kcfg, fPath); err != nil { return errors.Wrap(err, "writing kubeconfig") } return nil } // DeleteContext deletes the specified machine's kubeconfig context -func DeleteContext(kubeCfgPath, machineName string) error { - kcfg, err := readOrNew(kubeCfgPath) +func DeleteContext(machineName string, configPath ...string) error { + fPath := PathFromEnv() + if configPath != nil { + fPath = configPath[0] + } + kcfg, err := readOrNew(fPath) if err != nil { return errors.Wrap(err, "Error getting kubeconfig status") } @@ -74,7 +86,7 @@ func DeleteContext(kubeCfgPath, machineName string) error { kcfg.CurrentContext = "" } - if err := writeToFile(kcfg, kubeCfgPath); err != nil { + if err := writeToFile(kcfg, fPath); err != nil { return errors.Wrap(err, "writing kubeconfig") } return nil diff --git a/pkg/minikube/kubeconfig/context_test.go b/pkg/minikube/kubeconfig/context_test.go index a07ce8a0f..29a3b604c 100644 --- a/pkg/minikube/kubeconfig/context_test.go +++ b/pkg/minikube/kubeconfig/context_test.go @@ -24,12 +24,12 @@ import ( ) func TestDeleteContext(t *testing.T) { - configFilename := tempFile(t, fakeKubeCfg) - if err := DeleteContext(configFilename, "la-croix"); err != nil { + fn := tempFile(t, fakeKubeCfg) + if err := DeleteContext("la-croix", fn); err != nil { t.Fatal(err) } - cfg, err := readOrNew(configFilename) + cfg, err := readOrNew(fn) if err != nil { t.Fatal(err) } @@ -48,47 +48,47 @@ func TestDeleteContext(t *testing.T) { } func TestSetCurrentContext(t *testing.T) { - kubeConfigFile, err := ioutil.TempFile("/tmp", "kubeconfig") + f, err := ioutil.TempFile("/tmp", "kubeconfig") if err != nil { t.Fatalf("Error not expected but got %v", err) } - defer os.Remove(kubeConfigFile.Name()) + defer os.Remove(f.Name()) - cfg, err := readOrNew(kubeConfigFile.Name()) + kcfg, err := readOrNew(f.Name()) if err != nil { t.Fatalf("Error not expected but got %v", err) } - if cfg.CurrentContext != "" { - t.Errorf("Expected empty context but got %v", cfg.CurrentContext) + if kcfg.CurrentContext != "" { + t.Errorf("Expected empty context but got %v", kcfg.CurrentContext) } contextName := "minikube" - err = SetCurrentContext(kubeConfigFile.Name(), contextName) + err = SetCurrentContext(contextName, f.Name()) if err != nil { t.Fatalf("Error not expected but got %v", err) } defer func() { - err := UnsetCurrentContext(kubeConfigFile.Name(), contextName) + err := UnsetCurrentContext(contextName, f.Name()) if err != nil { t.Fatalf("Error not expected but got %v", err) } }() - cfg, err = readOrNew(kubeConfigFile.Name()) + kcfg, err = readOrNew(f.Name()) if err != nil { t.Fatalf("Error not expected but got %v", err) } - if cfg.CurrentContext != contextName { - t.Errorf("Expected context name %s but got %v : ", contextName, cfg.CurrentContext) + if kcfg.CurrentContext != contextName { + t.Errorf("Expected context name %s but got %v : ", contextName, kcfg.CurrentContext) } } func TestUnsetCurrentContext(t *testing.T) { - kubeConfigFile := "./testdata/kubeconfig/config1" + fn := filepath.Join("testdata", "kubeconfig", "config1") contextName := "minikube" - cfg, err := readOrNew(kubeConfigFile) + cfg, err := readOrNew(fn) if err != nil { t.Fatalf("Error not expected but got %v", err) } @@ -97,18 +97,18 @@ func TestUnsetCurrentContext(t *testing.T) { t.Errorf("Expected context name %s but got %s", contextName, cfg.CurrentContext) } - err = UnsetCurrentContext(kubeConfigFile, contextName) + err = UnsetCurrentContext(contextName, fn) if err != nil { t.Fatalf("Error not expected but got %v", err) } defer func() { - err := SetCurrentContext(kubeConfigFile, contextName) + err := SetCurrentContext(contextName, fn) if err != nil { t.Fatalf("Error not expected but got %v", err) } }() - cfg, err = readOrNew(kubeConfigFile) + cfg, err = readOrNew(fn) if err != nil { t.Fatalf("Error not expected but got %v", err) } @@ -121,8 +121,8 @@ func TestUnsetCurrentContext(t *testing.T) { func TestUnsetCurrentContextOnlyChangesIfProfileIsTheCurrentContext(t *testing.T) { contextName := "minikube" - kubeConfigFile := filepath.Join("testdata", "kubeconfig", "config2") - cfg, err := readOrNew(kubeConfigFile) + fn := filepath.Join("testdata", "kubeconfig", "config2") + cfg, err := readOrNew(fn) if err != nil { t.Fatalf("Error not expected but got %v", err) } @@ -131,12 +131,12 @@ func TestUnsetCurrentContextOnlyChangesIfProfileIsTheCurrentContext(t *testing.T t.Errorf("Expected context name %s but got %s", contextName, cfg.CurrentContext) } - err = UnsetCurrentContext(kubeConfigFile, "differentContextName") + err = UnsetCurrentContext("differentContextName", fn) if err != nil { t.Fatalf("Error not expected but got %v", err) } - cfg, err = readOrNew(kubeConfigFile) + cfg, err = readOrNew(fn) if err != nil { t.Fatalf("Error not expected but got %v", err) } diff --git a/pkg/minikube/kubeconfig/kcs.go b/pkg/minikube/kubeconfig/kcs.go index 634ad1190..5c90b277a 100644 --- a/pkg/minikube/kubeconfig/kcs.go +++ b/pkg/minikube/kubeconfig/kcs.go @@ -20,10 +20,12 @@ import ( "io/ioutil" "sync/atomic" + "github.com/golang/glog" + "github.com/pkg/errors" "k8s.io/client-go/tools/clientcmd/api" ) -// KCS is the kubeconfig setup +// KCS is the minikubes settings for kubeconfig type KCS struct { // The name of the cluster for this context ClusterName string @@ -51,18 +53,18 @@ type KCS struct { kubeConfigFile atomic.Value } -// SetKubeConfigFile sets the kubeconfig file -func (k *KCS) setPath(kubeConfigFile string) { +// SetPath sets the setting for kubeconfig filepath +func (k *KCS) SetPath(kubeConfigFile string) { k.kubeConfigFile.Store(kubeConfigFile) } -// fileContent gets the kubeconfig file -func (k *KCS) fileContent() string { +// filePath gets the kubeconfig file +func (k *KCS) filePath() string { return k.kubeConfigFile.Load().(string) } // Populate populates an api.Config object with values from *KCS -func Populate(cfg *KCS, apiCfg *api.Config) error { +func PopulateFromSetup(cfg *KCS, apiCfg *api.Config) error { var err error clusterName := cfg.ClusterName cluster := api.NewCluster() @@ -109,3 +111,27 @@ func Populate(cfg *KCS, apiCfg *api.Config) error { return nil } + +// update reads config from disk, adds the minikube settings, and writes it back. +// activeContext is true when minikube is the CurrentContext +// If no CurrentContext is set, the given name will be used. +func Update(kcs *KCS) error { + glog.Infoln("Using kubeconfig: ", kcs.filePath()) + + // read existing config or create new if does not exist + kcfg, err := readOrNew(kcs.filePath()) + if err != nil { + return err + } + + err = PopulateFromSetup(kcs, kcfg) + if err != nil { + return err + } + + // write back to disk + if err := writeToFile(kcfg, kcs.filePath()); err != nil { + return errors.Wrap(err, "writing kubeconfig") + } + return nil +} diff --git a/pkg/minikube/kubeconfig/kubeconfig.go b/pkg/minikube/kubeconfig/kubeconfig.go index a502690cd..4634b6a38 100644 --- a/pkg/minikube/kubeconfig/kubeconfig.go +++ b/pkg/minikube/kubeconfig/kubeconfig.go @@ -235,4 +235,4 @@ func decode(data []byte) (*api.Config, error) { } return kcfg.(*api.Config), nil -} \ No newline at end of file +} diff --git a/pkg/minikube/kubeconfig/kubeconfig_test.go b/pkg/minikube/kubeconfig/kubeconfig_test.go index d6a757afd..768cbad61 100644 --- a/pkg/minikube/kubeconfig/kubeconfig_test.go +++ b/pkg/minikube/kubeconfig/kubeconfig_test.go @@ -96,7 +96,7 @@ users: client-key: /home/la-croix/apiserver.key `) -func Test_update(t *testing.T) { +func TestUpdate(t *testing.T) { setupCfg := &KCS{ ClusterName: "test", ClusterServerAddress: "192.168.1.1:8080", @@ -147,20 +147,20 @@ func Test_update(t *testing.T) { if err != nil { t.Fatalf("Error making temp directory %v", err) } - test.cfg.setPath(filepath.Join(tmpDir, "kubeconfig")) + test.cfg.SetPath(filepath.Join(tmpDir, "kubeconfig")) if len(test.existingCfg) != 0 { - if err := ioutil.WriteFile(test.cfg.fileContent(), test.existingCfg, 0600); err != nil { + if err := ioutil.WriteFile(test.cfg.filePath(), test.existingCfg, 0600); err != nil { t.Fatalf("WriteFile: %v", err) } } - err = update(test.cfg) + err = Update(test.cfg) if err != nil && !test.err { t.Errorf("Got unexpected error: %v", err) } if err == nil && test.err { t.Errorf("Expected error but got none") } - config, err := readOrNew(test.cfg.fileContent()) + config, err := readOrNew(test.cfg.filePath()) if err != nil { t.Errorf("Error reading kubeconfig file: %v", err) } @@ -215,7 +215,7 @@ func TestIsClusterInConfig(t *testing.T) { t.Run(test.description, func(t *testing.T) { t.Parallel() configFilename := tempFile(t, test.existing) - statusActual, err := IsClusterInConfig(test.ip, configFilename, "minikube") + statusActual, err := IsClusterInConfig(test.ip, "minikube", configFilename) if err != nil && !test.err { t.Errorf("Got unexpected error: %v", err) } @@ -272,7 +272,7 @@ func TestUpdateIP(t *testing.T) { t.Run(test.description, func(t *testing.T) { t.Parallel() configFilename := tempFile(t, test.existing) - statusActual, err := UpdateIP(test.ip, configFilename, "minikube") + statusActual, err := UpdateIP(test.ip, "minikube", configFilename) if err != nil && !test.err { t.Errorf("Got unexpected error: %v", err) } @@ -359,7 +359,7 @@ func Test_extractIP(t *testing.T) { for _, test := range tests { t.Run(test.description, func(t *testing.T) { configFilename := tempFile(t, test.cfg) - ip, err := extractIP(configFilename, "minikube") + ip, err := extractIP("minikube", configFilename) if err != nil && !test.err { t.Errorf("Got unexpected error: %v", err) } @@ -559,7 +559,7 @@ func TestGetKubeConfigPath(t *testing.T) { for _, test := range tests { os.Setenv(clientcmd.RecommendedConfigPathEnvVar, test.input) - if result := Path(); result != test.want { + if result := PathFromEnv(); result != test.want { t.Errorf("Expected first splitted chunk, got: %s", result) } } diff --git a/pkg/minikube/kubeconfig/update.go b/pkg/minikube/kubeconfig/update.go deleted file mode 100644 index c0eeba4d7..000000000 --- a/pkg/minikube/kubeconfig/update.go +++ /dev/null @@ -1,193 +0,0 @@ -/* -Copyright 2019 The Kubernetes Authors All rights reserved. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package kubeconfig - -import ( - "fmt" - "io/ioutil" - "net" - "os" - "path/filepath" - "strconv" - "strings" - - "github.com/golang/glog" - "github.com/pkg/errors" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/client-go/tools/clientcmd/api" - "k8s.io/client-go/tools/clientcmd/api/latest" - cfg "k8s.io/minikube/pkg/minikube/config" - "k8s.io/minikube/pkg/minikube/constants" - pkgutil "k8s.io/minikube/pkg/util" -) - -// Setup sets up kubeconfig to be used by kubectl -func Setup(clusterURL string, c *cfg.Config) (*KCS, error) { - clusterURL = strings.Replace(clusterURL, "tcp://", "https://", -1) - clusterURL = strings.Replace(clusterURL, ":2376", ":"+strconv.Itoa(c.KubernetesConfig.NodePort), -1) - if c.KubernetesConfig.APIServerName != constants.APIServerName { - clusterURL = strings.Replace(clusterURL, c.KubernetesConfig.NodeIP, c.KubernetesConfig.APIServerName, -1) - } - - kcs := &KCS{ - ClusterName: cfg.GetMachineName(), - ClusterServerAddress: clusterURL, - ClientCertificate: constants.MakeMiniPath("client.crt"), - ClientKey: constants.MakeMiniPath("client.key"), - CertificateAuthority: constants.MakeMiniPath("ca.crt"), - KeepContext: c.MachineConfig.KeepContext, - EmbedCerts: c.MachineConfig.EmbedCerts, - } - kcs.setPath(Path()) - if err := update(kcs); err != nil { - return kcs, fmt.Errorf("error update kubeconfig: %v", err) - - } - return kcs, nil -} - -// update reads config from disk, adds the minikube settings, and writes it back. -// activeContext is true when minikube is the CurrentContext -// If no CurrentContext is set, the given name will be used. -func update(kcs *KCS) error { - glog.Infoln("Using kubeconfig: ", kcs.fileContent()) - - // read existing config or create new if does not exist - config, err := readOrNew(kcs.fileContent()) - if err != nil { - return err - } - - err = Populate(kcs, config) - if err != nil { - return err - } - - // write back to disk - if err := writeToFile(config, kcs.fileContent()); err != nil { - return errors.Wrap(err, "writing kubeconfig") - } - return nil -} - -// UpdateIP overwrites the IP stored in kubeconfig with the provided IP. -func UpdateIP(ip net.IP, filename string, machineName string) (bool, error) { - if ip == nil { - return false, fmt.Errorf("error, empty ip passed") - } - kip, err := extractIP(filename, machineName) - if err != nil { - return false, err - } - if kip.Equal(ip) { - return false, nil - } - kport, err := Port(filename, machineName) - if err != nil { - return false, err - } - con, err := readOrNew(filename) - if err != nil { - return false, errors.Wrap(err, "Error getting kubeconfig status") - } - // Safe to lookup server because if field non-existent getIPFromKubeconfig would have given an error - con.Clusters[machineName].Server = "https://" + ip.String() + ":" + strconv.Itoa(kport) - err = writeToFile(con, filename) - if err != nil { - return false, err - } - // Kubeconfig IP reconfigured - return true, nil -} - -// writeToFile encodes the configuration and writes it to the given file. -// If the file exists, it's contents will be overwritten. -func writeToFile(config runtime.Object, filename string) error { - if config == nil { - glog.Errorf("could not write to '%s': config can't be nil", filename) - } - - // encode config to YAML - data, err := runtime.Encode(latest.Codec, config) - if err != nil { - return errors.Errorf("could not write to '%s': failed to encode config: %v", filename, err) - } - - // create parent dir if doesn't exist - dir := filepath.Dir(filename) - if _, err := os.Stat(dir); os.IsNotExist(err) { - if err = os.MkdirAll(dir, 0755); err != nil { - return errors.Wrapf(err, "Error creating directory: %s", dir) - } - } - - // write with restricted permissions - if err := ioutil.WriteFile(filename, data, 0600); err != nil { - return errors.Wrapf(err, "Error writing file %s", filename) - } - if err := pkgutil.MaybeChownDirRecursiveToMinikubeUser(dir); err != nil { - return errors.Wrapf(err, "Error recursively changing ownership for dir: %s", dir) - } - - return nil -} - -// readOrNew retrieves Kubernetes client configuration from a file. -// If no files exists, an empty configuration is returned. -func readOrNew(filename string) (*api.Config, error) { - data, err := ioutil.ReadFile(filename) - if os.IsNotExist(err) { - return api.NewConfig(), nil - } else if err != nil { - return nil, errors.Wrapf(err, "Error reading file %q", filename) - } - - // decode config, empty if no bytes - config, err := decode(data) - if err != nil { - return nil, errors.Errorf("could not read config: %v", err) - } - - // initialize nil maps - if config.AuthInfos == nil { - config.AuthInfos = map[string]*api.AuthInfo{} - } - if config.Clusters == nil { - config.Clusters = map[string]*api.Cluster{} - } - if config.Contexts == nil { - config.Contexts = map[string]*api.Context{} - } - - return config, nil -} - -// decode reads a Config object from bytes. -// Returns empty config if no bytes. -func decode(data []byte) (*api.Config, error) { - // if no data, return empty config - if len(data) == 0 { - return api.NewConfig(), nil - } - - config, _, err := latest.Codec.Decode(data, nil, nil) - if err != nil { - return nil, errors.Wrapf(err, "Error decoding config from data: %s", string(data)) - } - - return config.(*api.Config), nil -} -- GitLab