Merge pull request #67362 from stewart-yu/stewart-controller-manager-codeclean#02

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. *-controller-manager: output flags in logical sections ![image](https://user-images.githubusercontent.com/30410021/44071436-8e619e72-9fbb-11e8-84c5-72c1ff9767b6.png) ``` Print *-controller-manager --help flag help in sections. ```
2018-08-27 07:31:08 -07:00 · 2018-08-27 07:31:08 -07:00 · b315ecfe67
parent 91e37eeb92 b0eb92cc52
commit b315ecfe67
11 changed files with 103 additions and 53 deletions
--- a/cmd/cloud-controller-manager/app/BUILD
+++ b/cmd/cloud-controller-manager/app/BUILD
@ -18,6 +18,7 @@ go_library(
        "//pkg/version/verflag:go_default_library",
        "//staging/src/k8s.io/apimachinery/pkg/util/uuid:go_default_library",
        "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/util/flag:go_default_library",
        "//staging/src/k8s.io/client-go/kubernetes:go_default_library",
        "//staging/src/k8s.io/client-go/tools/leaderelection:go_default_library",
        "//staging/src/k8s.io/client-go/tools/leaderelection/resourcelock:go_default_library",
--- a/cmd/cloud-controller-manager/app/controllermanager.go
+++ b/cmd/cloud-controller-manager/app/controllermanager.go
@ -29,6 +29,7 @@ import (

 	"k8s.io/apimachinery/pkg/util/uuid"
 	"k8s.io/apimachinery/pkg/util/wait"
+	apiserverflag "k8s.io/apiserver/pkg/util/flag"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/leaderelection"
 	"k8s.io/client-go/tools/leaderelection/resourcelock"
@ -77,7 +78,23 @@ the cloud specific control loops shipped with Kubernetes.`,

 		},
 	}
-	s.AddFlags(cmd.Flags())
+
+	fs := cmd.Flags()
+	namedFlagSets := s.Flags()
+	for _, f := range namedFlagSets.FlagSets {
+		fs.AddFlagSet(f)
+	}
+	usageFmt := "Usage:\n  %s\n"
+	cols, _, _ := apiserverflag.TerminalSize(cmd.OutOrStdout())
+	cmd.SetUsageFunc(func(cmd *cobra.Command) error {
+		fmt.Fprintf(cmd.OutOrStderr(), usageFmt, cmd.UseLine())
+		apiserverflag.PrintSections(cmd.OutOrStderr(), namedFlagSets, cols)
+		return nil
+	})
+	cmd.SetHelpFunc(func(cmd *cobra.Command, args []string) {
+		fmt.Fprintf(cmd.OutOrStdout(), "%s\n\n"+usageFmt, cmd.Long, cmd.UseLine())
+		apiserverflag.PrintSections(cmd.OutOrStdout(), namedFlagSets, cols)
+	})

 	return cmd
 }
--- a/cmd/cloud-controller-manager/app/options/BUILD
+++ b/cmd/cloud-controller-manager/app/options/BUILD
@ -25,6 +25,7 @@ go_library(
        "//staging/src/k8s.io/apimachinery/pkg/util/errors:go_default_library",
        "//staging/src/k8s.io/apiserver/pkg/server/options:go_default_library",
        "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/util/flag:go_default_library",
        "//staging/src/k8s.io/client-go/informers:go_default_library",
        "//staging/src/k8s.io/client-go/kubernetes:go_default_library",
        "//staging/src/k8s.io/client-go/kubernetes/typed/core/v1:go_default_library",
@ -32,7 +33,6 @@ go_library(
        "//staging/src/k8s.io/client-go/tools/clientcmd:go_default_library",
        "//staging/src/k8s.io/client-go/tools/record:go_default_library",
        "//vendor/github.com/golang/glog:go_default_library",
-        "//vendor/github.com/spf13/pflag:go_default_library",
    ],
 )

--- a/cmd/cloud-controller-manager/app/options/options.go
+++ b/cmd/cloud-controller-manager/app/options/options.go
@ -28,6 +28,7 @@ import (
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	apiserveroptions "k8s.io/apiserver/pkg/server/options"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	apiserverflag "k8s.io/apiserver/pkg/util/flag"
 	"k8s.io/client-go/informers"
 	clientset "k8s.io/client-go/kubernetes"
 	v1core "k8s.io/client-go/kubernetes/typed/core/v1"
@ -45,7 +46,6 @@ import (
 	_ "k8s.io/kubernetes/pkg/features"

 	"github.com/golang/glog"
-	"github.com/spf13/pflag"
 )

 const (
@ -131,24 +131,27 @@ func NewDefaultComponentConfig(insecurePort int32) (componentconfig.CloudControl
 	return internal, nil
 }

-// AddFlags adds flags for a specific ExternalCMServer to the specified FlagSet
-func (o *CloudControllerManagerOptions) AddFlags(fs *pflag.FlagSet) {
-	o.CloudProvider.AddFlags(fs)
-	o.Debugging.AddFlags(fs)
-	o.GenericComponent.AddFlags(fs)
-	o.KubeCloudShared.AddFlags(fs)
-	o.ServiceController.AddFlags(fs)
+// Flags returns flags for a specific APIServer by section name
+func (o *CloudControllerManagerOptions) Flags() (fss apiserverflag.NamedFlagSets) {
+	o.CloudProvider.AddFlags(fss.FlagSet("cloud provider"))
+	o.Debugging.AddFlags(fss.FlagSet("debugging"))
+	o.GenericComponent.AddFlags(fss.FlagSet("generic"))
+	o.KubeCloudShared.AddFlags(fss.FlagSet("generic"))
+	o.ServiceController.AddFlags(fss.FlagSet("service controller"))

-	o.SecureServing.AddFlags(fs)
-	o.InsecureServing.AddUnqualifiedFlags(fs)
-	o.Authentication.AddFlags(fs)
-	o.Authorization.AddFlags(fs)
+	o.SecureServing.AddFlags(fss.FlagSet("secure serving"))
+	o.InsecureServing.AddUnqualifiedFlags(fss.FlagSet("insecure serving"))
+	o.Authentication.AddFlags(fss.FlagSet("authentication"))
+	o.Authorization.AddFlags(fss.FlagSet("authorization"))

+	fs := fss.FlagSet("misc")
 	fs.StringVar(&o.Master, "master", o.Master, "The address of the Kubernetes API server (overrides any value in kubeconfig).")
 	fs.StringVar(&o.Kubeconfig, "kubeconfig", o.Kubeconfig, "Path to kubeconfig file with authorization and master location information.")
 	fs.DurationVar(&o.NodeStatusUpdateFrequency.Duration, "node-status-update-frequency", o.NodeStatusUpdateFrequency.Duration, "Specifies how often the controller updates nodes' status.")

-	utilfeature.DefaultFeatureGate.AddFlag(fs)
+	utilfeature.DefaultFeatureGate.AddFlag(fss.FlagSet("generic"))
+
+	return fss
 }

 // ApplyTo fills up cloud controller manager config with options.
--- a/cmd/cloud-controller-manager/app/options/options_test.go
+++ b/cmd/cloud-controller-manager/app/options/options_test.go
@ -94,9 +94,11 @@ func TestDefaultFlags(t *testing.T) {
 }

 func TestAddFlags(t *testing.T) {
-	f := pflag.NewFlagSet("addflagstest", pflag.ContinueOnError)
+	fs := pflag.NewFlagSet("addflagstest", pflag.ContinueOnError)
 	s, _ := NewCloudControllerManagerOptions()
-	s.AddFlags(f)
+	for _, f := range s.Flags().FlagSets {
+		fs.AddFlagSet(f)
+	}

 	args := []string{
 		"--address=192.168.4.10",
@ -129,7 +131,7 @@ func TestAddFlags(t *testing.T) {
 		"--secure-port=10001",
 		"--use-service-account-credentials=false",
 	}
-	f.Parse(args)
+	fs.Parse(args)

 	expected := &CloudControllerManagerOptions{
 		CloudProvider: &cmoptions.CloudProviderOptions{
--- a/cmd/kube-controller-manager/app/BUILD
+++ b/cmd/kube-controller-manager/app/BUILD
@ -113,6 +113,7 @@ go_library(
        "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
        "//staging/src/k8s.io/apiserver/pkg/server/mux:go_default_library",
        "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/util/flag:go_default_library",
        "//staging/src/k8s.io/client-go/discovery/cached:go_default_library",
        "//staging/src/k8s.io/client-go/dynamic:go_default_library",
        "//staging/src/k8s.io/client-go/informers:go_default_library",
--- a/cmd/kube-controller-manager/app/controllermanager.go
+++ b/cmd/kube-controller-manager/app/controllermanager.go
@ -39,6 +39,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/uuid"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/apiserver/pkg/server/mux"
+	apiserverflag "k8s.io/apiserver/pkg/util/flag"
 	cacheddiscovery "k8s.io/client-go/discovery/cached"
 	"k8s.io/client-go/informers"
 	restclient "k8s.io/client-go/rest"
@ -105,7 +106,23 @@ controller, and serviceaccounts controller.`,
 			}
 		},
 	}
-	s.AddFlags(cmd.Flags(), KnownControllers(), ControllersDisabledByDefault.List())
+
+	fs := cmd.Flags()
+	namedFlagSets := s.Flags(KnownControllers(), ControllersDisabledByDefault.List())
+	for _, f := range namedFlagSets.FlagSets {
+		fs.AddFlagSet(f)
+	}
+	usageFmt := "Usage:\n  %s\n"
+	cols, _, _ := apiserverflag.TerminalSize(cmd.OutOrStdout())
+	cmd.SetUsageFunc(func(cmd *cobra.Command) error {
+		fmt.Fprintf(cmd.OutOrStderr(), usageFmt, cmd.UseLine())
+		apiserverflag.PrintSections(cmd.OutOrStderr(), namedFlagSets, cols)
+		return nil
+	})
+	cmd.SetHelpFunc(func(cmd *cobra.Command, args []string) {
+		fmt.Fprintf(cmd.OutOrStdout(), "%s\n\n"+usageFmt, cmd.Long, cmd.UseLine())
+		apiserverflag.PrintSections(cmd.OutOrStdout(), namedFlagSets, cols)
+	})

 	return cmd
 }
--- a/cmd/kube-controller-manager/app/options/BUILD
+++ b/cmd/kube-controller-manager/app/options/BUILD
@ -46,6 +46,7 @@ go_library(
        "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
        "//staging/src/k8s.io/apiserver/pkg/server/options:go_default_library",
        "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/util/flag:go_default_library",
        "//staging/src/k8s.io/client-go/kubernetes:go_default_library",
        "//staging/src/k8s.io/client-go/kubernetes/typed/core/v1:go_default_library",
        "//staging/src/k8s.io/client-go/rest:go_default_library",
--- a/cmd/kube-controller-manager/app/options/options.go
+++ b/cmd/kube-controller-manager/app/options/options.go
@ -29,6 +29,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/sets"
 	apiserveroptions "k8s.io/apiserver/pkg/server/options"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	apiserverflag "k8s.io/apiserver/pkg/util/flag"
 	clientset "k8s.io/client-go/kubernetes"
 	v1core "k8s.io/client-go/kubernetes/typed/core/v1"
 	restclient "k8s.io/client-go/rest"
@ -45,7 +46,6 @@ import (
 	_ "k8s.io/kubernetes/pkg/features"

 	"github.com/golang/glog"
-	"github.com/spf13/pflag"
 )

 const (
@ -227,38 +227,39 @@ func NewDefaultComponentConfig(insecurePort int32) (componentconfig.KubeControll
 	return internal, nil
 }

-// AddFlags adds flags for a specific KubeControllerManagerOptions to the specified FlagSet
-func (s *KubeControllerManagerOptions) AddFlags(fs *pflag.FlagSet, allControllers []string, disabledByDefaultControllers []string) {
-	s.CloudProvider.AddFlags(fs)
-	s.Debugging.AddFlags(fs)
-	s.GenericComponent.AddFlags(fs)
-	s.KubeCloudShared.AddFlags(fs)
-	s.ServiceController.AddFlags(fs)
+// Flags returns flags for a specific APIServer by section name
+func (s *KubeControllerManagerOptions) Flags(allControllers []string, disabledByDefaultControllers []string) (fss apiserverflag.NamedFlagSets) {
+	s.CloudProvider.AddFlags(fss.FlagSet("cloud provider"))
+	s.Debugging.AddFlags(fss.FlagSet("debugging"))
+	s.GenericComponent.AddFlags(fss.FlagSet("generic"))
+	s.KubeCloudShared.AddFlags(fss.FlagSet("generic"))
+	s.ServiceController.AddFlags(fss.FlagSet("service controller"))

-	s.SecureServing.AddFlags(fs)
-	s.InsecureServing.AddUnqualifiedFlags(fs)
-	s.Authentication.AddFlags(fs)
-	s.Authorization.AddFlags(fs)
+	s.SecureServing.AddFlags(fss.FlagSet("secure serving"))
+	s.InsecureServing.AddUnqualifiedFlags(fss.FlagSet("insecure serving"))
+	s.Authentication.AddFlags(fss.FlagSet("authentication"))
+	s.Authorization.AddFlags(fss.FlagSet("authorization"))

-	s.AttachDetachController.AddFlags(fs)
-	s.CSRSigningController.AddFlags(fs)
-	s.DeploymentController.AddFlags(fs)
-	s.DaemonSetController.AddFlags(fs)
-	s.DeprecatedFlags.AddFlags(fs)
-	s.EndPointController.AddFlags(fs)
-	s.GarbageCollectorController.AddFlags(fs)
-	s.HPAController.AddFlags(fs)
-	s.JobController.AddFlags(fs)
-	s.NamespaceController.AddFlags(fs)
-	s.NodeIpamController.AddFlags(fs)
-	s.NodeLifecycleController.AddFlags(fs)
-	s.PersistentVolumeBinderController.AddFlags(fs)
-	s.PodGCController.AddFlags(fs)
-	s.ReplicaSetController.AddFlags(fs)
-	s.ReplicationController.AddFlags(fs)
-	s.ResourceQuotaController.AddFlags(fs)
-	s.SAController.AddFlags(fs)
+	s.AttachDetachController.AddFlags(fss.FlagSet("attachdetach controller"))
+	s.CSRSigningController.AddFlags(fss.FlagSet("csrsigning controller"))
+	s.DeploymentController.AddFlags(fss.FlagSet("deployment controller"))
+	s.DaemonSetController.AddFlags(fss.FlagSet("daemonset controller"))
+	s.DeprecatedFlags.AddFlags(fss.FlagSet("deprecated"))
+	s.EndPointController.AddFlags(fss.FlagSet("endpoint controller"))
+	s.GarbageCollectorController.AddFlags(fss.FlagSet("garbagecollector controller"))
+	s.HPAController.AddFlags(fss.FlagSet("horizontalpodautoscaling controller"))
+	s.JobController.AddFlags(fss.FlagSet("job controller"))
+	s.NamespaceController.AddFlags(fss.FlagSet("namespace controller"))
+	s.NodeIpamController.AddFlags(fss.FlagSet("nodeipam controller"))
+	s.NodeLifecycleController.AddFlags(fss.FlagSet("nodelifecycle controller"))
+	s.PersistentVolumeBinderController.AddFlags(fss.FlagSet("persistentvolume-binder controller"))
+	s.PodGCController.AddFlags(fss.FlagSet("podgc controller"))
+	s.ReplicaSetController.AddFlags(fss.FlagSet("replicaset controller"))
+	s.ReplicationController.AddFlags(fss.FlagSet("replicationcontroller"))
+	s.ResourceQuotaController.AddFlags(fss.FlagSet("resourcequota controller"))
+	s.SAController.AddFlags(fss.FlagSet("serviceaccount controller"))

+	fs := fss.FlagSet("misc")
 	fs.StringVar(&s.Master, "master", s.Master, "The address of the Kubernetes API server (overrides any value in kubeconfig).")
 	fs.StringVar(&s.Kubeconfig, "kubeconfig", s.Kubeconfig, "Path to kubeconfig file with authorization and master location information.")
 	fs.StringSliceVar(&s.Controllers, "controllers", s.Controllers, fmt.Sprintf(""+
@ -269,7 +270,9 @@ func (s *KubeControllerManagerOptions) AddFlags(fs *pflag.FlagSet, allController
 	var dummy string
 	fs.MarkDeprecated("insecure-experimental-approve-all-kubelet-csrs-for-group", "This flag does nothing.")
 	fs.StringVar(&dummy, "insecure-experimental-approve-all-kubelet-csrs-for-group", "", "This flag does nothing.")
-	utilfeature.DefaultFeatureGate.AddFlag(fs)
+	utilfeature.DefaultFeatureGate.AddFlag(fss.FlagSet("generic"))
+
+	return fss
 }

 // ApplyTo fills up controller manager config with options.
--- a/cmd/kube-controller-manager/app/options/options_test.go
+++ b/cmd/kube-controller-manager/app/options/options_test.go
@ -34,9 +34,11 @@ import (
 )

 func TestAddFlags(t *testing.T) {
-	f := pflag.NewFlagSet("addflagstest", pflag.ContinueOnError)
+	fs := pflag.NewFlagSet("addflagstest", pflag.ContinueOnError)
 	s, _ := NewKubeControllerManagerOptions()
-	s.AddFlags(f, []string{""}, []string{""})
+	for _, f := range s.Flags([]string{""}, []string{""}).FlagSets {
+		fs.AddFlagSet(f)
+	}

 	args := []string{
 		"--address=192.168.4.10",
@ -113,7 +115,7 @@ func TestAddFlags(t *testing.T) {
 		"--bind-address=192.168.4.21",
 		"--secure-port=10001",
 	}
-	f.Parse(args)
+	fs.Parse(args)
 	// Sort GCIgnoredResources because it's built from a map, which means the
 	// insertion order is random.
 	sort.Sort(sortedGCIgnoredResources(s.GarbageCollectorController.GCIgnoredResources))
--- a/staging/src/k8s.io/apiserver/pkg/util/flag/sectioned.go
+++ b/staging/src/k8s.io/apiserver/pkg/util/flag/sectioned.go
@ -52,6 +52,9 @@ func (nfs *NamedFlagSets) FlagSet(name string) *pflag.FlagSet {
 func PrintSections(w io.Writer, fss NamedFlagSets, cols int) {
 	for _, name := range fss.Order {
 		fs := fss.FlagSets[name]
+		if !fs.HasFlags() {
+			continue
+		}

 		wideFS := pflag.NewFlagSet("", pflag.ExitOnError)
 		wideFS.AddFlagSet(fs)