Repository: code-ready/crc
Pull request #1877 (2021-01-22 08:25:03)

Add crc setup instruction if start fail in preflight
**Fixes:** Issue #1876 

## Solution/Idea
In case of error during preflight, better to advice the user to
run crc setup once if user directly started executed `crc start`.



openshift-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/code-ready/crc/pull/1877#" title="Author self-approved">praveenkumar</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=code-ready%2Fcrc).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/code-ready/crc/blob/master/OWNERS)~~ [praveenkumar]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/cmd/crc/cmd/daemon.go b/cmd/crc/cmd/daemon.go
index 13d0fc267..ab76563ae 100644
--- a/cmd/crc/cmd/daemon.go
+++ b/cmd/crc/cmd/daemon.go
@@ -15,7 +15,6 @@ import (
 	"github.com/code-ready/crc/pkg/crc/api"
 	crcConfig "github.com/code-ready/crc/pkg/crc/config"
 	"github.com/code-ready/crc/pkg/crc/constants"
-	"github.com/code-ready/crc/pkg/crc/logging"
 	"github.com/code-ready/gvisor-tap-vsock/pkg/transport"
 	"github.com/code-ready/gvisor-tap-vsock/pkg/types"
 	"github.com/code-ready/gvisor-tap-vsock/pkg/virtualnetwork"
@@ -35,10 +34,6 @@ var daemonCmd = &cobra.Command{
 	Long:   "Run the crc daemon",
 	Hidden: true,
 	RunE: func(cmd *cobra.Command, args []string) error {
-		// setup separate logging for daemon
-		logging.CloseLogging()
-		logging.InitLogrus(logging.LogLevel, constants.DaemonLogFilePath)
-
 		var endpoints []string
 		if runtime.GOOS == "windows" {
 			endpoints = append(endpoints, transport.DefaultURL)
diff --git a/cmd/crc/cmd/root.go b/cmd/crc/cmd/root.go
index 7d5062da8..caca9406c 100644
--- a/cmd/crc/cmd/root.go
+++ b/cmd/crc/cmd/root.go
@@ -27,7 +27,7 @@ var rootCmd = &cobra.Command{
 	Short: descriptionShort,
 	Long:  descriptionLong,
 	PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
-		return runPrerun()
+		return runPrerun(cmd)
 	},
 	Run: func(cmd *cobra.Command, args []string) {
 		runRoot()
@@ -64,9 +64,13 @@ func init() {
 	rootCmd.PersistentFlags().StringVar(&logging.LogLevel, "log-level", constants.DefaultLogLevel, "log level (e.g. \"debug | info | warn | error\")")
 }
 
-func runPrerun() error {
+func runPrerun(cmd *cobra.Command) error {
 	// Setting up logrus
-	logging.InitLogrus(logging.LogLevel, constants.LogFilePath)
+	logFile := constants.LogFilePath
+	if cmd == daemonCmd {
+		logFile = constants.DaemonLogFilePath
+	}
+	logging.InitLogrus(logging.LogLevel, logFile)
 	if err := setProxyDefaults(); err != nil {
 		return err
 	}
@@ -78,8 +82,8 @@ func runPrerun() error {
 }
 
 func runPostrun() {
-	logging.CloseLogging()
 	segmentClient.Close()
+	logging.CloseLogging()
 }
 
 func runRoot() {
diff --git a/cmd/crc/cmd/start.go b/cmd/crc/cmd/start.go
index af3e9dc07..e2e3a812a 100644
--- a/cmd/crc/cmd/start.go
+++ b/cmd/crc/cmd/start.go
@@ -129,6 +129,10 @@ type startResult struct {
 
 func (s *startResult) prettyPrintTo(writer io.Writer) error {
 	if s.Error != nil {
+		var e *crcErrors.PreflightError
+		if errors.As(s.Error, &e) {
+			logging.Warn("Preflight checks failed during `crc start`, please try to run `crc setup` first in case you haven't done so yet")
+		}
 		return s.Error
 	}
 	if s.ClusterConfig == nil {
diff --git a/pkg/crc/config/viper_config.go b/pkg/crc/config/viper_config.go
index ba52376a4..12b0e9fda 100644
--- a/pkg/crc/config/viper_config.go
+++ b/pkg/crc/config/viper_config.go
@@ -1,7 +1,6 @@
 package config
 
 import (
-	"bytes"
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
@@ -45,21 +44,43 @@ func (c *ViperStorage) Get(key string) interface{} {
 }
 
 func (c *ViperStorage) Set(key string, value interface{}) error {
-	c.viper.Set(key, value)
-	return atomicWrite(c.viper, c.configFile)
+	in, err := ioutil.ReadFile(c.configFile)
+	if err != nil {
+		return err
+	}
+	var cfg map[string]interface{}
+	if err := json.Unmarshal(in, &cfg); err != nil {
+		return err
+	}
+	cfg[key] = value
+	bin, err := json.MarshalIndent(cfg, "", "  ")
+	if err != nil {
+		return err
+	}
+	if err := atomicWrite(bin, c.configFile); err != nil {
+		return err
+	}
+	return c.viper.ReadInConfig()
 }
 
 func (c *ViperStorage) Unset(key string) error {
-	settings := c.viper.AllSettings()
-	delete(settings, key)
-	bin, err := json.Marshal(settings)
+	in, err := ioutil.ReadFile(c.configFile)
+	if err != nil {
+		return err
+	}
+	var cfg map[string]interface{}
+	if err := json.Unmarshal(in, &cfg); err != nil {
+		return err
+	}
+	delete(cfg, key)
+	bin, err := json.MarshalIndent(cfg, "", "  ")
 	if err != nil {
 		return err
 	}
-	if err = c.viper.ReadConfig(bytes.NewReader(bin)); err != nil {
+	if err := atomicWrite(bin, c.configFile); err != nil {
 		return err
 	}
-	return atomicWrite(c.viper, c.configFile)
+	return c.viper.ReadInConfig()
 }
 
 // BindFlagset binds a flagset to their respective config properties
@@ -76,7 +97,7 @@ func ensureConfigFileExists(file string) error {
 	return err
 }
 
-func atomicWrite(viper *viper.Viper, configFile string) error {
+func atomicWrite(bin []byte, configFile string) error {
 	ext := filepath.Ext(configFile)
 	pattern := fmt.Sprintf("%s*%s", strings.TrimSuffix(filepath.Base(configFile), ext), ext)
 	tmpFile, err := ioutil.TempFile(filepath.Dir(configFile), pattern)
@@ -89,7 +110,7 @@ func atomicWrite(viper *viper.Viper, configFile string) error {
 	if err := tmpFile.Close(); err != nil {
 		return err
 	}
-	if err := viper.WriteConfigAs(tmpFile.Name()); err != nil {
+	if err := ioutil.WriteFile(tmpFile.Name(), bin, 0600); err != nil {
 		return err
 	}
 	return os.Rename(tmpFile.Name(), configFile)
diff --git a/pkg/crc/errors/multierror.go b/pkg/crc/errors/multierror.go
index 6d811b481..ea917c9f5 100644
--- a/pkg/crc/errors/multierror.go
+++ b/pkg/crc/errors/multierror.go
@@ -16,6 +16,18 @@ func (v vmNotExist) Error() string {
 
 const VMNotExist vmNotExist = "Machine does not exist. Use 'crc start' to create it"
 
+type PreflightError struct {
+	Err error
+}
+
+func (p *PreflightError) Error() string {
+	return p.Err.Error()
+}
+
+func (p *PreflightError) Unwrap() error {
+	return p.Err
+}
+
 type MultiError struct {
 	Errors []error
 }
diff --git a/pkg/crc/preflight/preflight.go b/pkg/crc/preflight/preflight.go
index ada6d0e19..00a4a943d 100644
--- a/pkg/crc/preflight/preflight.go
+++ b/pkg/crc/preflight/preflight.go
@@ -153,7 +153,10 @@ func doRegisterSettings(cfg config.Schema, checks []Check) {
 func StartPreflightChecks(config config.Storage) error {
 	experimentalFeatures := config.Get(cmdConfig.ExperimentalFeatures).AsBool()
 	mode := network.ParseMode(config.Get(cmdConfig.NetworkMode).AsString())
-	return doPreflightChecks(config, getPreflightChecks(experimentalFeatures, mode))
+	if err := doPreflightChecks(config, getPreflightChecks(experimentalFeatures, mode)); err != nil {
+		return &errors.PreflightError{Err: err}
+	}
+	return nil
 }
 
 // SetupHost performs the prerequisite checks and setups the host to run the cluster


Test patch:
diff --git a/pkg/crc/config/viper_config_test.go b/pkg/crc/config/viper_config_test.go
index 11f10c0cb..88819e8d7 100644
--- a/pkg/crc/config/viper_config_test.go
+++ b/pkg/crc/config/viper_config_test.go
@@ -157,6 +157,7 @@ func TestViperConfigBindFlagSet(t *testing.T) {
 	flagSet := pflag.NewFlagSet("start", pflag.ExitOnError)
 	flagSet.IntP(CPUs, "c", 4, "")
 	flagSet.StringP(NameServer, "n", "", "")
+	flagSet.StringP("extra", "e", "", "")
 
 	_ = storage.BindFlagSet(flagSet)
 
@@ -179,10 +180,9 @@ func TestViperConfigBindFlagSet(t *testing.T) {
 	_, err = config.Set(CPUs, "6")
 	assert.NoError(t, err)
 
-	assert.Equal(t, SettingValue{
-		Value:     6,
-		IsDefault: false,
-	}, config.Get(CPUs))
+	bin, err := ioutil.ReadFile(configFile)
+	assert.NoError(t, err)
+	assert.JSONEq(t, `{"cpus":6}`, string(bin))
 }
 
 func TestViperConfigCastSet(t *testing.T) {