From 4a4ae87e9e1819d9fc072390b20d4eb3307ee4a1 Mon Sep 17 00:00:00 2001 From: josedonizetti Date: Thu, 1 Aug 2019 14:37:30 -0300 Subject: [PATCH 1/5] refactor: Move WriteConfig to pkg/minikube/config --- cmd/minikube/cmd/config/config.go | 34 +---------------- cmd/minikube/cmd/config/config_test.go | 52 -------------------------- cmd/minikube/cmd/config/set.go | 2 +- cmd/minikube/cmd/config/unset.go | 2 +- pkg/minikube/config/config.go | 25 +++++++++++++ pkg/minikube/config/config_test.go | 16 +++++++- 6 files changed, 44 insertions(+), 87 deletions(-) diff --git a/cmd/minikube/cmd/config/config.go b/cmd/minikube/cmd/config/config.go index e8143caabc..ba9a3d53be 100644 --- a/cmd/minikube/cmd/config/config.go +++ b/cmd/minikube/cmd/config/config.go @@ -17,16 +17,11 @@ limitations under the License. package config import ( - "encoding/json" - "fmt" - "io" - "os" "strings" "github.com/golang/glog" "github.com/spf13/cobra" "k8s.io/minikube/pkg/minikube/config" - "k8s.io/minikube/pkg/minikube/constants" ) // Bootstrapper is the name for bootstrapper @@ -319,7 +314,7 @@ func AddToConfigMap(name string, images []string) error { return err } // Write the values - return WriteConfig(configFile) + return config.WriteConfig(configFile) } // DeleteFromConfigMap deletes entries from a map in the config file @@ -344,30 +339,5 @@ func DeleteFromConfigMap(name string, images []string) error { return err } // Write the values - return WriteConfig(configFile) -} - -// WriteConfig writes a minikube config to the JSON file -func WriteConfig(m config.MinikubeConfig) error { - f, err := os.Create(constants.ConfigFile) - if err != nil { - return fmt.Errorf("create %s: %s", constants.ConfigFile, err) - } - defer f.Close() - err = encode(f, m) - if err != nil { - return fmt.Errorf("encode %s: %s", constants.ConfigFile, err) - } - return nil -} - -func encode(w io.Writer, m config.MinikubeConfig) error { - b, err := json.MarshalIndent(m, "", " ") - if err != nil { - return err - } - - _, err = w.Write(b) - - return err + return config.WriteConfig(configFile) } diff --git a/cmd/minikube/cmd/config/config_test.go b/cmd/minikube/cmd/config/config_test.go index 07388fef4e..56a84b28d0 100644 --- a/cmd/minikube/cmd/config/config_test.go +++ b/cmd/minikube/cmd/config/config_test.go @@ -20,46 +20,8 @@ import ( "bytes" "fmt" "testing" - - "k8s.io/minikube/pkg/minikube/constants" ) -type configTestCase struct { - data string - config map[string]interface{} -} - -var configTestCases = []configTestCase{ - { - data: `{ - "memory": 2 -}`, - config: map[string]interface{}{ - "memory": 2, - }, - }, - { - data: `{ - "ReminderWaitPeriodInHours": 99, - "cpus": 4, - "disk-size": "20g", - "log_dir": "/etc/hosts", - "show-libmachine-logs": true, - "v": 5, - "vm-driver": "kvm2" -}`, - config: map[string]interface{}{ - "vm-driver": constants.DriverKvm2, - "cpus": 4, - "disk-size": "20g", - "v": 5, - "show-libmachine-logs": true, - "log_dir": "/etc/hosts", - "ReminderWaitPeriodInHours": 99, - }, - }, -} - func TestHiddenPrint(t *testing.T) { testCases := []struct { TestString string @@ -90,17 +52,3 @@ func TestHiddenPrint(t *testing.T) { } } } - -func TestWriteConfig(t *testing.T) { - var b bytes.Buffer - for _, tt := range configTestCases { - err := encode(&b, tt.config) - if err != nil { - t.Errorf("Error encoding: %v", err) - } - if b.String() != tt.data { - t.Errorf("Did not write config correctly, \n\n expected:\n %+v \n\n actual:\n %+v", tt.data, b.String()) - } - b.Reset() - } -} diff --git a/cmd/minikube/cmd/config/set.go b/cmd/minikube/cmd/config/set.go index 096473a151..4b3a37755e 100644 --- a/cmd/minikube/cmd/config/set.go +++ b/cmd/minikube/cmd/config/set.go @@ -71,5 +71,5 @@ func Set(name string, value string) error { } // Write the value - return WriteConfig(config) + return pkgConfig.WriteConfig(config) } diff --git a/cmd/minikube/cmd/config/unset.go b/cmd/minikube/cmd/config/unset.go index 39b90b0a99..8fb1fdcea1 100644 --- a/cmd/minikube/cmd/config/unset.go +++ b/cmd/minikube/cmd/config/unset.go @@ -48,5 +48,5 @@ func Unset(name string) error { return err } delete(m, name) - return WriteConfig(m) + return pkgConfig.WriteConfig(m) } diff --git a/pkg/minikube/config/config.go b/pkg/minikube/config/config.go index 84d8c9c704..ff202726a9 100644 --- a/pkg/minikube/config/config.go +++ b/pkg/minikube/config/config.go @@ -74,6 +74,20 @@ func get(name string, config MinikubeConfig) (string, error) { return "", ErrKeyNotFound } +// WriteConfig writes a minikube config to the JSON file +func WriteConfig(m MinikubeConfig) error { + f, err := os.Create(constants.ConfigFile) + if err != nil { + return fmt.Errorf("create %s: %s", constants.ConfigFile, err) + } + defer f.Close() + err = encode(f, m) + if err != nil { + return fmt.Errorf("encode %s: %s", constants.ConfigFile, err) + } + return nil +} + // ReadConfig reads in the JSON minikube config func ReadConfig() (MinikubeConfig, error) { return readConfig(constants.ConfigFile) @@ -103,6 +117,17 @@ func decode(r io.Reader) (MinikubeConfig, error) { return data, err } +func encode(w io.Writer, m MinikubeConfig) error { + b, err := json.MarshalIndent(m, "", " ") + if err != nil { + return err + } + + _, err = w.Write(b) + + return err +} + // GetMachineName gets the machine name for the VM func GetMachineName() string { // REFACTOR NECESSARY: This function should not rely on globals. diff --git a/pkg/minikube/config/config_test.go b/pkg/minikube/config/config_test.go index 7a92291e41..23bf1cb7b3 100644 --- a/pkg/minikube/config/config_test.go +++ b/pkg/minikube/config/config_test.go @@ -46,7 +46,7 @@ var configTestCases = []configTestCase{ "log_dir": "/etc/hosts", "show-libmachine-logs": true, "v": 5, - "vm-driver": "kvm" + "vm-driver": "kvm2" }`, config: map[string]interface{}{ "vm-driver": constants.DriverKvm2, @@ -141,3 +141,17 @@ func Test_readConfig(t *testing.T) { t.Errorf("Did not read config correctly,\n\n wanted %+v, \n\n got %+v", expectedConfig, mkConfig) } } + +func TestWriteConfig(t *testing.T) { + var b bytes.Buffer + for _, tt := range configTestCases { + err := encode(&b, tt.config) + if err != nil { + t.Errorf("Error encoding: %v", err) + } + if b.String() != tt.data { + t.Errorf("Did not write config correctly, \n\n expected:\n %+v \n\n actual:\n %+v", tt.data, b.String()) + } + b.Reset() + } +} From 487c104ab91e70ee436b3a363b8fbe96bb1828fb Mon Sep 17 00:00:00 2001 From: josedonizetti Date: Thu, 1 Aug 2019 14:48:45 -0300 Subject: [PATCH 2/5] Rename test to Test_encode --- pkg/minikube/config/config_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/minikube/config/config_test.go b/pkg/minikube/config/config_test.go index 23bf1cb7b3..5ffdaab035 100644 --- a/pkg/minikube/config/config_test.go +++ b/pkg/minikube/config/config_test.go @@ -142,7 +142,7 @@ func Test_readConfig(t *testing.T) { } } -func TestWriteConfig(t *testing.T) { +func Test_encode(t *testing.T) { var b bytes.Buffer for _, tt := range configTestCases { err := encode(&b, tt.config) From 021eaa723d83c52c0c1cdda4ba0636c3a6ddf707 Mon Sep 17 00:00:00 2001 From: josedonizetti Date: Thu, 1 Aug 2019 14:54:47 -0300 Subject: [PATCH 3/5] Extract writeConfig --- pkg/minikube/config/config.go | 10 +++++++--- pkg/minikube/config/config_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/pkg/minikube/config/config.go b/pkg/minikube/config/config.go index ff202726a9..3200342edb 100644 --- a/pkg/minikube/config/config.go +++ b/pkg/minikube/config/config.go @@ -76,14 +76,18 @@ func get(name string, config MinikubeConfig) (string, error) { // WriteConfig writes a minikube config to the JSON file func WriteConfig(m MinikubeConfig) error { - f, err := os.Create(constants.ConfigFile) + return writeConfig(constants.ConfigFile, m) +} + +func writeConfig(configFile string, m MinikubeConfig) error { + f, err := os.Create(configFile) if err != nil { - return fmt.Errorf("create %s: %s", constants.ConfigFile, err) + return fmt.Errorf("create %s: %s", configFile, err) } defer f.Close() err = encode(f, m) if err != nil { - return fmt.Errorf("encode %s: %s", constants.ConfigFile, err) + return fmt.Errorf("encode %s: %s", configFile, err) } return nil } diff --git a/pkg/minikube/config/config_test.go b/pkg/minikube/config/config_test.go index 5ffdaab035..e8dfea7dae 100644 --- a/pkg/minikube/config/config_test.go +++ b/pkg/minikube/config/config_test.go @@ -18,6 +18,7 @@ package config import ( "bytes" + "os" "reflect" "testing" @@ -142,6 +143,32 @@ func Test_readConfig(t *testing.T) { } } +func Test_writeConfig(t *testing.T) { + configFile := "./testdata/configTest" + cfg := map[string]interface{}{ + "vm-driver": constants.DriverKvm2, + "cpus": 4, + "disk-size": "20g", + "show-libmachine-logs": true, + "log_dir": "/etc/hosts", + } + + err := writeConfig(configFile, cfg) + if err != nil { + t.Fatalf("Error not expected but got %v", err) + } + defer os.Remove(configFile) + + mkConfig, err := readConfig("./testdata/.minikube/config/valid_config.json") + if err != nil { + t.Fatalf("Error not expected but got %v", err) + } + + if reflect.DeepEqual(cfg, mkConfig) || err != nil { + t.Errorf("Did not read config correctly,\n\n wanted %+v, \n\n got %+v", cfg, mkConfig) + } +} + func Test_encode(t *testing.T) { var b bytes.Buffer for _, tt := range configTestCases { From 8b42e28d4491c384a0c8c3ab2fc78372770630fc Mon Sep 17 00:00:00 2001 From: josedonizetti Date: Fri, 2 Aug 2019 21:45:01 -0300 Subject: [PATCH 4/5] Refactor WriteConfig to accept a path --- cmd/minikube/cmd/config/config.go | 5 +++-- cmd/minikube/cmd/config/set.go | 3 ++- cmd/minikube/cmd/config/unset.go | 3 ++- pkg/minikube/config/config.go | 6 +----- pkg/minikube/config/config_test.go | 15 ++++++++++----- 5 files changed, 18 insertions(+), 14 deletions(-) diff --git a/cmd/minikube/cmd/config/config.go b/cmd/minikube/cmd/config/config.go index ba9a3d53be..5f0e44403a 100644 --- a/cmd/minikube/cmd/config/config.go +++ b/cmd/minikube/cmd/config/config.go @@ -22,6 +22,7 @@ import ( "github.com/golang/glog" "github.com/spf13/cobra" "k8s.io/minikube/pkg/minikube/config" + "k8s.io/minikube/pkg/minikube/constants" ) // Bootstrapper is the name for bootstrapper @@ -314,7 +315,7 @@ func AddToConfigMap(name string, images []string) error { return err } // Write the values - return config.WriteConfig(configFile) + return config.WriteConfig(constants.ConfigFile, configFile) } // DeleteFromConfigMap deletes entries from a map in the config file @@ -339,5 +340,5 @@ func DeleteFromConfigMap(name string, images []string) error { return err } // Write the values - return config.WriteConfig(configFile) + return config.WriteConfig(constants.ConfigFile, configFile) } diff --git a/cmd/minikube/cmd/config/set.go b/cmd/minikube/cmd/config/set.go index 4b3a37755e..3019c793e2 100644 --- a/cmd/minikube/cmd/config/set.go +++ b/cmd/minikube/cmd/config/set.go @@ -19,6 +19,7 @@ package config import ( "github.com/spf13/cobra" pkgConfig "k8s.io/minikube/pkg/minikube/config" + "k8s.io/minikube/pkg/minikube/constants" "k8s.io/minikube/pkg/minikube/exit" ) @@ -71,5 +72,5 @@ func Set(name string, value string) error { } // Write the value - return pkgConfig.WriteConfig(config) + return pkgConfig.WriteConfig(constants.ConfigFile, config) } diff --git a/cmd/minikube/cmd/config/unset.go b/cmd/minikube/cmd/config/unset.go index 8fb1fdcea1..fc91390ed8 100644 --- a/cmd/minikube/cmd/config/unset.go +++ b/cmd/minikube/cmd/config/unset.go @@ -19,6 +19,7 @@ package config import ( "github.com/spf13/cobra" pkgConfig "k8s.io/minikube/pkg/minikube/config" + "k8s.io/minikube/pkg/minikube/constants" "k8s.io/minikube/pkg/minikube/exit" ) @@ -48,5 +49,5 @@ func Unset(name string) error { return err } delete(m, name) - return pkgConfig.WriteConfig(m) + return pkgConfig.WriteConfig(constants.ConfigFile, m) } diff --git a/pkg/minikube/config/config.go b/pkg/minikube/config/config.go index 3200342edb..11268ec138 100644 --- a/pkg/minikube/config/config.go +++ b/pkg/minikube/config/config.go @@ -75,11 +75,7 @@ func get(name string, config MinikubeConfig) (string, error) { } // WriteConfig writes a minikube config to the JSON file -func WriteConfig(m MinikubeConfig) error { - return writeConfig(constants.ConfigFile, m) -} - -func writeConfig(configFile string, m MinikubeConfig) error { +func WriteConfig(configFile string, m MinikubeConfig) error { f, err := os.Create(configFile) if err != nil { return fmt.Errorf("create %s: %s", configFile, err) diff --git a/pkg/minikube/config/config_test.go b/pkg/minikube/config/config_test.go index e8dfea7dae..b6390a2ae3 100644 --- a/pkg/minikube/config/config_test.go +++ b/pkg/minikube/config/config_test.go @@ -18,6 +18,7 @@ package config import ( "bytes" + "io/ioutil" "os" "reflect" "testing" @@ -143,8 +144,12 @@ func Test_readConfig(t *testing.T) { } } -func Test_writeConfig(t *testing.T) { - configFile := "./testdata/configTest" +func TestWriteConfig(t *testing.T) { + configFile, err := ioutil.TempFile("/tmp", "configTest") + if err != nil { + t.Fatalf("Error not expected but got %v", err) + } + cfg := map[string]interface{}{ "vm-driver": constants.DriverKvm2, "cpus": 4, @@ -153,13 +158,13 @@ func Test_writeConfig(t *testing.T) { "log_dir": "/etc/hosts", } - err := writeConfig(configFile, cfg) + err = WriteConfig(configFile.Name(), cfg) if err != nil { t.Fatalf("Error not expected but got %v", err) } - defer os.Remove(configFile) + defer os.Remove(configFile.Name()) - mkConfig, err := readConfig("./testdata/.minikube/config/valid_config.json") + mkConfig, err := readConfig(configFile.Name()) if err != nil { t.Fatalf("Error not expected but got %v", err) } From 2fbd028c072bea5fe7c9186e25fee103dc3fd1eb Mon Sep 17 00:00:00 2001 From: josedonizetti Date: Fri, 2 Aug 2019 21:57:31 -0300 Subject: [PATCH 5/5] Refactor improve variable names --- cmd/minikube/cmd/config/config.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/cmd/minikube/cmd/config/config.go b/cmd/minikube/cmd/config/config.go index 5f0e44403a..ac2423760c 100644 --- a/cmd/minikube/cmd/config/config.go +++ b/cmd/minikube/cmd/config/config.go @@ -298,7 +298,7 @@ func AddToConfigMap(name string, images []string) error { return err } // Set the values - configFile, err := config.ReadConfig() + cfg, err := config.ReadConfig() if err != nil { return err } @@ -306,16 +306,16 @@ func AddToConfigMap(name string, images []string) error { for _, image := range images { newImages[image] = nil } - if values, ok := configFile[name].(map[string]interface{}); ok { + if values, ok := cfg[name].(map[string]interface{}); ok { for key := range values { newImages[key] = nil } } - if err = s.setMap(configFile, name, newImages); err != nil { + if err = s.setMap(cfg, name, newImages); err != nil { return err } // Write the values - return config.WriteConfig(constants.ConfigFile, configFile) + return config.WriteConfig(constants.ConfigFile, cfg) } // DeleteFromConfigMap deletes entries from a map in the config file @@ -325,20 +325,20 @@ func DeleteFromConfigMap(name string, images []string) error { return err } // Set the values - configFile, err := config.ReadConfig() + cfg, err := config.ReadConfig() if err != nil { return err } - values, ok := configFile[name] + values, ok := cfg[name] if !ok { return nil } for _, image := range images { delete(values.(map[string]interface{}), image) } - if err = s.setMap(configFile, name, values.(map[string]interface{})); err != nil { + if err = s.setMap(cfg, name, values.(map[string]interface{})); err != nil { return err } // Write the values - return config.WriteConfig(constants.ConfigFile, configFile) + return config.WriteConfig(constants.ConfigFile, cfg) }