Improve istioctl options command description (#38236)

* improve description

* revise and add releasenotes

* revise run to runE

* lint

istioctl/cmd/config_test.go

@@ -25,7 +25,7 @@ func TestConfigList(t *testing.T) {
 	cases := []testCase{
 		{ // case 0
 			args:           strings.Split("experimental config get istioNamespace", " "),
-			expectedRegexp: regexp.MustCompile("Usage:\n  istioctl experimental config"),
+			expectedRegexp: regexp.MustCompile("Examples:\n  # list configuration parameters\n  istioctl config list"),
 			wantException:  false,
 		},
 		{ // case 1

istioctl/cmd/options.go

@@ -20,26 +20,31 @@ import (
 )
 
 var helpFlags = map[string]bool{
+	"context":              true,
+	"istioNamespace":       true,
+	"kubeconfig":           true,
 	"log_as_json":          true,
 	"log_stacktrace_level": true,
 	"log_target":           true,
 	"log_caller":           true,
 	"log_output_level":     true,
+	"namespace":            true,
+	"vklog":                true,
 	// istioctl also inherits support for log_rotate, log_rotate_max_age, log_rotate_max_backups,
 	// log_rotate_max_size, but these are rarely appropriate for a user-facing CLI so we ignore them
 }
 
-func optionsCommand(rootCmd *cobra.Command) *cobra.Command {
+func optionsCommand() *cobra.Command {
 	retval := &cobra.Command{
 		Use:   "options",
 		Short: "Displays istioctl global options",
 		Args:  cobra.ExactArgs(0),
 	}
 
 	retval.SetHelpFunc(func(c *cobra.Command, args []string) {
-		c.Printf("The following options can be passed to any command:\n")
+		c.Printf("The following options can be passed to any command:\n\n")
 		// (Currently the only global options we show are help options)
-		rootCmd.PersistentFlags().VisitAll(func(flag *pflag.Flag) {
+		retval.Root().PersistentFlags().VisitAll(func(flag *pflag.Flag) {
 			if _, ok := helpFlags[flag.Name]; ok {
 				// Currently every flag.Shorthand is "", so there is no point in showing shorthands
 				shorthand := "   "

istioctl/cmd/options_test.go

@@ -22,11 +22,16 @@ import (
 
 // nolint: lll
 var expectedOutput = `The following options can be passed to any command:
+
+      --context: The name of the kubeconfig context to use
+  -i, --istioNamespace: Istio system namespace
+  -c, --kubeconfig: Kubernetes configuration file
       --log_as_json: Whether to format output as JSON or in plain console-friendly format
       --log_caller: Comma-separated list of scopes for which to include caller information, scopes can be any of \[.*\]
       --log_output_level: Comma-separated minimum per-scope logging level of messages to output, in the form of <scope>:<level>,<scope>:<level>,... where scope can be one of \[.*\] and level can be one of \[.*\]
       --log_stacktrace_level: Comma-separated minimum per-scope logging level at which stack traces are captured, in the form of <scope>:<level>,<scope:level>,... where scope can be one of \[.*\] and level can be one of \[.*\]
       --log_target: The set of paths where to output the log. This can be any path as well as the special values stdout and stderr
+  -n, --namespace: Config namespace
 `
 
 func TestLogHelp(t *testing.T) {

istioctl/cmd/root.go

@@ -18,13 +18,15 @@ import (
 	"errors"
 	"fmt"
 	"path/filepath"
+	"sort"
 	"strings"
 
 	"github.com/spf13/cobra"
 	"github.com/spf13/cobra/doc"
 	"github.com/spf13/viper"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/client-go/tools/clientcmd"
+	"k8s.io/kubectl/pkg/util/templates"
 
 	"istio.io/istio/istioctl/pkg/install"
 	"istio.io/istio/istioctl/pkg/multicluster"
@@ -144,6 +146,10 @@ func GetRootCmd(args []string) *cobra.Command {
 		Long: `Istio configuration command line utility for service operators to
 debug and diagnose their Istio mesh.
 `,
+		RunE: func(cmd *cobra.Command, args []string) error {
+			cmd.HelpFunc()(cmd, args)
+			return nil
+		},
 		PersistentPreRunE: configureLogging,
 	}
 
@@ -170,6 +176,11 @@ debug and diagnose their Istio mesh.
 		"log_as_json", "log_rotate", "log_rotate_max_age", "log_rotate_max_backups",
 		"log_rotate_max_size", "log_stacktrace_level", "log_target", "log_caller", "log_output_level",
 	}
+
+	group := templates.CommandGroup{
+		Message:  "Available Commands:",
+		Commands: []*cobra.Command{},
+	}
 	for _, opt := range hiddenFlags {
 		_ = rootCmd.PersistentFlags().MarkHidden(opt)
 	}
@@ -178,7 +189,7 @@ debug and diagnose their Istio mesh.
 
 	kubeInjectCmd := injectCommand()
 	hideInheritedFlags(kubeInjectCmd, FlagNamespace)
-	rootCmd.AddCommand(kubeInjectCmd)
+	group.Commands = append(group.Commands, kubeInjectCmd)
 
 	experimentalCmd := &cobra.Command{
 		Use:     "experimental",
@@ -194,37 +205,26 @@ debug and diagnose their Istio mesh.
 		newVersionCommand(),
 		statusCommand(),
 	}
-	var debugCmdAttachmentPoint *cobra.Command
 	if viper.GetBool("PREFER-EXPERIMENTAL") {
 		legacyCmd := &cobra.Command{
 			Use:   "legacy",
 			Short: "Legacy command variants",
 		}
-		rootCmd.AddCommand(legacyCmd)
-		for _, c := range xdsBasedTroubleshooting {
-			rootCmd.AddCommand(c)
-		}
-		debugCmdAttachmentPoint = legacyCmd
+		group.Commands = append(group.Commands, legacyCmd)
+		group.Commands = append(group.Commands, xdsBasedTroubleshooting...)
 	} else {
-		debugCmdAttachmentPoint = rootCmd
+		group.Commands = append(group.Commands, debugBasedTroubleshooting...)
 	}
 	for _, c := range xdsBasedTroubleshooting {
 		experimentalCmd.AddCommand(c)
 	}
-	for _, c := range debugBasedTroubleshooting {
-		debugCmdAttachmentPoint.AddCommand(c)
-	}
 
-	rootCmd.AddCommand(experimentalCmd)
-	rootCmd.AddCommand(proxyConfig())
-	rootCmd.AddCommand(adminCmd())
+	group.Commands = append(group.Commands, experimentalCmd, proxyConfig(), adminCmd())
 	experimentalCmd.AddCommand(injectorCommand())
 
-	rootCmd.AddCommand(install.NewVerifyCommand())
-	rootCmd.AddCommand(mesh.UninstallCmd(loggingOptions))
-
+	group.Commands = append(group.Commands, install.NewVerifyCommand(), mesh.UninstallCmd(loggingOptions))
 	experimentalCmd.AddCommand(AuthZ())
-	rootCmd.AddCommand(seeExperimentalCmd("authz"))
+	group.Commands = append(group.Commands, seeExperimentalCmd("authz"))
 	experimentalCmd.AddCommand(uninjectCommand())
 	experimentalCmd.AddCommand(metricsCmd())
 	experimentalCmd.AddCommand(describe())
@@ -241,59 +241,66 @@ debug and diagnose their Istio mesh.
 
 	analyzeCmd := Analyze()
 	hideInheritedFlags(analyzeCmd, FlagIstioNamespace)
-	rootCmd.AddCommand(analyzeCmd)
+	group.Commands = append(group.Commands, analyzeCmd)
 
 	dashboardCmd := dashboard()
 	hideInheritedFlags(dashboardCmd, FlagNamespace, FlagIstioNamespace)
-	rootCmd.AddCommand(dashboardCmd)
+	group.Commands = append(group.Commands, dashboardCmd)
 
 	manifestCmd := mesh.ManifestCmd(loggingOptions)
 	hideInheritedFlags(manifestCmd, FlagNamespace, FlagIstioNamespace, FlagCharts)
-	rootCmd.AddCommand(manifestCmd)
+	group.Commands = append(group.Commands, manifestCmd)
 
 	operatorCmd := mesh.OperatorCmd()
 	hideInheritedFlags(operatorCmd, FlagNamespace, FlagIstioNamespace, FlagCharts)
-	rootCmd.AddCommand(operatorCmd)
+	group.Commands = append(group.Commands, operatorCmd)
 
 	installCmd := mesh.InstallCmd(loggingOptions)
 	hideInheritedFlags(installCmd, FlagNamespace, FlagIstioNamespace, FlagCharts)
-	rootCmd.AddCommand(installCmd)
+	group.Commands = append(group.Commands, installCmd)
 
 	profileCmd := mesh.ProfileCmd(loggingOptions)
 	hideInheritedFlags(profileCmd, FlagNamespace, FlagIstioNamespace, FlagCharts)
-	rootCmd.AddCommand(profileCmd)
+	group.Commands = append(group.Commands, profileCmd)
 
 	upgradeCmd := mesh.UpgradeCmd(loggingOptions)
 	hideInheritedFlags(upgradeCmd, FlagNamespace, FlagIstioNamespace, FlagCharts)
-	rootCmd.AddCommand(upgradeCmd)
+	group.Commands = append(group.Commands, upgradeCmd)
 
 	bugReportCmd := bugreport.Cmd(loggingOptions)
 	hideInheritedFlags(bugReportCmd, FlagNamespace, FlagIstioNamespace)
-	rootCmd.AddCommand(bugReportCmd)
+	group.Commands = append(group.Commands, bugReportCmd)
 
 	tagCmd := tagCommand()
 	hideInheritedFlags(tagCommand(), FlagNamespace, FlagIstioNamespace, FlagCharts)
-	rootCmd.AddCommand(tagCmd)
+	group.Commands = append(group.Commands, tagCmd)
 
 	remoteSecretCmd := multicluster.NewCreateRemoteSecretCommand()
 	remoteClustersCmd := clustersCommand()
 	// leave the multicluster commands in x for backwards compat
-	rootCmd.AddCommand(remoteSecretCmd)
-	rootCmd.AddCommand(remoteClustersCmd)
+	group.Commands = append(group.Commands, remoteSecretCmd, remoteClustersCmd)
 	experimentalCmd.AddCommand(remoteSecretCmd)
 	experimentalCmd.AddCommand(remoteClustersCmd)
 
-	rootCmd.AddCommand(collateral.CobraCommand(rootCmd, &doc.GenManHeader{
+	group.Commands = append(group.Commands, collateral.CobraCommand(rootCmd, &doc.GenManHeader{
 		Title:   "Istio Control",
 		Section: "istioctl CLI",
 		Manual:  "Istio Control",
 	}))
 
 	validateCmd := validate.NewValidateCommand(&istioNamespace, &namespace)
 	hideInheritedFlags(validateCmd, "kubeconfig")
-	rootCmd.AddCommand(validateCmd)
+	group.Commands = append(group.Commands, validateCmd, optionsCommand())
+
+	sort.SliceStable(group.Commands, func(i, j int) bool {
+		return group.Commands[i].Use < group.Commands[j].Use
+	})
+
+	groups := templates.CommandGroups{group}
+	groups.Add(rootCmd)
 
-	rootCmd.AddCommand(optionsCommand(rootCmd))
+	filters := []string{"options"}
+	templates.ActsAsRootCommand(rootCmd, filters, groups...)
 
 	// BFS applies the flag error function to all subcommands
 	seenCommands := make(map[*cobra.Command]bool)

releasenotes/notes/38236.yaml

@@ -0,0 +1,6 @@
+apiVersion: release-notes/v2
+kind: feature
+area: istioctl
+releaseNotes:
+  - |
+    **Improved** `options` command to be similar like `kubectl` , and put all global flags into the `options` command.