Merge pull request #71298 from stewart-yu/stewart-controller-manager-flags

add missing flags in *-controller-manager --help

cmd/cloud-controller-manager/BUILD

@@ -23,9 +23,7 @@ go_library(
         "//pkg/client/metrics/prometheus:go_default_library",
         "//pkg/cloudprovider/providers:go_default_library",
         "//pkg/version/prometheus:go_default_library",
-        "//staging/src/k8s.io/apiserver/pkg/util/flag:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/util/logs:go_default_library",
-        "//vendor/github.com/spf13/pflag:go_default_library",
     ],
 )
 

cmd/cloud-controller-manager/app/BUILD

@@ -9,6 +9,7 @@ go_library(
         "//cmd/cloud-controller-manager/app/config:go_default_library",
         "//cmd/cloud-controller-manager/app/options:go_default_library",
         "//cmd/controller-manager/app:go_default_library",
+        "//cmd/controller-manager/app/options:go_default_library",
         "//pkg/controller/cloud:go_default_library",
         "//pkg/controller/route:go_default_library",
         "//pkg/controller/service:go_default_library",
@@ -21,6 +22,7 @@ go_library(
         "//staging/src/k8s.io/apiserver/pkg/server:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/server/healthz:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/util/flag:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/util/globalflag: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",

cmd/cloud-controller-manager/app/controllermanager.go

@@ -25,20 +25,22 @@ import (
 	"time"
 
 	"github.com/spf13/cobra"
-	"k8s.io/klog"
 
 	"k8s.io/apimachinery/pkg/util/uuid"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/apiserver/pkg/server"
 	"k8s.io/apiserver/pkg/server/healthz"
 	apiserverflag "k8s.io/apiserver/pkg/util/flag"
+	"k8s.io/apiserver/pkg/util/globalflag"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/leaderelection"
 	"k8s.io/client-go/tools/leaderelection/resourcelock"
 	cloudprovider "k8s.io/cloud-provider"
+	"k8s.io/klog"
 	cloudcontrollerconfig "k8s.io/kubernetes/cmd/cloud-controller-manager/app/config"
 	"k8s.io/kubernetes/cmd/cloud-controller-manager/app/options"
 	genericcontrollermanager "k8s.io/kubernetes/cmd/controller-manager/app"
+	cmoptions "k8s.io/kubernetes/cmd/controller-manager/app/options"
 	cloudcontrollers "k8s.io/kubernetes/pkg/controller/cloud"
 	routecontroller "k8s.io/kubernetes/pkg/controller/route"
 	servicecontroller "k8s.io/kubernetes/pkg/controller/service"
@@ -86,6 +88,9 @@ the cloud specific control loops shipped with Kubernetes.`,
 
 	fs := cmd.Flags()
 	namedFlagSets := s.Flags()
+	verflag.AddFlags(namedFlagSets.FlagSet("global"))
+	globalflag.AddGlobalFlags(namedFlagSets.FlagSet("global"), cmd.Name())
+	cmoptions.AddCustomGlobalFlags(namedFlagSets.FlagSet("generic"))
 	for _, f := range namedFlagSets.FlagSets {
 		fs.AddFlagSet(f)
 	}

cmd/cloud-controller-manager/controller-manager.go

@@ -20,13 +20,11 @@ limitations under the License.
 package main
 
 import (
-	goflag "flag"
 	"fmt"
 	"math/rand"
 	"os"
 	"time"
 
-	utilflag "k8s.io/apiserver/pkg/util/flag"
 	"k8s.io/apiserver/pkg/util/logs"
 	"k8s.io/kubernetes/cmd/cloud-controller-manager/app"
 	_ "k8s.io/kubernetes/pkg/client/metrics/prometheus" // for client metric registration
@@ -34,8 +32,6 @@ import (
 	// implementing an out-of-tree cloud-provider.
 	_ "k8s.io/kubernetes/pkg/cloudprovider/providers"
 	_ "k8s.io/kubernetes/pkg/version/prometheus" // for version metric registration
-
-	"github.com/spf13/pflag"
 )
 
 func main() {
@@ -46,8 +42,6 @@ func main() {
 	// TODO: once we switch everything over to Cobra commands, we can go back to calling
 	// utilflag.InitFlags() (by removing its pflag.Parse() call). For now, we have to set the
 	// normalize func and add the go flag set by hand.
-	pflag.CommandLine.SetNormalizeFunc(utilflag.WordSepNormalizeFunc)
-	pflag.CommandLine.AddGoFlagSet(goflag.CommandLine)
 	// utilflag.InitFlags()
 	logs.InitLogs()
 	defer logs.FlushLogs()

cmd/controller-manager/app/options/BUILD

@@ -1,4 +1,4 @@
-load("@io_bazel_rules_go//go:def.bzl", "go_library")
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 
 go_library(
     name = "go_default_library",
@@ -6,6 +6,7 @@ go_library(
         "cloudprovider.go",
         "debugging.go",
         "generic.go",
+        "globalflags.go",
         "kubecloudshared.go",
         "servicecontroller.go",
     ],
@@ -13,12 +14,14 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//pkg/client/leaderelectionconfig:go_default_library",
+        "//pkg/cloudprovider/providers:go_default_library",
         "//pkg/controller/apis/config:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/config:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/apis/config:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/util/flag:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/util/globalflag:go_default_library",
         "//vendor/github.com/spf13/pflag:go_default_library",
     ],
 )
@@ -36,3 +39,14 @@ filegroup(
     tags = ["automanaged"],
     visibility = ["//visibility:public"],
 )
+
+go_test(
+    name = "go_default_test",
+    srcs = ["globalflags_test.go"],
+    embed = [":go_default_library"],
+    deps = [
+        "//staging/src/k8s.io/apiserver/pkg/util/flag:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/util/globalflag:go_default_library",
+        "//vendor/github.com/spf13/pflag:go_default_library",
+    ],
+)

cmd/controller-manager/app/options/globalflags.go

@@ -0,0 +1,35 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package options
+
+import (
+	"github.com/spf13/pflag"
+
+	"k8s.io/apiserver/pkg/util/globalflag"
+
+	// ensure libs have a chance to globally register their flags
+	_ "k8s.io/kubernetes/pkg/cloudprovider/providers"
+)
+
+// AddCustomGlobalFlags explicitly registers flags that internal packages register
+// against the global flagsets from "flag". We do this in order to prevent
+// unwanted flags from leaking into the *-controller-manager's flagset.
+func AddCustomGlobalFlags(fs *pflag.FlagSet) {
+	// lookup flags in global flag set and re-register the values with our flagset
+	// adds flags from k8s.io/kubernetes/pkg/cloudprovider/providers
+	globalflag.Register(fs, "cloud-provider-gce-lb-src-cidrs")
+}

cmd/controller-manager/app/options/globalflags_test.go

@@ -0,0 +1,60 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package options
+
+import (
+	"flag"
+	"reflect"
+	"sort"
+	"strings"
+	"testing"
+
+	"github.com/spf13/pflag"
+
+	apiserverflag "k8s.io/apiserver/pkg/util/flag"
+	"k8s.io/apiserver/pkg/util/globalflag"
+)
+
+func TestAddCustomGlobalFlags(t *testing.T) {
+	namedFlagSets := &apiserverflag.NamedFlagSets{}
+	// Note that we will register all flags (including klog flags) into the same
+	// flag set. This allows us to test against all global flags from
+	// flags.CommandLine.
+	nfs := namedFlagSets.FlagSet("generic")
+	globalflag.AddGlobalFlags(nfs, "test-cmd")
+	AddCustomGlobalFlags(nfs)
+
+	actualFlag := []string{}
+	nfs.VisitAll(func(flag *pflag.Flag) {
+		actualFlag = append(actualFlag, flag.Name)
+	})
+
+	// Get all flags from flags.CommandLine, except flag `test.*`.
+	wantedFlag := []string{"help"}
+	pflag.CommandLine.SetNormalizeFunc(apiserverflag.WordSepNormalizeFunc)
+	pflag.CommandLine.AddGoFlagSet(flag.CommandLine)
+	pflag.VisitAll(func(flag *pflag.Flag) {
+		if !strings.Contains(flag.Name, "test.") {
+			wantedFlag = append(wantedFlag, flag.Name)
+		}
+	})
+	sort.Strings(wantedFlag)
+
+	if !reflect.DeepEqual(wantedFlag, actualFlag) {
+		t.Errorf("Got different flags than expected: expected %+v, got %+v", wantedFlag, actualFlag)
+	}
+}

cmd/kube-controller-manager/BUILD

@@ -24,9 +24,7 @@ go_library(
         "//pkg/util/reflector/prometheus:go_default_library",
         "//pkg/util/workqueue/prometheus:go_default_library",
         "//pkg/version/prometheus:go_default_library",
-        "//staging/src/k8s.io/apiserver/pkg/util/flag:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/util/logs:go_default_library",
-        "//vendor/github.com/spf13/pflag:go_default_library",
     ],
 )
 

cmd/kube-controller-manager/app/BUILD

@@ -20,6 +20,7 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//cmd/controller-manager/app:go_default_library",
+        "//cmd/controller-manager/app/options:go_default_library",
         "//cmd/kube-controller-manager/app/config:go_default_library",
         "//cmd/kube-controller-manager/app/options:go_default_library",
         "//pkg/apis/apps/install:go_default_library",
@@ -117,6 +118,7 @@ go_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/apiserver/pkg/util/globalflag: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",

cmd/kube-controller-manager/app/controllermanager.go

@@ -30,7 +30,6 @@ import (
 	"time"
 
 	"github.com/spf13/cobra"
-	"k8s.io/klog"
 
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
@@ -41,6 +40,7 @@ import (
 	"k8s.io/apiserver/pkg/server/healthz"
 	"k8s.io/apiserver/pkg/server/mux"
 	apiserverflag "k8s.io/apiserver/pkg/util/flag"
+	"k8s.io/apiserver/pkg/util/globalflag"
 	cacheddiscovery "k8s.io/client-go/discovery/cached"
 	"k8s.io/client-go/informers"
 	restclient "k8s.io/client-go/rest"
@@ -49,7 +49,9 @@ import (
 	"k8s.io/client-go/tools/leaderelection/resourcelock"
 	certutil "k8s.io/client-go/util/cert"
 	cloudprovider "k8s.io/cloud-provider"
+	"k8s.io/klog"
 	genericcontrollermanager "k8s.io/kubernetes/cmd/controller-manager/app"
+	cmoptions "k8s.io/kubernetes/cmd/controller-manager/app/options"
 	"k8s.io/kubernetes/cmd/kube-controller-manager/app/config"
 	"k8s.io/kubernetes/cmd/kube-controller-manager/app/options"
 	"k8s.io/kubernetes/pkg/controller"
@@ -112,6 +114,9 @@ controller, and serviceaccounts controller.`,
 
 	fs := cmd.Flags()
 	namedFlagSets := s.Flags(KnownControllers(), ControllersDisabledByDefault.List())
+	verflag.AddFlags(namedFlagSets.FlagSet("global"))
+	globalflag.AddGlobalFlags(namedFlagSets.FlagSet("global"), cmd.Name())
+	cmoptions.AddCustomGlobalFlags(namedFlagSets.FlagSet("generic"))
 	for _, f := range namedFlagSets.FlagSets {
 		fs.AddFlagSet(f)
 	}

cmd/kube-controller-manager/controller-manager.go

@@ -21,15 +21,11 @@ limitations under the License.
 package main
 
 import (
-	goflag "flag"
 	"fmt"
 	"math/rand"
 	"os"
 	"time"
 
-	"github.com/spf13/pflag"
-
-	utilflag "k8s.io/apiserver/pkg/util/flag"
 	"k8s.io/apiserver/pkg/util/logs"
 	"k8s.io/kubernetes/cmd/kube-controller-manager/app"
 	_ "k8s.io/kubernetes/pkg/client/metrics/prometheus" // for client metric registration
@@ -46,8 +42,6 @@ func main() {
 	// TODO: once we switch everything over to Cobra commands, we can go back to calling
 	// utilflag.InitFlags() (by removing its pflag.Parse() call). For now, we have to set the
 	// normalize func and add the go flag set by hand.
-	pflag.CommandLine.SetNormalizeFunc(utilflag.WordSepNormalizeFunc)
-	pflag.CommandLine.AddGoFlagSet(goflag.CommandLine)
 	// utilflag.InitFlags()
 	logs.InitLogs()
 	defer logs.FlushLogs()

staging/src/k8s.io/apiserver/pkg/util/globalflag/globalflags.go

@@ -19,7 +19,6 @@ package globalflag
 import (
 	"flag"
 	"fmt"
-	"os"
 	"strings"
 
 	"github.com/spf13/pflag"
@@ -28,7 +27,7 @@ import (
 )
 
 // AddGlobalFlags explicitly registers flags that libraries (klog, verflag, etc.) register
-// against the global flagsets from "flag" and "github.com/spf13/pflag".
+// against the global flagsets from "flag" and "k8s.io/klog".
 // We do this in order to prevent unwanted flags from leaking into the component's flagset.
 func AddGlobalFlags(fs *pflag.FlagSet, name string) {
 	addGlogFlags(fs)
@@ -39,21 +38,16 @@ func AddGlobalFlags(fs *pflag.FlagSet, name string) {
 
 // addGlogFlags explicitly registers flags that klog libraries(k8s.io/klog) register.
 func addGlogFlags(fs *pflag.FlagSet) {
-	// lookup flags in global flag set and re-register the values with our flagset
+	// lookup flags of klog libraries in global flag set and re-register the values with our flagset
-	global := flag.CommandLine
+	Register(fs, "logtostderr")
-	local := pflag.NewFlagSet(os.Args[0], pflag.ExitOnError)
+	Register(fs, "alsologtostderr")
-
+	Register(fs, "v")
-	register(global, local, "logtostderr")
+	Register(fs, "skip_headers")
-	register(global, local, "alsologtostderr")
+	Register(fs, "stderrthreshold")
-	register(global, local, "v")
+	Register(fs, "vmodule")
-	register(global, local, "skip_headers")
+	Register(fs, "log_backtrace_at")
-	register(global, local, "stderrthreshold")
+	Register(fs, "log_dir")
-	register(global, local, "vmodule")
+	Register(fs, "log_file")
-	register(global, local, "log_backtrace_at")
-	register(global, local, "log_dir")
-	register(global, local, "log_file")
-
-	fs.AddFlagSet(local)
 }
 
 // normalize replaces underscores with hyphens
@@ -62,9 +56,9 @@ func normalize(s string) string {
 	return strings.Replace(s, "_", "-", -1)
 }
 
-// register adds a flag to local that targets the Value associated with the Flag named globalName in global
+// Register adds a flag to local that targets the Value associated with the Flag named globalName in flag.CommandLine.
-func register(global *flag.FlagSet, local *pflag.FlagSet, globalName string) {
+func Register(local *pflag.FlagSet, globalName string) {
-	if f := global.Lookup(globalName); f != nil {
+	if f := flag.CommandLine.Lookup(globalName); f != nil {
 		pflagFlag := pflag.PFlagFromGoFlag(f)
 		pflagFlag.Name = normalize(pflagFlag.Name)
 		local.AddFlag(pflagFlag)