diff --git a/cmd/cloud-controller-manager/BUILD b/cmd/cloud-controller-manager/BUILD index e9f571902c3..e76c21f2211 100644 --- a/cmd/cloud-controller-manager/BUILD +++ b/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", ], ) diff --git a/cmd/cloud-controller-manager/app/BUILD b/cmd/cloud-controller-manager/app/BUILD index 78e517a9b30..75d7f8b3351 100644 --- a/cmd/cloud-controller-manager/app/BUILD +++ b/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", diff --git a/cmd/cloud-controller-manager/app/controllermanager.go b/cmd/cloud-controller-manager/app/controllermanager.go index 99cf6741926..73f99ecf778 100644 --- a/cmd/cloud-controller-manager/app/controllermanager.go +++ b/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) } diff --git a/cmd/cloud-controller-manager/controller-manager.go b/cmd/cloud-controller-manager/controller-manager.go index 39eacd52d61..280069e4d8f 100644 --- a/cmd/cloud-controller-manager/controller-manager.go +++ b/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() diff --git a/cmd/controller-manager/app/options/BUILD b/cmd/controller-manager/app/options/BUILD index 81939e51df2..aa5744e335a 100644 --- a/cmd/controller-manager/app/options/BUILD +++ b/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", + ], +) diff --git a/cmd/controller-manager/app/options/globalflags.go b/cmd/controller-manager/app/options/globalflags.go new file mode 100644 index 00000000000..b7a05ee2b06 --- /dev/null +++ b/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") +} diff --git a/cmd/controller-manager/app/options/globalflags_test.go b/cmd/controller-manager/app/options/globalflags_test.go new file mode 100644 index 00000000000..4c27d0223f7 --- /dev/null +++ b/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) + } +} diff --git a/cmd/kube-controller-manager/BUILD b/cmd/kube-controller-manager/BUILD index 20874148aa1..e21b302681b 100644 --- a/cmd/kube-controller-manager/BUILD +++ b/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", ], ) diff --git a/cmd/kube-controller-manager/app/BUILD b/cmd/kube-controller-manager/app/BUILD index 2fa0f43dc78..f4fdeb6af49 100644 --- a/cmd/kube-controller-manager/app/BUILD +++ b/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", diff --git a/cmd/kube-controller-manager/app/controllermanager.go b/cmd/kube-controller-manager/app/controllermanager.go index 5e686092194..2cf1ddf932f 100644 --- a/cmd/kube-controller-manager/app/controllermanager.go +++ b/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) } diff --git a/cmd/kube-controller-manager/controller-manager.go b/cmd/kube-controller-manager/controller-manager.go index 8dd1f29883f..7ae1192f97d 100644 --- a/cmd/kube-controller-manager/controller-manager.go +++ b/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() diff --git a/staging/src/k8s.io/apiserver/pkg/util/globalflag/globalflags.go b/staging/src/k8s.io/apiserver/pkg/util/globalflag/globalflags.go index c00ef7b9f1e..b49c8e9dc8f 100644 --- a/staging/src/k8s.io/apiserver/pkg/util/globalflag/globalflags.go +++ b/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 - global := flag.CommandLine - local := pflag.NewFlagSet(os.Args[0], pflag.ExitOnError) - - register(global, local, "logtostderr") - register(global, local, "alsologtostderr") - register(global, local, "v") - register(global, local, "skip_headers") - register(global, local, "stderrthreshold") - register(global, local, "vmodule") - register(global, local, "log_backtrace_at") - register(global, local, "log_dir") - register(global, local, "log_file") - - fs.AddFlagSet(local) + // lookup flags of klog libraries in global flag set and re-register the values with our flagset + Register(fs, "logtostderr") + Register(fs, "alsologtostderr") + Register(fs, "v") + Register(fs, "skip_headers") + Register(fs, "stderrthreshold") + Register(fs, "vmodule") + Register(fs, "log_backtrace_at") + Register(fs, "log_dir") + Register(fs, "log_file") } // 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 -func register(global *flag.FlagSet, local *pflag.FlagSet, globalName string) { - if f := global.Lookup(globalName); f != nil { +// Register adds a flag to local that targets the Value associated with the Flag named globalName in flag.CommandLine. +func Register(local *pflag.FlagSet, globalName string) { + if f := flag.CommandLine.Lookup(globalName); f != nil { pflagFlag := pflag.PFlagFromGoFlag(f) pflagFlag.Name = normalize(pflagFlag.Name) local.AddFlag(pflagFlag)