Repository: code-ready/crc
Pull request #1816 (2021-01-05 14:25:06)

Remove warn flags from the config
`warn-<preflight-check>` is not used in crc context because we
are already capturing the checks data as part of debug logs. `skip`
is there to skip a check if a user don't want to run a check.

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

This pull-request has been approved by: *<a href="https://github.com/code-ready/crc/pull/1816#" 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":[]} -->
openshift-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/code-ready/crc/pull/1816#pullrequestreview-561831138" title="Approved">cfergeau</a>*, *<a href="https://github.com/code-ready/crc/pull/1816#" 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)~~ [cfergeau,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":[]} -->
openshift-ci-robot: New changes are detected. LGTM label has been removed.

Patch:
diff --git a/cmd/crc/cmd/config/config.go b/cmd/crc/cmd/config/config.go
index 6a191653b..4aa2e4cdb 100644
--- a/cmd/crc/cmd/config/config.go
+++ b/cmd/crc/cmd/config/config.go
@@ -52,22 +52,15 @@ func RegisterSettings(cfg *config.Config) {
 }
 
 func isPreflightKey(key string) bool {
-	return strings.HasPrefix(key, "skip-") || strings.HasPrefix(key, "warn-")
+	return strings.HasPrefix(key, "skip-")
 }
 
 // less is used to sort the config keys. We want to sort first the regular keys, and
-// then the keys related to preflight starting with a skip- or warn- prefix. We want
-// these preflight keys to be grouped by pair: 'skip-bar', 'warn-bar', 'skip-foo', 'warn-foo'
-// would be sorted in that order.
+// then the keys related to preflight starting with a skip- prefix.
 func less(lhsKey, rhsKey string) bool {
 	if isPreflightKey(lhsKey) {
 		if isPreflightKey(rhsKey) {
-			// lhs is preflight, rhs is preflight
-			if lhsKey[4:] == rhsKey[4:] {
-				// we want skip-foo before warn-foo
-				return lhsKey < rhsKey
-			}
-			// ignore skip-/warn- prefix
+			// ignore skip prefix
 			return lhsKey[4:] < rhsKey[4:]
 		}
 		// lhs is preflight, rhs is not preflight
diff --git a/cmd/crc/cmd/root.go b/cmd/crc/cmd/root.go
index deac2af5f..36e7a13b2 100644
--- a/cmd/crc/cmd/root.go
+++ b/cmd/crc/cmd/root.go
@@ -174,9 +174,10 @@ func addForceFlag(cmd *cobra.Command) {
 
 func executeWithLogging(fullCmd string, input func(cmd *cobra.Command, args []string) error) func(cmd *cobra.Command, args []string) error {
 	return func(cmd *cobra.Command, args []string) error {
+		logging.Debugf("Running '%s'", fullCmd)
 		if err := input(cmd, args); err != nil {
 			if serr := segmentClient.Upload(fullCmd, err); serr != nil {
-				fmt.Println(serr.Error())
+				logging.Debugf("Cannot send data to telemetry: %v", serr)
 			}
 			return err
 		}
diff --git a/docs/source/topics/con_about-codeready-containers-configuration.adoc b/docs/source/topics/con_about-codeready-containers-configuration.adoc
index 160bc0043..8c858f24c 100644
--- a/docs/source/topics/con_about-codeready-containers-configuration.adoc
+++ b/docs/source/topics/con_about-codeready-containers-configuration.adoc
@@ -9,4 +9,4 @@ Run the [command]`{bin} config --help` command to list the available properties.
 
 You can also use the [command]`{bin} config` command to configure the behavior of the startup checks for the [command]`{bin} start` and [command]`{bin} setup` commands.
 By default, startup checks report an error and stop execution when their conditions are not met.
-Set the value of a property starting with `skip-check` or `warn-check` to `true` to skip the check or report a warning rather than an error, respectively.
+Set the value of a property starting with `skip-check` to `true` to skip the check.
diff --git a/pkg/crc/preflight/preflight.go b/pkg/crc/preflight/preflight.go
index 7652758ab..ada6d0e19 100644
--- a/pkg/crc/preflight/preflight.go
+++ b/pkg/crc/preflight/preflight.go
@@ -50,20 +50,6 @@ func (check *Check) shouldSkip(config config.Storage) bool {
 	return config.Get(check.getSkipConfigName()).AsBool()
 }
 
-func (check *Check) getWarnConfigName() string {
-	if check.configKeySuffix == "" {
-		return ""
-	}
-	return "warn-" + check.configKeySuffix
-}
-
-func (check *Check) shouldWarn(config config.Storage) bool {
-	if check.configKeySuffix == "" {
-		return false
-	}
-	return config.Get(check.getWarnConfigName()).AsBool()
-}
-
 func (check *Check) doCheck(config config.Storage) error {
 	if check.checkDescription == "" {
 		panic(fmt.Sprintf("Should not happen, empty description for check '%s'", check.configKeySuffix))
@@ -110,13 +96,8 @@ func doPreflightChecks(config config.Storage, checks []Check) error {
 		if check.flags&SetupOnly == SetupOnly || check.flags&CleanUpOnly == CleanUpOnly {
 			continue
 		}
-		err := check.doCheck(config)
-		if err != nil {
-			if check.shouldWarn(config) {
-				logging.Warn(err.Error())
-			} else {
-				return err
-			}
+		if err := check.doCheck(config); err != nil {
+			return err
 		}
 	}
 	return nil
@@ -131,13 +112,8 @@ func doFixPreflightChecks(config config.Storage, checks []Check) error {
 		if err == nil {
 			continue
 		}
-		err = check.doFix()
-		if err != nil {
-			if check.shouldWarn(config) {
-				logging.Warn(err.Error())
-			} else {
-				return err
-			}
+		if err = check.doFix(); err != nil {
+			return err
 		}
 	}
 	return nil
@@ -169,7 +145,6 @@ func doRegisterSettings(cfg config.Schema, checks []Check) {
 	for _, check := range checks {
 		if check.configKeySuffix != "" {
 			cfg.AddSetting(check.getSkipConfigName(), false, config.ValidateBool, config.SuccessfullyApplied)
-			cfg.AddSetting(check.getWarnConfigName(), false, config.ValidateBool, config.SuccessfullyApplied)
 		}
 	}
 }
diff --git a/pkg/os/linux/release_info.go b/pkg/os/linux/release_info.go
index ce9a361af..836a6e8ce 100644
--- a/pkg/os/linux/release_info.go
+++ b/pkg/os/linux/release_info.go
@@ -92,9 +92,7 @@ func UnmarshalOsRelease(osReleaseContents []byte, release *OsRelease) error {
 			logging.Warnf("Warning: got an invalid line error parsing /etc/os-release: %s", err)
 			continue
 		}
-		if err := release.setIfPossible(key, val); err != nil {
-			logging.Debug(err)
-		}
+		_ = release.setIfPossible(key, val)
 	}
 	return nil
 }


Test patch:
diff --git a/pkg/crc/preflight/preflight_darwin_test.go b/pkg/crc/preflight/preflight_darwin_test.go
index 9b0b840c6..5f94d8c75 100644
--- a/pkg/crc/preflight/preflight_darwin_test.go
+++ b/pkg/crc/preflight/preflight_darwin_test.go
@@ -12,7 +12,7 @@ import (
 func TestCountConfigurationOptions(t *testing.T) {
 	cfg := config.New(config.NewEmptyInMemoryStorage())
 	RegisterSettings(cfg)
-	assert.Len(t, cfg.AllConfigs(), 18)
+	assert.Len(t, cfg.AllConfigs(), 9)
 }
 
 func TestCountPreflights(t *testing.T) {
diff --git a/pkg/crc/preflight/preflight_linux_test.go b/pkg/crc/preflight/preflight_linux_test.go
index c0cd327ac..3b9e22649 100644
--- a/pkg/crc/preflight/preflight_linux_test.go
+++ b/pkg/crc/preflight/preflight_linux_test.go
@@ -21,7 +21,7 @@ func TestCountConfigurationOptions(t *testing.T) {
 	var preflightChecksCount int
 	for _, check := range getAllPreflightChecks() {
 		if check.configKeySuffix != "" {
-			preflightChecksCount += 2
+			preflightChecksCount++
 		}
 	}
 	assert.True(t, options == preflightChecksCount, "Unexpected number of preflight configuration flags, got %d, expected %d", options, preflightChecksCount)
diff --git a/pkg/crc/preflight/preflight_test.go b/pkg/crc/preflight/preflight_test.go
index 47726a8b0..c624566ca 100644
--- a/pkg/crc/preflight/preflight_test.go
+++ b/pkg/crc/preflight/preflight_test.go
@@ -39,18 +39,6 @@ func TestFixPreflight(t *testing.T) {
 	assert.True(t, calls.fixed)
 }
 
-func TestWarnPreflight(t *testing.T) {
-	check, calls := sampleCheck(errors.New("check failed"), errors.New("fix failed"))
-	cfg := config.New(config.NewEmptyInMemoryStorage())
-	doRegisterSettings(cfg, []Check{*check})
-	_, err := cfg.Set("warn-sample", true)
-	assert.NoError(t, err)
-
-	assert.NoError(t, doFixPreflightChecks(cfg, []Check{*check}))
-	assert.True(t, calls.checked)
-	assert.True(t, calls.fixed)
-}
-
 func sampleCheck(checkErr, fixErr error) (*Check, *status) {
 	status := &status{}
 	return &Check{
diff --git a/pkg/crc/preflight/preflight_windows_test.go b/pkg/crc/preflight/preflight_windows_test.go
index b9e84e819..ac08d4bda 100644
--- a/pkg/crc/preflight/preflight_windows_test.go
+++ b/pkg/crc/preflight/preflight_windows_test.go
@@ -11,7 +11,7 @@ import (
 func TestCountConfigurationOptions(t *testing.T) {
 	cfg := config.New(config.NewEmptyInMemoryStorage())
 	RegisterSettings(cfg)
-	assert.Len(t, cfg.AllConfigs(), 24)
+	assert.Len(t, cfg.AllConfigs(), 12)
 }
 
 func TestCountPreflights(t *testing.T) {
diff --git a/test/integration/features/config.feature b/test/integration/features/config.feature
index dd323b76c..8ce0c47b3 100644
--- a/test/integration/features/config.feature
+++ b/test/integration/features/config.feature
@@ -56,55 +56,6 @@ Feature: Test configuration settings
         When unsetting config property "disable-update-check" succeeds
         Then "JSON" config file "crc.json" in CRC home folder does not contain key "disable-update-check"
 
-    # WARNINGS
-
-    Scenario Outline: CRC config checks (warnings)
-        When setting config property "<property>" to value "<value1>" succeeds
-        Then "JSON" config file "crc.json" in CRC home folder contains key "<property>" with value matching "<value1>"
-        When setting config property "<property>" to value "<value2>" succeeds
-        Then "JSON" config file "crc.json" in CRC home folder contains key "<property>" with value matching "<value2>"
-        When unsetting config property "<property>" succeeds
-        Then "JSON" config file "crc.json" in CRC home folder does not contain key "<property>"
-
-        @darwin
-        Examples: Config warnings on Mac
-            | property                             | value1 | value2 |
-            | warn-check-bundle-extracted          | true   | false  |
-            | warn-check-hosts-file-permissions    | true   | false  |
-            | warn-check-hyperkit-driver           | true   | false  |
-            | warn-check-hyperkit-installed        | true   | false  |
-            | warn-check-resolver-file-permissions | true   | false  |
-            | warn-check-root-user                 | true   | false  |
-
-        @linux
-        Examples: Config warnings on Linux
-            | property                             | value1 | value2 |
-            | warn-check-bundle-extracted          | true   | false  |
-            | warn-check-crc-dnsmasq-file          | true   | false  |
-            | warn-check-crc-network               | true   | false  |
-            | warn-check-crc-network-active        | true   | false  |
-            | warn-check-kvm-enabled               | true   | false  |
-            | warn-check-libvirt-driver            | true   | false  |
-            | warn-check-libvirt-installed         | true   | false  |
-            | warn-check-libvirt-running           | true   | false  |
-            | warn-check-libvirt-version           | true   | false  |
-            | warn-check-network-manager-config    | true   | false  |
-            | warn-check-network-manager-installed | true   | false  |
-            | warn-check-network-manager-running   | true   | false  |
-            | warn-check-root-user                 | true   | false  |
-            | warn-check-user-in-libvirt-group     | true   | false  |
-            | warn-check-virt-enabled              | true   | false  |
-
-        @windows
-        Examples: Config warnings on Windows
-            | property                        | value1 | value2 |
-            | warn-check-administrator-user   | true   | false  |
-            | warn-check-bundle-extracted     | true   | false  |
-            | warn-check-hyperv-installed     | true   | false  |
-            | warn-check-hyperv-switch        | true   | false  |
-            | warn-check-user-in-hyperv-group | true   | false  |
-            | warn-check-windows-version      | true   | false  |
-
     # SKIP
 
     Scenario Outline: CRC config checks (skips)