Encapsulate debug flag handling

Introduce internal.DebugFlags to replace debug and verbose logging flag handling. Move the flag definitions from the "PersistentFlagSet" to a struct method. Remove the debug flag handling from the root command and move it to the sub-commands. This allows for the removal of CallPersistentPreRun, since the commands no longer rely on proper chaining of the PersistentPreRun hooks. Remove some redundant calls to the "PersistentFlagSet" in some sub-commands. Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>
k0sproject · Dec 24, 2024 · f7d165f · f7d165f
1 parent 0e0d033
commit f7d165f
Show file tree

Hide file tree

Showing 38 changed files with 410 additions and 226 deletions.
diff --git a/cmd/airgap/airgap.go b/cmd/airgap/airgap.go
@@ -19,19 +19,23 @@ package airgap
 import (
 	"github.com/spf13/cobra"
 
-	"github.com/k0sproject/k0s/pkg/config"
+	"github.com/k0sproject/k0s/cmd/internal"
 )
 
 func NewAirgapCmd() *cobra.Command {
+	var debugFlags internal.DebugFlags
+
 	cmd := &cobra.Command{
-		Use:   "airgap",
-		Short: "Manage airgap setup",
-		Args:  cobra.NoArgs,
-		Run:   func(*cobra.Command, []string) { /* Enforce arg validation. */ },
+		Use:              "airgap",
+		Short:            "Manage airgap setup",
+		Args:             cobra.NoArgs,
+		PersistentPreRun: debugFlags.Run,
+		Run:              func(*cobra.Command, []string) { /* Enforce arg validation. */ },
 	}
 
+	debugFlags.AddToFlagSet(cmd.PersistentFlags())
+
 	cmd.AddCommand(NewAirgapListImagesCmd())
-	cmd.PersistentFlags().AddFlagSet(config.FileInputFlag())
-	cmd.PersistentFlags().AddFlagSet(config.GetPersistentFlagSet())
+
 	return cmd
 }
diff --git a/cmd/airgap/listimages.go b/cmd/airgap/listimages.go
@@ -53,8 +53,11 @@ func NewAirgapListImagesCmd() *cobra.Command {
 			return nil
 		},
 	}
+
+	cmd.PersistentFlags().AddFlagSet(config.GetPersistentFlagSet())
+
 	cmd.Flags().AddFlagSet(config.FileInputFlag())
 	cmd.Flags().BoolVar(&all, "all", false, "include all images, even if they are not used in the current configuration")
-	cmd.PersistentFlags().AddFlagSet(config.GetPersistentFlagSet())
+
 	return cmd
 }
diff --git a/cmd/api/api.go b/cmd/api/api.go
@@ -29,7 +29,7 @@ import (
 	"strings"
 	"time"
 
-	k0slog "github.com/k0sproject/k0s/internal/pkg/log"
+	"github.com/k0sproject/k0s/cmd/internal"
 	mw "github.com/k0sproject/k0s/internal/pkg/middleware"
 	"github.com/k0sproject/k0s/pkg/apis/k0s/v1beta1"
 	"github.com/k0sproject/k0s/pkg/config"
@@ -50,15 +50,13 @@ type command struct {
 }
 
 func NewAPICmd() *cobra.Command {
+	var debugFlags internal.DebugFlags
+
 	cmd := &cobra.Command{
-		Use:   "api",
-		Short: "Run the controller API",
-		Args:  cobra.NoArgs,
-		PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
-			logrus.SetOutput(cmd.OutOrStdout())
-			k0slog.SetInfoLevel()
-			return config.CallParentPersistentPreRun(cmd, args)
-		},
+		Use:              "api",
+		Short:            "Run the controller API",
+		Args:             cobra.NoArgs,
+		PersistentPreRun: debugFlags.Run,
 		RunE: func(cmd *cobra.Command, _ []string) error {
 			opts, err := config.GetCmdOpts(cmd)
 			if err != nil {
@@ -67,7 +65,11 @@ func NewAPICmd() *cobra.Command {
 			return (&command{CLIOptions: opts}).start()
 		},
 	}
-	cmd.PersistentFlags().AddFlagSet(config.GetPersistentFlagSet())
+
+	pflags := cmd.PersistentFlags()
+	debugFlags.LongRunning().AddToFlagSet(pflags)
+	pflags.AddFlagSet(config.GetPersistentFlagSet())
+
 	return cmd
 }
 

diff --git a/cmd/backup/backup_unix.go b/cmd/backup/backup_unix.go
@@ -25,6 +25,7 @@ import (
 	"os"
 	"strings"
 
+	"github.com/k0sproject/k0s/cmd/internal"
 	"github.com/k0sproject/k0s/internal/pkg/dir"
 	"github.com/k0sproject/k0s/pkg/backup"
 	"github.com/k0sproject/k0s/pkg/component/status"
@@ -37,12 +38,16 @@ import (
 type command config.CLIOptions
 
 func NewBackupCmd() *cobra.Command {
-	var savePath string
+	var (
+		debugFlags internal.DebugFlags
+		savePath   string
+	)
 
 	cmd := &cobra.Command{
-		Use:   "backup",
-		Short: "Back-Up k0s configuration. Must be run as root (or with sudo)",
-		Args:  cobra.NoArgs,
+		Use:              "backup",
+		Short:            "Back-Up k0s configuration. Must be run as root (or with sudo)",
+		Args:             cobra.NoArgs,
+		PersistentPreRun: debugFlags.Run,
 		PreRun: func(cmd *cobra.Command, args []string) {
 			// ensure logs don't mess up output
 			logrus.SetOutput(cmd.ErrOrStderr())
@@ -63,8 +68,13 @@ func NewBackupCmd() *cobra.Command {
 			return c.backup(savePath, cmd.OutOrStdout())
 		},
 	}
+
+	pflags := cmd.PersistentFlags()
+	debugFlags.AddToFlagSet(pflags)
+	pflags.AddFlagSet(config.GetPersistentFlagSet())
+
 	cmd.Flags().StringVar(&savePath, "save-path", "", "destination directory path for backup assets, use '-' for stdout")
-	cmd.PersistentFlags().AddFlagSet(config.GetPersistentFlagSet())
+
 	return cmd
 }
 

diff --git a/cmd/config/config.go b/cmd/config/config.go
@@ -17,20 +17,27 @@ limitations under the License.
 package config
 
 import (
+	"github.com/k0sproject/k0s/cmd/internal"
 	"github.com/spf13/cobra"
 )
 
 func NewConfigCmd() *cobra.Command {
+	var debugFlags internal.DebugFlags
+
 	cmd := &cobra.Command{
-		Use:   "config",
-		Short: "Configuration related sub-commands",
-		Args:  cobra.NoArgs,
-		Run:   func(*cobra.Command, []string) { /* Enforce arg validation. */ },
+		Use:              "config",
+		Short:            "Configuration related sub-commands",
+		Args:             cobra.NoArgs,
+		PersistentPreRun: debugFlags.Run,
+		Run:              func(*cobra.Command, []string) { /* Enforce arg validation. */ },
 	}
 	cmd.AddCommand(NewCreateCmd())
 	cmd.AddCommand(NewEditCmd())
 	cmd.AddCommand(NewStatusCmd())
 	cmd.AddCommand(NewValidateCmd())
+
+	debugFlags.AddToFlagSet(cmd.PersistentFlags())
+
 	return cmd
 }
 

diff --git a/cmd/config/create.go b/cmd/config/create.go
@@ -17,13 +17,15 @@ limitations under the License.
 package config
 
 import (
-	"github.com/spf13/cobra"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"sigs.k8s.io/yaml"
-
 	"github.com/k0sproject/k0s/pkg/apis/k0s/v1beta1"
 	k0sscheme "github.com/k0sproject/k0s/pkg/client/clientset/scheme"
 	"github.com/k0sproject/k0s/pkg/config"
+
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+
+	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
+	"sigs.k8s.io/yaml"
 )
 
 func NewCreateCmd() *cobra.Command {
@@ -55,7 +57,15 @@ func NewCreateCmd() *cobra.Command {
 			return err
 		},
 	}
+
+	pflags := cmd.PersistentFlags()
+	// Add unused persistent flags for backwards compatibility.
+	config.GetPersistentFlagSet().VisitAll(func(f *pflag.Flag) {
+		f.Hidden = true
+		pflags.AddFlag(f)
+	})
+
 	cmd.Flags().BoolVar(&includeImages, "include-images", false, "include the default images in the output")
-	cmd.PersistentFlags().AddFlagSet(config.GetPersistentFlagSet())
+
 	return cmd
 }
diff --git a/cmd/config/edit.go b/cmd/config/edit.go
@@ -31,6 +31,8 @@ func NewEditCmd() *cobra.Command {
 			return reExecKubectl(cmd, "-n", "kube-system", "edit", "clusterconfig", "k0s")
 		},
 	}
+
 	cmd.PersistentFlags().AddFlagSet(config.GetKubeCtlFlagSet())
+
 	return cmd
 }
diff --git a/cmd/config/status.go b/cmd/config/status.go
@@ -38,7 +38,10 @@ func NewStatusCmd() *cobra.Command {
 			return reExecKubectl(cmd, args...)
 		},
 	}
+
 	cmd.PersistentFlags().AddFlagSet(config.GetKubeCtlFlagSet())
+
 	cmd.Flags().StringVarP(&outputFormat, "output", "o", "", "Output format. Must be one of yaml|json")
+
 	return cmd
 }
diff --git a/cmd/config/validate.go b/cmd/config/validate.go
@@ -25,7 +25,10 @@ import (
 	"github.com/k0sproject/k0s/pkg/apis/k0s/v1beta1"
 	"github.com/k0sproject/k0s/pkg/config"
 
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+
 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
 )
 
 func NewValidateCmd() *cobra.Command {
@@ -62,8 +65,15 @@ func NewValidateCmd() *cobra.Command {
 		},
 	}
 
-	cmd.PersistentFlags().AddFlagSet(config.GetPersistentFlagSet())
+	pflags := cmd.PersistentFlags()
+	// Add unused persistent flags for backwards compatibility.
+	config.GetPersistentFlagSet().VisitAll(func(f *pflag.Flag) {
+		f.Hidden = true
+		pflags.AddFlag(f)
+	})
+
 	cmd.Flags().AddFlagSet(config.FileInputFlag())
-	_ = cmd.MarkFlagRequired("config")
+	utilruntime.Must(cmd.MarkFlagRequired("config"))
+
 	return cmd
 }
diff --git a/cmd/controller/controller.go b/cmd/controller/controller.go
@@ -31,10 +31,10 @@ import (
 	"time"
 
 	"github.com/avast/retry-go"
+	"github.com/k0sproject/k0s/cmd/internal"
 	workercmd "github.com/k0sproject/k0s/cmd/worker"
 	"github.com/k0sproject/k0s/internal/pkg/dir"
 	"github.com/k0sproject/k0s/internal/pkg/file"
-	k0slog "github.com/k0sproject/k0s/internal/pkg/log"
 	"github.com/k0sproject/k0s/internal/pkg/sysinfo"
 	"github.com/k0sproject/k0s/internal/sync/value"
 	"github.com/k0sproject/k0s/pkg/apis/k0s/v1beta1"
@@ -67,7 +67,10 @@ import (
 type command config.CLIOptions
 
 func NewControllerCmd() *cobra.Command {
-	var ignorePreFlightChecks bool
+	var (
+		debugFlags            internal.DebugFlags
+		ignorePreFlightChecks bool
+	)
 
 	cmd := &cobra.Command{
 		Use:     "controller [join-token]",
@@ -80,12 +83,8 @@ func NewControllerCmd() *cobra.Command {
 	or CLI flag:
 	$ k0s controller --token-file [path_to_file]
 	Note: Token can be passed either as a CLI argument or as a flag`,
-		Args: cobra.MaximumNArgs(1),
-		PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
-			logrus.SetOutput(cmd.OutOrStdout())
-			k0slog.SetInfoLevel()
-			return config.CallParentPersistentPreRun(cmd, args)
-		},
+		Args:             cobra.MaximumNArgs(1),
+		PersistentPreRun: debugFlags.Run,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			opts, err := config.GetCmdOpts(cmd)
 			if err != nil {
@@ -114,19 +113,23 @@ func NewControllerCmd() *cobra.Command {
 
 			ctx, cancel := signal.NotifyContext(cmd.Context(), os.Interrupt, syscall.SIGINT, syscall.SIGTERM)
 			defer cancel()
-			return c.start(ctx)
+			return c.start(ctx, debugFlags.IsDebug())
 		},
 	}
 
 	// append flags
+	pflags := cmd.PersistentFlags()
+	debugFlags.LongRunning().AddToFlagSet(pflags)
+	pflags.AddFlagSet(config.GetPersistentFlagSet())
+	pflags.AddFlagSet(config.GetControllerFlags())
+	pflags.AddFlagSet(config.GetWorkerFlags())
+
 	cmd.Flags().BoolVar(&ignorePreFlightChecks, "ignore-pre-flight-checks", false, "continue even if pre-flight checks fail")
-	cmd.Flags().AddFlagSet(config.GetPersistentFlagSet())
-	cmd.PersistentFlags().AddFlagSet(config.GetControllerFlags())
-	cmd.PersistentFlags().AddFlagSet(config.GetWorkerFlags())
+
 	return cmd
 }
 
-func (c *command) start(ctx context.Context) error {
+func (c *command) start(ctx context.Context, debug bool) error {
 	perfTimer := performance.NewTimer("controller-start").Buffer().Start()
 
 	nodeConfig, err := c.K0sVars.NodeConfig()
@@ -247,8 +250,8 @@ func (c *command) start(ctx context.Context) error {
 		nodeComponents.Add(ctx, &cplb.Keepalived{
 			K0sVars:         c.K0sVars,
 			Config:          cplbCfg.Keepalived,
-			DetailedLogging: c.Debug,
-			LogConfig:       c.Debug,
+			DetailedLogging: debug,
+			LogConfig:       debug,
 			KubeConfigPath:  c.K0sVars.AdminKubeConfigPath,
 			APIPort:         nodeConfig.Spec.API.Port,
 		})

diff --git a/cmd/controller/controller_test.go b/cmd/controller/controller_test.go
@@ -59,7 +59,7 @@ Flags:
   -c, --config string                                  config file, use '-' to read the config from stdin (default `+defaultConfigPath+`)
       --cri-socket string                              container runtime socket to use, default to internal containerd. Format: [remote|docker]:[path-to-socket]
       --data-dir string                                Data Directory for k0s. DO NOT CHANGE for an existing setup, things will break! (default `+defaultDataDir+`)
-  -d, --debug                                          Debug logging (default: false)
+  -d, --debug                                          Debug logging (implies verbose logging)
       --debugListenOn string                           Http listenOn for Debug pprof handler (default ":6060")
       --disable-components strings                     disable components (valid items: applier-manager,autopilot,control-api,coredns,csr-approver,endpoint-reconciler,helm,konnectivity-server,kube-controller-manager,kube-proxy,kube-scheduler,metrics-server,network-provider,node-role,system-rbac,windows-node,worker-config)
       --enable-cloud-provider                          Whether or not to enable cloud provider support in kubelet
@@ -82,6 +82,6 @@ Flags:
       --status-socket string                           Full file path to the socket file. (default: <rundir>/status.sock)
       --taints strings                                 Node taints, list of key=value:effect strings
       --token-file string                              Path to the file containing join-token.
-  -v, --verbose                                        Verbose logging (default: false)
+  -v, --verbose                                        Verbose logging (default true)
 `, out.String())
 }
diff --git a/cmd/etcd/etcd.go b/cmd/etcd/etcd.go
@@ -20,21 +20,22 @@ import (
 	"errors"
 	"fmt"
 
+	"github.com/k0sproject/k0s/cmd/internal"
 	"github.com/k0sproject/k0s/pkg/apis/k0s/v1beta1"
 	"github.com/k0sproject/k0s/pkg/config"
 
 	"github.com/spf13/cobra"
 )
 
 func NewEtcdCmd() *cobra.Command {
+	var debugFlags internal.DebugFlags
+
 	cmd := &cobra.Command{
 		Use:   "etcd",
 		Short: "Manage etcd cluster",
 		Args:  cobra.NoArgs,
 		PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
-			if err := config.CallParentPersistentPreRun(cmd, args); err != nil {
-				return err
-			}
+			debugFlags.Run(cmd, args)
 
 			opts, err := config.GetCmdOpts(cmd)
 			if err != nil {
@@ -54,8 +55,11 @@ func NewEtcdCmd() *cobra.Command {
 		},
 		Run: func(*cobra.Command, []string) { /* Enforce arg validation. */ },
 	}
+
+	debugFlags.AddToFlagSet(cmd.PersistentFlags())
+
 	cmd.AddCommand(etcdLeaveCmd())
 	cmd.AddCommand(etcdListCmd())
-	cmd.PersistentFlags().AddFlagSet(config.GetPersistentFlagSet())
+
 	return cmd
 }