config refactoring: address review comments

This commit:
  - Removes the default value in cobra config flag
  - Adds a logic to set the default file under the config package

Signed-off-by: Karen Almog <[email protected]>

cmd/backup/backup.go

@@ -80,7 +80,7 @@ func (c *CmdOpts) backup() error {
 		if err != nil {
 			return err
 		}
-		return mgr.RunBackup(c.CfgFile, c.NodeConfig.Spec, c.K0sVars, savePath)
+		return mgr.RunBackup(c.NodeConfig.Spec, c.K0sVars, savePath)
 	}
 	return fmt.Errorf("backup command must be run on the controller node, have `%s`", status.Role)
 }

cmd/restore/restore.go

@@ -58,7 +58,7 @@ func NewRestoreCmd() *cobra.Command {
 
 	cwd, err := os.Getwd()
 	if err != nil {
-		return nil
+		logrus.Fatal("failed to get local path")
 	}
 
 	restoredConfigPathDescription := fmt.Sprintf("Specify desired name and full path for the restored k0s.yaml file (default: %s/k0s_<archive timestamp>.yaml", cwd)

internal/testutil/runtime_config.go

@@ -64,7 +64,6 @@ func (c *ConfigGetter) FakeConfigFromFile() (*v1beta1.ClusterConfig, error) {
 	loadingRules := config.ClientConfigLoadingRules{
 		RuntimeConfigPath: RuntimeFakePath,
 		Nodeconfig:        c.NodeConfig,
-		CfgFileOverride:   c.cfgFilePath,
 		K0sVars:           c.k0sVars,
 	}
 	return loadingRules.Load()
@@ -87,7 +86,6 @@ func (c *ConfigGetter) FakeAPIConfig() (*v1beta1.ClusterConfig, error) {
 	loadingRules := config.ClientConfigLoadingRules{
 		RuntimeConfigPath: RuntimeFakePath,
 		Nodeconfig:        c.NodeConfig,
-		CfgFileOverride:   c.cfgFilePath,
 		APIClient:         client.K0sV1beta1(),
 		K0sVars:           c.k0sVars,
 	}

inttest/common/footloosesuite.go

@@ -564,7 +564,6 @@ func (s *FootlooseSuite) GetKubeClientConfig(node string, k0sKubeconfigArgs ...s
 	kubeConfigCmd := fmt.Sprintf("%s kubeconfig admin %s 2>/dev/null", s.K0sFullPath, strings.Join(k0sKubeconfigArgs, " "))
 	kubeConf, err := ssh.ExecWithOutput(kubeConfigCmd)
 	if err != nil {
-		fmt.Println(string(kubeConf))
 		return nil, err
 	}
 	cfg, err := clientcmd.Load([]byte(kubeConf))

pkg/backup/manager.go

@@ -44,8 +44,14 @@ type Manager struct {
 }
 
 // RunBackup backups cluster
-func (bm *Manager) RunBackup(cfgPath string, nodeSpec *v1beta1.ClusterSpec, vars constant.CfgVars, savePathDir string) error {
-	bm.discoverSteps(cfgPath, nodeSpec, vars, "backup", "")
+func (bm *Manager) RunBackup(nodeSpec *v1beta1.ClusterSpec, vars constant.CfgVars, savePathDir string) error {
+	configLoader := config.ClientConfigLoadingRules{}
+	_, err := configLoader.Load()
+	if err != nil {
+		return err
+	}
+
+	bm.discoverSteps(configLoader.RuntimeConfigPath, nodeSpec, vars, "backup", "")
 	defer os.RemoveAll(bm.tmpDir)
 	assets := make([]string, 0, len(bm.steps))
 

pkg/backup/util.go

@@ -44,7 +44,7 @@ func createArchive(archive io.Writer, files []string, baseDir string) error {
 	for _, file := range files {
 		err := addToArchive(tw, file, baseDir)
 		if err != nil {
-			return err
+			return fmt.Errorf("failed to add file to backup archive: %v", err)
 		}
 	}
 	return nil

pkg/config/cli.go

@@ -16,6 +16,7 @@ limitations under the License.
 package config
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
@@ -189,7 +190,8 @@ func GetControllerFlags() *pflag.FlagSet {
 // it in multiple places
 func FileInputFlag() *pflag.FlagSet {
 	flagset := &pflag.FlagSet{}
-	flagset.StringVarP(&CfgFile, "config", "c", constant.K0sConfigPathDefault, "config file, use '-' to read the config from stdin")
+	descString := fmt.Sprintf("config file, use '-' to read the config from stdin (default \"%s\")", constant.K0sConfigPathDefault)
+	flagset.StringVarP(&CfgFile, "config", "c", "", descString)
 
 	return flagset
 }
@@ -202,8 +204,8 @@ func GetCmdOpts() CLIOptions {
 		K0sVars.DefaultStorageType = "kine"
 	}
 
-	// when a custom config file is used, verify that it can be opened
-	if CfgFile != constant.K0sConfigPathDefault {
+	// When CfgFile is set, verify the file can be opened
+	if CfgFile != "" {
 		_, err := os.Open(CfgFile)
 		if err != nil {
 			logrus.Fatalf("failed to load config file (%s): %v", CfgFile, err)

pkg/config/config.go

@@ -71,9 +71,6 @@ type ClientConfigLoadingRules struct {
 	// this parameter is mainly used for testing purposes, to override the default location on local dev system
 	RuntimeConfigPath string
 
-	// CfgFileOverride is an optional field for overriding the CfgFile parameter from cobra. Used mainly for testing purposes.
-	CfgFileOverride string
-
 	// K0sVars is needed for fetching the right config from the API
 	K0sVars constant.CfgVars
 }
@@ -111,10 +108,6 @@ func (rules *ClientConfigLoadingRules) IsDefaultConfig() bool {
 }
 
 func (rules *ClientConfigLoadingRules) Load() (*v1beta1.ClusterConfig, error) {
-	if rules.CfgFileOverride != "" {
-		CfgFile = rules.CfgFileOverride
-	}
-
 	if rules.Nodeconfig {
 		return rules.fetchNodeConfig()
 	}

pkg/config/config_test.go

@@ -144,10 +144,15 @@ spec:
 
 // Test using config from a yaml file
 func TestConfigFromDefaults(t *testing.T) {
-	CfgFile = constant.K0sConfigPathDefault // this path doesn't exist, so default values should be generated
 	defer os.Remove(configPathRuntimeTest)
 
+	CfgFile = ""
 	loadingRules := ClientConfigLoadingRules{RuntimeConfigPath: configPathRuntimeTest}
+	err := loadingRules.InitRuntimeConfig(constant.GetConfig(""))
+	if err != nil {
+		t.Fatalf("failed to initialize k0s config: %s", err.Error())
+	}
+
 	cfg, err := loadingRules.Load()
 	if err != nil {
 		t.Fatalf("failed to load config: %s", err.Error())
@@ -215,12 +220,12 @@ func TestNodeConfigWithAPIConfig(t *testing.T) {
 }
 
 func TestSingleNodeConfig(t *testing.T) {
-	CfgFile = constant.K0sConfigPathDefault // this path doesn't exist, so default values should be generated
 	defer os.Remove(configPathRuntimeTest)
 
 	loadingRules := ClientConfigLoadingRules{RuntimeConfigPath: configPathRuntimeTest, Nodeconfig: true}
 	k0sVars := constant.GetConfig("")
 	k0sVars.DefaultStorageType = "kine"
+	CfgFile = ""
 
 	err := loadingRules.InitRuntimeConfig(k0sVars)
 	if err != nil {
@@ -231,9 +236,11 @@ func TestSingleNodeConfig(t *testing.T) {
 	if err != nil {
 		t.Fatalf("failed to load config: %s", err.Error())
 	}
+
 	if cfg == nil {
 		t.Fatal("received an empty config! failing")
 	}
+
 	testCases := []struct {
 		name     string
 		got      string

pkg/config/file_config.go

@@ -61,34 +61,57 @@ func (rules *ClientConfigLoadingRules) readRuntimeConfig() (clusterConfig *v1bet
 func (rules *ClientConfigLoadingRules) ParseRuntimeConfig() (*v1beta1.ClusterConfig, error) {
 	var cfg *v1beta1.ClusterConfig
 
+	var storage *v1beta1.StorageSpec
+	if rules.K0sVars.DefaultStorageType == "kine" {
+		storage = &v1beta1.StorageSpec{
+			Type: v1beta1.KineStorageType,
+			Kine: v1beta1.DefaultKineConfig(rules.K0sVars.DataDir),
+		}
+	}
 	if rules.RuntimeConfigPath == "" {
 		rules.RuntimeConfigPath = runtimeConfigPathDefault
 	}
 
-	// don't create the runtime config file, if it already exists
+	// If runtime config already exists, use it as the source of truth
 	if file.Exists(rules.RuntimeConfigPath) {
 		logrus.Debugf("runtime config found: using %s", rules.RuntimeConfigPath)
-		CfgFile = rules.RuntimeConfigPath
-	}
 
-	var storage *v1beta1.StorageSpec
-	if rules.K0sVars.DefaultStorageType == "kine" {
-		storage = &v1beta1.StorageSpec{
-			Type: v1beta1.KineStorageType,
-			Kine: v1beta1.DefaultKineConfig(rules.K0sVars.DataDir),
+		// read config from runtime config
+		f, err := os.Open(rules.RuntimeConfigPath)
+		if err != nil {
+			return nil, err
 		}
+		defer f.Close()
+
+		cfg, err = v1beta1.ConfigFromReader(f, storage)
+		if err != nil {
+			return nil, err
+		}
+		return cfg, nil
 	}
 
 	switch CfgFile {
 	// stdin input
 	case "-":
 		return v1beta1.ConfigFromReader(os.Stdin, storage)
-	default:
-		f, err := os.Open(CfgFile)
+	case "":
+		// if no config is set, look for config in the default location
+		// if it does not exist there either, generate default config
+		f, err := os.Open(constant.K0sConfigPathDefault)
 		if err != nil {
 			if os.IsNotExist(err) {
 				return rules.generateDefaults(storage), nil
 			}
+		}
+		defer f.Close()
+
+		cfg, err = v1beta1.ConfigFromReader(f, storage)
+		if err != nil {
+			return nil, err
+		}
+	default:
+		f, err := os.Open(CfgFile)
+		if err != nil {
 			return nil, err
 		}
 		defer f.Close()