Repository: code-ready/crc
Pull request #1960 (2021-02-08 12:07:38)

Add Help parameter for config setting struct
This patch will make use of help string for key and show output
as following.
```
* bundle                          Bundle path (string, default '/home/prkumar/.crc/cache/crc_libvirt_4.6.15.crcbundle')
* consent-telemetry               Send feedback about CodeReady Containers (yes/no)
* cpus                            Number of CPU cores (integer, default '4')
* disable-update-check            Don't check for update (default false)
* disk-size                       Total size in GiB of the disk (integer, default '31')
* enable-cluster-monitoring       Enable cluster monitoring operator (default false)
```

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

This pull-request has been approved by: *<a href="https://github.com/code-ready/crc/pull/1960#" 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/1960#pullrequestreview-585659936" title="Approved">guillaumerose</a>*, *<a href="https://github.com/code-ready/crc/pull/1960#" 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)~~ [guillaumerose,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.
praveenkumar: /assign @robin-owen 
openshift-ci[bot]: @praveenkumar: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
ci/prow/e2e-crc | 23bde64b88afad0c24086e9e0542805616ede681 | [link](https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/code-ready_crc/1960/pull-ci-code-ready-crc-master-e2e-crc/1359740730632835072) | `/test e2e-crc`

[Full PR test history](https://prow.ci.openshift.org/pr-history?org=code-ready&repo=crc&pr=1960). [Your PR dashboard](https://prow.ci.openshift.org/pr?query=is:pr+state:open+author:praveenkumar).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
praveenkumar: /retest

Patch:
diff --git a/cmd/crc/cmd/config/config.go b/cmd/crc/cmd/config/config.go
index 6a30d580d..249b323a1 100644
--- a/cmd/crc/cmd/config/config.go
+++ b/cmd/crc/cmd/config/config.go
@@ -33,25 +33,40 @@ const (
 
 func RegisterSettings(cfg *config.Config) {
 	// Start command settings in config
-	cfg.AddSetting(Bundle, constants.DefaultBundlePath, config.ValidateBundlePath, config.SuccessfullyApplied)
-	cfg.AddSetting(CPUs, constants.DefaultCPUs, config.ValidateCPUs, config.RequiresRestartMsg)
-	cfg.AddSetting(Memory, constants.DefaultMemory, config.ValidateMemory, config.RequiresRestartMsg)
-	cfg.AddSetting(DiskSize, constants.DefaultDiskSize, config.ValidateDiskSize, config.RequiresRestartMsg)
-	cfg.AddSetting(NameServer, "", config.ValidateIPAddress, config.SuccessfullyApplied)
-	cfg.AddSetting(PullSecretFile, "", config.ValidatePath, config.SuccessfullyApplied)
-	cfg.AddSetting(DisableUpdateCheck, false, config.ValidateBool, config.SuccessfullyApplied)
-	cfg.AddSetting(ExperimentalFeatures, false, config.ValidateBool, config.SuccessfullyApplied)
-	cfg.AddSetting(NetworkMode, string(network.DefaultMode), network.ValidateMode, network.SuccessfullyAppliedMode)
+	cfg.AddSetting(Bundle, constants.DefaultBundlePath, config.ValidateBundlePath, config.SuccessfullyApplied,
+		fmt.Sprintf("Bundle path (string, default '%s')", constants.DefaultBundlePath))
+	cfg.AddSetting(CPUs, constants.DefaultCPUs, config.ValidateCPUs, config.RequiresRestartMsg,
+		fmt.Sprintf("Number of CPU cores (must be greater than or equal to '%d')", constants.DefaultCPUs))
+	cfg.AddSetting(Memory, constants.DefaultMemory, config.ValidateMemory, config.RequiresRestartMsg,
+		fmt.Sprintf("Memory size in MiB (must be greater than or equal to '%d')", constants.DefaultMemory))
+	cfg.AddSetting(DiskSize, constants.DefaultDiskSize, config.ValidateDiskSize, config.RequiresRestartMsg,
+		fmt.Sprintf("Total size in GiB of the disk (must be greater than or equal to '%d')", constants.DefaultDiskSize))
+	cfg.AddSetting(NameServer, "", config.ValidateIPAddress, config.SuccessfullyApplied,
+		"IPv4 address of nameserver (string, like '1.1.1.1 or 8.8.8.8')")
+	cfg.AddSetting(PullSecretFile, "", config.ValidatePath, config.SuccessfullyApplied,
+		fmt.Sprintf("Path of image pull secret (download from %s)", constants.CrcLandingPageURL))
+	cfg.AddSetting(DisableUpdateCheck, false, config.ValidateBool, config.SuccessfullyApplied,
+		"Disable update check (true/false, default: false)")
+	cfg.AddSetting(ExperimentalFeatures, false, config.ValidateBool, config.SuccessfullyApplied,
+		"Enable experimental features (true/false, default: false)")
+	cfg.AddSetting(NetworkMode, string(network.DefaultMode), network.ValidateMode, network.SuccessfullyAppliedMode,
+		"Network mode (default or vsock)")
 	// Proxy Configuration
-	cfg.AddSetting(HTTPProxy, "", config.ValidateURI, config.SuccessfullyApplied)
-	cfg.AddSetting(HTTPSProxy, "", config.ValidateURI, config.SuccessfullyApplied)
-	cfg.AddSetting(NoProxy, "", config.ValidateNoProxy, config.SuccessfullyApplied)
-	cfg.AddSetting(ProxyCAFile, "", config.ValidatePath, config.SuccessfullyApplied)
+	cfg.AddSetting(HTTPProxy, "", config.ValidateURI, config.SuccessfullyApplied,
+		"HTTP proxy URL (string, like 'http://my-proxy.com:8443')")
+	cfg.AddSetting(HTTPSProxy, "", config.ValidateURI, config.SuccessfullyApplied,
+		"HTTPS proxy URL (string, like 'https://my-proxy.com:8443')")
+	cfg.AddSetting(NoProxy, "", config.ValidateNoProxy, config.SuccessfullyApplied,
+		"Hosts, ipv4 addresses or CIDR which do not use a proxy (string, comma-separated list such as '127.0.0.1,192.168.100.1/24')")
+	cfg.AddSetting(ProxyCAFile, "", config.ValidatePath, config.SuccessfullyApplied,
+		"Path to an HTTPS proxy certificate authority (CA)")
 
-	cfg.AddSetting(EnableClusterMonitoring, false, config.ValidateBool, config.SuccessfullyApplied)
+	cfg.AddSetting(EnableClusterMonitoring, false, config.ValidateBool, config.SuccessfullyApplied,
+		"Enable cluster monitoring Operator (true/false, default: false)")
 
 	// Telemeter Configuration
-	cfg.AddSetting(ConsentTelemetry, "", config.ValidateYesNo, config.SuccessfullyApplied)
+	cfg.AddSetting(ConsentTelemetry, "", config.ValidateYesNo, config.SuccessfullyApplied,
+		"Consent to collection of anonymous usage data (yes/no)")
 }
 
 func isPreflightKey(key string) bool {
@@ -80,42 +95,19 @@ func less(lhsKey, rhsKey string) bool {
 }
 
 func configurableFields(config *config.Config) string {
-	var fields []string
+	settings := config.AllSettings()
+	sort.Slice(settings, func(i, j int) bool {
+		return less(settings[i].Name, settings[j].Name)
+	})
+
 	var buf bytes.Buffer
 	writer := tabwriter.NewWriter(&buf, 0, 8, 1, ' ', tabwriter.TabIndent)
-	for _, keyAndValueType := range keysAndValueType(config) {
-		fmt.Fprintln(writer, keyAndValueType)
+	for _, cfg := range settings {
+		fmt.Fprintf(writer, "* %s\t%s\n", cfg.Name, cfg.Help)
 	}
 	writer.Flush()
-	keys := strings.Split(buf.String(), "\n")
-	sort.Slice(keys, func(i, j int) bool {
-		return less(keys[i], keys[j])
-	})
-	for _, key := range keys {
-		if key == "" {
-			continue
-		}
-		fields = append(fields, " * "+key)
-	}
-	return strings.Join(fields, "\n")
-}
-func keysAndValueType(config *config.Config) []string {
-	var keyAndValueType []string
-	for key, value := range config.AllConfigs() {
-		var valueType string
-		switch value.Value.(type) {
-		case int:
-			valueType = "Number"
-		case string:
-			valueType = "String"
-		case bool:
-			valueType = "true/false"
-		default:
-			valueType = fmt.Sprintf("%T", value.Value)
-		}
-		keyAndValueType = append(keyAndValueType, fmt.Sprintf("%s\t%s", key, valueType))
-	}
-	return keyAndValueType
+
+	return buf.String()
 }
 
 func GetConfigCmd(config *config.Config) *cobra.Command {
diff --git a/cmd/crc/cmd/config/set.go b/cmd/crc/cmd/config/set.go
index e13b40f76..cec3e8469 100644
--- a/cmd/crc/cmd/config/set.go
+++ b/cmd/crc/cmd/config/set.go
@@ -9,11 +9,12 @@ import (
 	"github.com/spf13/cobra"
 )
 
-func configSetCmd(config config.Storage) *cobra.Command {
+func configSetCmd(config *config.Config) *cobra.Command {
 	return &cobra.Command{
 		Use:   "set CONFIG-KEY VALUE",
 		Short: "Set a crc configuration property",
-		Long:  `Sets a crc configuration property.`,
+		Long: `Sets a crc configuration property.
+CONFIG-KEYS: ` + "\n\n" + configurableFields(config),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			if len(args) < 2 {
 				return errors.New("Please provide a configuration property and its value as in 'crc config set KEY VALUE'")
diff --git a/pkg/crc/config/config.go b/pkg/crc/config/config.go
index 7ce9b9220..13b620a69 100644
--- a/pkg/crc/config/config.go
+++ b/pkg/crc/config/config.go
@@ -37,14 +37,23 @@ func (c *Config) AllConfigs() map[string]SettingValue {
 	return allConfigs
 }
 
+func (c *Config) AllSettings() []Setting {
+	var settings []Setting
+	for _, setting := range c.settingsByName {
+		settings = append(settings, setting)
+	}
+	return settings
+}
+
 // AddSetting returns a filled struct of ConfigSetting
 // takes the config name and default value as arguments
-func (c *Config) AddSetting(name string, defValue interface{}, validationFn ValidationFnType, callbackFn SetFn) {
+func (c *Config) AddSetting(name string, defValue interface{}, validationFn ValidationFnType, callbackFn SetFn, help string) {
 	c.settingsByName[name] = Setting{
 		Name:         name,
 		defaultValue: defValue,
 		validationFn: validationFn,
 		callbackFn:   callbackFn,
+		Help:         help,
 	}
 }
 
diff --git a/pkg/crc/config/types.go b/pkg/crc/config/types.go
index d0365e539..174fc7670 100644
--- a/pkg/crc/config/types.go
+++ b/pkg/crc/config/types.go
@@ -10,7 +10,7 @@ type Storage interface {
 }
 
 type Schema interface {
-	AddSetting(name string, defValue interface{}, validationFn ValidationFnType, callbackFn SetFn)
+	AddSetting(name string, defValue interface{}, validationFn ValidationFnType, callbackFn SetFn, help string)
 }
 
 type Setting struct {
@@ -18,6 +18,7 @@ type Setting struct {
 	defaultValue interface{}
 	validationFn ValidationFnType
 	callbackFn   SetFn
+	Help         string
 }
 
 type SettingValue struct {
diff --git a/pkg/crc/preflight/preflight.go b/pkg/crc/preflight/preflight.go
index 00a4a943d..70dd886f1 100644
--- a/pkg/crc/preflight/preflight.go
+++ b/pkg/crc/preflight/preflight.go
@@ -144,7 +144,8 @@ func doCleanUpPreflightChecks(checks []Check) error {
 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.getSkipConfigName(), false, config.ValidateBool, config.SuccessfullyApplied,
+				"Skip preflight check (true/false, default: false)")
 		}
 	}
 }


Test patch:
diff --git a/pkg/crc/config/viper_config_test.go b/pkg/crc/config/viper_config_test.go
index 5002d02a2..11f10ff5a 100644
--- a/pkg/crc/config/viper_config_test.go
+++ b/pkg/crc/config/viper_config_test.go
@@ -22,8 +22,8 @@ func newTestConfig(configFile, envPrefix string) (*Config, error) {
 		return nil, err
 	}
 	config := New(storage)
-	config.AddSetting(CPUs, 4, ValidateCPUs, RequiresRestartMsg)
-	config.AddSetting(NameServer, "", ValidateIPAddress, SuccessfullyApplied)
+	config.AddSetting(CPUs, 4, ValidateCPUs, RequiresRestartMsg, "")
+	config.AddSetting(NameServer, "", ValidateIPAddress, SuccessfullyApplied, "")
 	return config, nil
 }
 
@@ -151,8 +151,8 @@ func TestViperConfigBindFlagSet(t *testing.T) {
 	storage, err := NewViperStorage(configFile, "CRC")
 	require.NoError(t, err)
 	config := New(storage)
-	config.AddSetting(CPUs, 4, ValidateCPUs, RequiresRestartMsg)
-	config.AddSetting(NameServer, "", ValidateIPAddress, SuccessfullyApplied)
+	config.AddSetting(CPUs, 4, ValidateCPUs, RequiresRestartMsg, "")
+	config.AddSetting(NameServer, "", ValidateIPAddress, SuccessfullyApplied, "")
 
 	flagSet := pflag.NewFlagSet("start", pflag.ExitOnError)
 	flagSet.IntP(CPUs, "c", 4, "")