diff --git a/cmd/kube-scheduler/app/BUILD b/cmd/kube-scheduler/app/BUILD index 33a2b5a6c8..a3243f96ab 100644 --- a/cmd/kube-scheduler/app/BUILD +++ b/cmd/kube-scheduler/app/BUILD @@ -39,6 +39,8 @@ go_library( "//staging/src/k8s.io/apiserver/pkg/server/mux:go_default_library", "//staging/src/k8s.io/apiserver/pkg/server/routes: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/informers/storage/v1:go_default_library", "//staging/src/k8s.io/client-go/kubernetes/typed/core/v1:go_default_library", "//staging/src/k8s.io/client-go/tools/leaderelection:go_default_library", diff --git a/cmd/kube-scheduler/app/options/BUILD b/cmd/kube-scheduler/app/options/BUILD index dc66d81886..65050784ee 100644 --- a/cmd/kube-scheduler/app/options/BUILD +++ b/cmd/kube-scheduler/app/options/BUILD @@ -29,6 +29,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/util/validation/field: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", diff --git a/cmd/kube-scheduler/app/options/options.go b/cmd/kube-scheduler/app/options/options.go index 9fe7127334..0f4c7d0467 100644 --- a/cmd/kube-scheduler/app/options/options.go +++ b/cmd/kube-scheduler/app/options/options.go @@ -23,15 +23,13 @@ import ( "strconv" "time" - "github.com/spf13/pflag" - "k8s.io/klog" - corev1 "k8s.io/api/core/v1" apimachineryconfig "k8s.io/apimachinery/pkg/apis/config" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/uuid" 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" @@ -41,6 +39,7 @@ import ( "k8s.io/client-go/tools/leaderelection" "k8s.io/client-go/tools/leaderelection/resourcelock" "k8s.io/client-go/tools/record" + "k8s.io/klog" kubeschedulerconfigv1alpha1 "k8s.io/kube-scheduler/config/v1alpha1" schedulerappconfig "k8s.io/kubernetes/cmd/kube-scheduler/app/config" "k8s.io/kubernetes/pkg/api/legacyscheme" @@ -139,20 +138,23 @@ func newDefaultComponentConfig() (*kubeschedulerconfig.KubeSchedulerConfiguratio return &cfg, nil } -// AddFlags adds flags for the scheduler options. -func (o *Options) AddFlags(fs *pflag.FlagSet) { +// Flags returns flags for a specific scheduler by section name +func (o *Options) Flags() (nfs apiserverflag.NamedFlagSets) { + fs := nfs.FlagSet("misc") fs.StringVar(&o.ConfigFile, "config", o.ConfigFile, "The path to the configuration file. Flags override values in this file.") fs.StringVar(&o.WriteConfigTo, "write-config-to", o.WriteConfigTo, "If set, write the configuration values to this file and exit.") fs.StringVar(&o.Master, "master", o.Master, "The address of the Kubernetes API server (overrides any value in kubeconfig)") - o.SecureServing.AddFlags(fs) - o.CombinedInsecureServing.AddFlags(fs) - o.Authentication.AddFlags(fs) - o.Authorization.AddFlags(fs) - o.Deprecated.AddFlags(fs, &o.ComponentConfig) + o.SecureServing.AddFlags(nfs.FlagSet("secure serving")) + o.CombinedInsecureServing.AddFlags(nfs.FlagSet("insecure serving")) + o.Authentication.AddFlags(nfs.FlagSet("authentication")) + o.Authorization.AddFlags(nfs.FlagSet("authorization")) + o.Deprecated.AddFlags(nfs.FlagSet("deprecated"), &o.ComponentConfig) - leaderelectionconfig.BindFlags(&o.ComponentConfig.LeaderElection.LeaderElectionConfiguration, fs) - utilfeature.DefaultFeatureGate.AddFlag(fs) + leaderelectionconfig.BindFlags(&o.ComponentConfig.LeaderElection.LeaderElectionConfiguration, nfs.FlagSet("leader election")) + utilfeature.DefaultFeatureGate.AddFlag(nfs.FlagSet("feature gate")) + + return nfs } // ApplyTo applies the scheduler options to the given scheduler app configuration. diff --git a/cmd/kube-scheduler/app/server.go b/cmd/kube-scheduler/app/server.go index 7973b9cd50..9f43680ddb 100644 --- a/cmd/kube-scheduler/app/server.go +++ b/cmd/kube-scheduler/app/server.go @@ -39,6 +39,8 @@ import ( "k8s.io/apiserver/pkg/server/mux" "k8s.io/apiserver/pkg/server/routes" utilfeature "k8s.io/apiserver/pkg/util/feature" + apiserverflag "k8s.io/apiserver/pkg/util/flag" + "k8s.io/apiserver/pkg/util/globalflag" storageinformers "k8s.io/client-go/informers/storage/v1" v1core "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/tools/leaderelection" @@ -87,8 +89,25 @@ through the API as necessary.`, } }, } + fs := cmd.Flags() + namedFlagSets := opts.Flags() + verflag.AddFlags(namedFlagSets.FlagSet("global")) + globalflag.AddGlobalFlags(namedFlagSets.FlagSet("global"), cmd.Name()) + for _, f := range namedFlagSets.FlagSets { + fs.AddFlagSet(f) + } - opts.AddFlags(cmd.Flags()) + 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) + }) cmd.MarkFlagFilename("config", "yaml", "yml", "json") return cmd diff --git a/cmd/kube-scheduler/app/testing/testserver.go b/cmd/kube-scheduler/app/testing/testserver.go index 0c5250cd44..9ce7ec3dbe 100644 --- a/cmd/kube-scheduler/app/testing/testserver.go +++ b/cmd/kube-scheduler/app/testing/testserver.go @@ -86,7 +86,10 @@ func StartTestServer(t Logger, customFlags []string) (result TestServer, err err if err != nil { return TestServer{}, err } - s.AddFlags(fs) + namedFlagSets := s.Flags() + for _, f := range namedFlagSets.FlagSets { + fs.AddFlagSet(f) + } fs.Parse(customFlags) diff --git a/cmd/kube-scheduler/scheduler.go b/cmd/kube-scheduler/scheduler.go index 45e1b2617f..5d44ed612c 100644 --- a/cmd/kube-scheduler/scheduler.go +++ b/cmd/kube-scheduler/scheduler.go @@ -17,13 +17,13 @@ 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-scheduler/app" @@ -40,7 +40,6 @@ func main() { // 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/BUILD b/staging/src/BUILD index eef546e103..ded0e944e7 100644 --- a/staging/src/BUILD +++ b/staging/src/BUILD @@ -102,6 +102,7 @@ filegroup( "//staging/src/k8s.io/apiserver/pkg/util/feature:all-srcs", "//staging/src/k8s.io/apiserver/pkg/util/flag:all-srcs", "//staging/src/k8s.io/apiserver/pkg/util/flushwriter:all-srcs", + "//staging/src/k8s.io/apiserver/pkg/util/globalflag:all-srcs", "//staging/src/k8s.io/apiserver/pkg/util/logs:all-srcs", "//staging/src/k8s.io/apiserver/pkg/util/openapi:all-srcs", "//staging/src/k8s.io/apiserver/pkg/util/proxy:all-srcs", diff --git a/staging/src/k8s.io/apiserver/pkg/util/globalflag/BUILD b/staging/src/k8s.io/apiserver/pkg/util/globalflag/BUILD new file mode 100644 index 0000000000..9600afb0b4 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/util/globalflag/BUILD @@ -0,0 +1,37 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = ["globalflags.go"], + importmap = "k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/util/globalflag", + importpath = "k8s.io/apiserver/pkg/util/globalflag", + visibility = ["//visibility:public"], + deps = [ + "//staging/src/k8s.io/apiserver/pkg/util/logs:go_default_library", + "//vendor/github.com/spf13/pflag:go_default_library", + ], +) + +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", + "//vendor/github.com/spf13/pflag:go_default_library", + ], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/staging/src/k8s.io/apiserver/pkg/util/globalflag/globalflags.go b/staging/src/k8s.io/apiserver/pkg/util/globalflag/globalflags.go new file mode 100644 index 0000000000..c00ef7b9f1 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/util/globalflag/globalflags.go @@ -0,0 +1,74 @@ +/* +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 globalflag + +import ( + "flag" + "fmt" + "os" + "strings" + + "github.com/spf13/pflag" + + "k8s.io/apiserver/pkg/util/logs" +) + +// AddGlobalFlags explicitly registers flags that libraries (klog, verflag, etc.) register +// against the global flagsets from "flag" and "github.com/spf13/pflag". +// 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) + logs.AddFlags(fs) + + fs.BoolP("help", "h", false, fmt.Sprintf("help for %s", name)) +} + +// 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) +} + +// normalize replaces underscores with hyphens +// we should always use hyphens instead of underscores when registering component flags +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 { + pflagFlag := pflag.PFlagFromGoFlag(f) + pflagFlag.Name = normalize(pflagFlag.Name) + local.AddFlag(pflagFlag) + } else { + panic(fmt.Sprintf("failed to find flag in global flagset (flag): %s", globalName)) + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/util/globalflag/globalflags_test.go b/staging/src/k8s.io/apiserver/pkg/util/globalflag/globalflags_test.go new file mode 100644 index 0000000000..16475154ab --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/util/globalflag/globalflags_test.go @@ -0,0 +1,86 @@ +/* +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 globalflag + +import ( + "flag" + "reflect" + "sort" + "strings" + "testing" + + "github.com/spf13/pflag" + + apiserverflag "k8s.io/apiserver/pkg/util/flag" +) + +func TestAddGlobalFlags(t *testing.T) { + namedFlagSets := &apiserverflag.NamedFlagSets{} + nfs := namedFlagSets.FlagSet("global") + AddGlobalFlags(nfs, "test-cmd") + + 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.AddGoFlagSet(flag.CommandLine) + pflag.VisitAll(func(flag *pflag.Flag) { + if !strings.Contains(flag.Name, "test.") { + wantedFlag = append(wantedFlag, normalize(flag.Name)) + } + }) + sort.Strings(wantedFlag) + + if !reflect.DeepEqual(wantedFlag, actualFlag) { + t.Errorf("[Default]: expected %+v, got %+v", wantedFlag, actualFlag) + } + + tests := []struct { + expectedFlag []string + matchExpected bool + }{ + { + // Happy case + expectedFlag: []string{"alsologtostderr", "help", "log-backtrace-at", "log-dir", "log-file", "log-flush-frequency", "logtostderr", "skip-headers", "stderrthreshold", "v", "vmodule"}, + matchExpected: false, + }, + { + // Missing flag + expectedFlag: []string{"logtostderr", "log-dir"}, + matchExpected: true, + }, + { + // Empty flag + expectedFlag: []string{}, + matchExpected: true, + }, + { + // Invalid flag + expectedFlag: []string{"foo"}, + matchExpected: true, + }, + } + + for i, test := range tests { + if reflect.DeepEqual(test.expectedFlag, actualFlag) == test.matchExpected { + t.Errorf("[%d]: expected %+v, got %+v", i, test.expectedFlag, actualFlag) + } + } +}