Merge pull request openshift#1384 from pacevedom/USHIFT-387

USHIFT-387: simplify configuration mechanisms

docs/howto_config.md

@@ -1,10 +1,4 @@
 # Configuration
-
-MicroShift can be configured in the following ways, in order of precedence:
-* Command line arguments
-* Environment variables
-* Configuration file
-
 The MicroShift configuration file must be located at `~/.microshift/config.yaml` (user-specific) and `/etc/microshift/config.yaml` (system-wide), while the former takes precedence if it exists. A sample `/etc/microshift/config.yaml.default` configuration file is installed by the MicroShift RPM and it can be used as a template when customizing MicroShift.
 
 The format of the `config.yaml` configuration file is as follows.
@@ -28,19 +22,6 @@ debugging:
   logLevel: ""
 ```
 
-The configuration settings alongside with the supported command line arguments and environment variables are presented below.
-
-| Field Name            | CLI Argument              | Environment Variable                    | Description |
-|-----------------------|---------------------------|-----------------------------------------|-------------|
-| cidr (clusterNetwork) | --cluster-cidr            | MICROSHIFT_CLUSTER_CLUSTERCIDR          | A block of IP addresses from which Pod IP addresses are allocated
-| serviceNetwork        | --service-cidr            | MICROSHIFT_CLUSTER_SERVICECIDR          | A block of virtual IP addresses for Kubernetes services
-| serviceNodePortRange  | --service-node-port-range | MICROSHIFT_CLUSTER_SERVICENODEPORTRANGE | The port range allowed for Kubernetes services of type NodePort
-| baseDomain            | --base-domain             | MICROSHIFT_BASEDOMAIN                   | Base domain of the cluster. All managed DNS records will be sub-domains of this base.
-| nodeIP                | --node-ip                 | MICROSHIFT_NODEIP                       | The IP address of the node, defaults to IP of the default route
-| hostnameOverride      | --hostname-override       | MICROSHIFT_HOSTNAMEOVERRIDE             | The name of the node, defaults to hostname
-| logLevel              | --v                       | MICROSHIFT_LOGVLEVEL                    | Log verbosity (Normal, Debug, Trace, TraceAll)
-| subjectAltNames       | --subject-alt-names       | MICROSHIFT_SUBJECTALTNAMES              | Subject Alternative Names for apiserver certificates
-
 ## Default Settings
 
 In case `config.yaml` is not provided, the following default settings will be used.

go.mod

@@ -5,7 +5,6 @@ go 1.19
 require (
 	github.com/apparentlymart/go-cidr v1.1.0
 	github.com/coreos/go-systemd v0.0.0-20190321100706-95778dfbb74e // openshift-controller-manager
-	github.com/kelseyhightower/envconfig v1.4.0 // microshift
 	github.com/miekg/dns v1.1.35 // microshift
 	github.com/mitchellh/go-homedir v1.1.0 // microshift
 	github.com/openshift/api v0.0.0-20230201213816-61d971884921

go.sum

@@ -489,8 +489,6 @@ github.com/julienschmidt/httprouter v1.2.0/go.mod h1:SYymIcj16QtmaHHD7aYtjjsJG7V
 github.com/julienschmidt/httprouter v1.3.0/go.mod h1:JR6WtHb+2LUe8TCKY3cZOxFyyO8IZAc4RVcycCCAKdM=
 github.com/karrick/godirwalk v1.17.0 h1:b4kY7nqDdioR/6qnbHQyDvmA17u5G1cZ6J+CZXwSWoI=
 github.com/karrick/godirwalk v1.17.0/go.mod h1:j4mkqPuvaLI8mp1DroR3P6ad7cyYd4c1qeJ3RV7ULlk=
-github.com/kelseyhightower/envconfig v1.4.0 h1:Im6hONhd3pLkfDFsbRgu68RDNkGF1r3dvMUtDTo2cv8=
-github.com/kelseyhightower/envconfig v1.4.0/go.mod h1:cccZRl6mQpaq41TPp5QxidR+Sa3axMbJDNb//FQX6Gg=
 github.com/kisielk/errcheck v1.1.0/go.mod h1:EZBBE59ingxPouuu3KfxchcWSUPOHkagtvWXihfKN4Q=
 github.com/kisielk/errcheck v1.2.0/go.mod h1:/BMXB+zMLi60iA8Vv6Ksmxu/1UDYcXs4uQLJ+jE2L00=
 github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8=

pkg/cmd/run.go

@@ -18,7 +18,6 @@ import (
 	"github.com/openshift/microshift/pkg/util"
 	"github.com/openshift/microshift/pkg/util/cryptomaterial/certchains"
 	"github.com/spf13/cobra"
-	"github.com/spf13/pflag"
 
 	"k8s.io/klog/v2"
 )
@@ -27,36 +26,23 @@ const (
 	gracefulShutdownTimeout = 60
 )
 
-func addRunFlags(cmd *cobra.Command, cfg *config.MicroshiftConfig) {
-	flags := cmd.Flags()
-	// All other flags will be read after reading both config file and env vars.
-	flags.String("hostname-override", cfg.NodeName, "The name to use to identify this node instead of the hostname.")
-	flags.String("node-ip", cfg.NodeIP, "The IP address of the node.")
-	flags.String("cluster-cidr", cfg.Cluster.ClusterCIDR, "The IP range in CIDR notation for pods in the cluster.")
-	flags.String("service-cidr", cfg.Cluster.ServiceCIDR, "The IP range in CIDR notation for services in the cluster.")
-	flags.String("service-node-port-range", cfg.Cluster.ServiceNodePortRange, "The port range to reserve for services with NodePort visibility. This must not overlap with the ephemeral port range on nodes.")
-	flags.String("base-domain", cfg.BaseDomain, "The base domain for this cluster.")
-}
-
 func NewRunMicroshiftCommand() *cobra.Command {
 	cfg := config.NewMicroshiftConfig()
 
 	cmd := &cobra.Command{
 		Use:   "run",
 		Short: "Run MicroShift",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return RunMicroshift(cfg, cmd.Flags())
+			return RunMicroshift(cfg)
 		},
 	}
 
-	addRunFlags(cmd, cfg)
-
 	return cmd
 }
 
-func RunMicroshift(cfg *config.MicroshiftConfig, flags *pflag.FlagSet) error {
-	if err := cfg.ReadAndValidate(config.GetConfigFile(), flags); err != nil {
-		klog.Fatalf("Error in reading and validating flags: %v", err)
+func RunMicroshift(cfg *config.MicroshiftConfig) error {
+	if err := cfg.ReadAndValidate(config.GetConfigFile()); err != nil {
+		klog.Fatalf("Error in reading or validating configuration: %v", err)
 	}
 
 	// fail early if we don't have enough privileges

pkg/cmd/showConfig.go

@@ -35,7 +35,7 @@ func NewShowConfigCommand(ioStreams genericclioptions.IOStreams) *cobra.Command
 				cfg.NodeName = ""
 			case "effective":
 				// Load the current configuration
-				if err := cfg.ReadAndValidate(config.GetConfigFile(), cmd.Flags()); err != nil {
+				if err := cfg.ReadAndValidate(config.GetConfigFile()); err != nil {
 					cmdutil.CheckErr(err)
 				}
 			default:
@@ -78,7 +78,6 @@ func NewShowConfigCommand(ioStreams genericclioptions.IOStreams) *cobra.Command
 
 	flags := cmd.Flags()
 	flags.StringVarP(&opts.Mode, "mode", "m", opts.Mode, "One of 'default' or 'effective'.")
-	addRunFlags(cmd, cfg)
 
 	return cmd
 }

pkg/config/config.go

@@ -13,7 +13,6 @@ import (
 	"strings"
 
 	"github.com/apparentlymart/go-cidr/cidr"
-	"github.com/kelseyhightower/envconfig"
 	"github.com/mitchellh/go-homedir"
 	"github.com/spf13/pflag"
 
@@ -384,56 +383,14 @@ func (c *MicroshiftConfig) ReadFromConfigFile(configFile string) error {
 	return nil
 }
 
-func (c *MicroshiftConfig) ReadFromEnv() error {
-	if err := envconfig.Process("microshift", c); err != nil {
-		return err
-	}
-	return nil
-}
-
-func (c *MicroshiftConfig) ReadFromCmdLine(flags *pflag.FlagSet) error {
-	if f := flags.Lookup("v"); f != nil && flags.Changed("v") {
-		c.LogVLevel, _ = strconv.Atoi(f.Value.String())
-	}
-	if s, err := flags.GetStringSlice("subject-alt-names"); err == nil && flags.Changed("subject-alt-names") {
-		c.SubjectAltNames = s
-	}
-	if s, err := flags.GetString("hostname-override"); err == nil && flags.Changed("hostname-override") {
-		c.NodeName = s
-	}
-	if s, err := flags.GetString("node-ip"); err == nil && flags.Changed("node-ip") {
-		c.NodeIP = s
-	}
-	if s, err := flags.GetString("cluster-cidr"); err == nil && flags.Changed("cluster-cidr") {
-		c.Cluster.ClusterCIDR = s
-	}
-	if s, err := flags.GetString("service-cidr"); err == nil && flags.Changed("service-cidr") {
-		c.Cluster.ServiceCIDR = s
-	}
-	if s, err := flags.GetString("service-node-port-range"); err == nil && flags.Changed("service-node-port-range") {
-		c.Cluster.ServiceNodePortRange = s
-	}
-	if s, err := flags.GetString("base-domain"); err == nil && flags.Changed("base-domain") {
-		c.BaseDomain = s
-	}
-
-	return nil
-}
-
 // Note: add a configFile parameter here because of unit test requiring custom
 // local directory
-func (c *MicroshiftConfig) ReadAndValidate(configFile string, flags *pflag.FlagSet) error {
+func (c *MicroshiftConfig) ReadAndValidate(configFile string) error {
 	if configFile != "" {
 		if err := c.ReadFromConfigFile(configFile); err != nil {
 			return err
 		}
 	}
-	if err := c.ReadFromEnv(); err != nil {
-		return err
-	}
-	if err := c.ReadFromCmdLine(flags); err != nil {
-		return err
-	}
 
 	// validate serviceCIDR
 	clusterDNS, err := getClusterDNS(c.Cluster.ServiceCIDR)
@@ -537,22 +494,6 @@ func stringSliceContains(list []string, elements ...string) bool {
 	return false
 }
 
-func HideUnsupportedFlags(flags *pflag.FlagSet) {
-	// hide logging flags that we do not use/support
-	loggingFlags := pflag.NewFlagSet("logging-flags", pflag.ContinueOnError)
-	logs.AddFlags(loggingFlags)
-
-	supportedLoggingFlags := sets.NewString("v")
-
-	loggingFlags.VisitAll(func(pf *pflag.Flag) {
-		if !supportedLoggingFlags.Has(pf.Name) {
-			flags.MarkHidden(pf.Name)
-		}
-	})
-
-	flags.MarkHidden("version")
-}
-
 // GetVerbosity returns the numerical value for LogLevel which is an enum
 func (c *Config) GetVerbosity() int {
 	var verbosity int
@@ -570,3 +511,19 @@ func (c *Config) GetVerbosity() int {
 	}
 	return verbosity
 }
+
+func HideUnsupportedFlags(flags *pflag.FlagSet) {
+	// hide logging flags that we do not use/support
+	loggingFlags := pflag.NewFlagSet("logging-flags", pflag.ContinueOnError)
+	logs.AddFlags(loggingFlags)
+
+	supportedLoggingFlags := sets.NewString("v")
+
+	loggingFlags.VisitAll(func(pf *pflag.Flag) {
+		if !supportedLoggingFlags.Has(pf.Name) {
+			flags.MarkHidden(pf.Name)
+		}
+	})
+
+	flags.MarkHidden("version")
+}