Merge pull request #85266 from serathius/refactor-show-hidden-metric

Refactor show-hidden-metric-for-version flag

cmd/kube-apiserver/app/BUILD

@@ -65,7 +65,6 @@ go_library(
         "//staging/src/k8s.io/cloud-provider:go_default_library",
         "//staging/src/k8s.io/component-base/cli/flag:go_default_library",
         "//staging/src/k8s.io/component-base/cli/globalflag:go_default_library",
-        "//staging/src/k8s.io/component-base/metrics:go_default_library",
         "//staging/src/k8s.io/component-base/metrics/prometheus/workqueue:go_default_library",
         "//staging/src/k8s.io/component-base/term:go_default_library",
         "//staging/src/k8s.io/component-base/version:go_default_library",

cmd/kube-apiserver/app/options/BUILD

@@ -66,6 +66,7 @@ go_test(
         "//staging/src/k8s.io/component-base/cli/flag:go_default_library",
         "//staging/src/k8s.io/component-base/cli/globalflag:go_default_library",
         "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library",
+        "//staging/src/k8s.io/component-base/metrics:go_default_library",
         "//vendor/github.com/google/go-cmp/cmp:go_default_library",
         "//vendor/github.com/google/go-cmp/cmp/cmpopts:go_default_library",
         "//vendor/github.com/spf13/pflag:go_default_library",

cmd/kube-apiserver/app/options/options.go

@@ -26,6 +26,7 @@ import (
 	genericoptions "k8s.io/apiserver/pkg/server/options"
 	"k8s.io/apiserver/pkg/storage/storagebackend"
 	cliflag "k8s.io/component-base/cli/flag"
+	"k8s.io/component-base/metrics"
 	api "k8s.io/kubernetes/pkg/apis/core"
 	_ "k8s.io/kubernetes/pkg/features" // add the kubernetes feature gates
 	kubeoptions "k8s.io/kubernetes/pkg/kubeapiserver/options"
@@ -49,6 +50,7 @@ type ServerRunOptions struct {
 	CloudProvider           *kubeoptions.CloudProviderOptions
 	APIEnablement           *genericoptions.APIEnablementOptions
 	EgressSelector          *genericoptions.EgressSelectorOptions
+	Metrics                 *metrics.Options
 
 	AllowPrivileged           bool
 	EnableLogsHandler         bool
@@ -97,6 +99,7 @@ func NewServerRunOptions() *ServerRunOptions {
 		CloudProvider:           kubeoptions.NewCloudProviderOptions(),
 		APIEnablement:           genericoptions.NewAPIEnablementOptions(),
 		EgressSelector:          genericoptions.NewEgressSelectorOptions(),
+		Metrics:                 metrics.NewOptions(),
 
 		EnableLogsHandler:      true,
 		EventTTL:               1 * time.Hour,
@@ -145,15 +148,7 @@ func (s *ServerRunOptions) Flags() (fss cliflag.NamedFlagSets) {
 	s.APIEnablement.AddFlags(fss.FlagSet("API enablement"))
 	s.EgressSelector.AddFlags(fss.FlagSet("egress selector"))
 	s.Admission.AddFlags(fss.FlagSet("admission"))
-
+	s.Metrics.AddFlags(fss.FlagSet("metrics"))
-	// TODO(RainbowMango): move it to genericoptions before next flag comes.
-	mfs := fss.FlagSet("metrics")
-	mfs.StringVar(&s.ShowHiddenMetricsForVersion, "show-hidden-metrics-for-version", s.ShowHiddenMetricsForVersion,
-		"The previous version for which you want to show hidden metrics. "+
-			"Only the previous minor version is meaningful, other values will not be allowed. "+
-			"The format is <major>.<minor>, e.g.: '1.16'. "+
-			"The purpose of this format is make sure you have the opportunity to notice if the next release hides additional metrics, "+
-			"rather than being surprised when they are permanently removed in the release after that.")
 
 	// Note: the weird ""+ in below lines seems to be the only way to get gofmt to
 	// arrange these text blocks sensibly. Grrr.

cmd/kube-apiserver/app/options/options_test.go

@@ -34,6 +34,7 @@ import (
 	audittruncate "k8s.io/apiserver/plugin/pkg/audit/truncate"
 	restclient "k8s.io/client-go/rest"
 	cliflag "k8s.io/component-base/cli/flag"
+	"k8s.io/component-base/metrics"
 	kapi "k8s.io/kubernetes/pkg/apis/core"
 	kubeoptions "k8s.io/kubernetes/pkg/kubeapiserver/options"
 	kubeletclient "k8s.io/kubernetes/pkg/kubelet/client"
@@ -307,6 +308,7 @@ func TestAddFlags(t *testing.T) {
 		EnableAggregatorRouting: true,
 		ProxyClientKeyFile:      "/var/run/kubernetes/proxy.key",
 		ProxyClientCertFile:     "/var/run/kubernetes/proxy.crt",
+		Metrics:                 &metrics.Options{},
 	}
 
 	if !reflect.DeepEqual(expected, s) {

cmd/kube-apiserver/app/options/validation.go

@@ -25,7 +25,6 @@ import (
 	apiextensionsapiserver "k8s.io/apiextensions-apiserver/pkg/apiserver"
 	genericfeatures "k8s.io/apiserver/pkg/features"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
-	"k8s.io/component-base/metrics"
 	aggregatorscheme "k8s.io/kube-aggregator/pkg/apiserver/scheme"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	"k8s.io/kubernetes/pkg/features"
@@ -165,7 +164,7 @@ func (s *ServerRunOptions) Validate() []error {
 	errs = append(errs, s.InsecureServing.Validate()...)
 	errs = append(errs, s.APIEnablement.Validate(legacyscheme.Scheme, apiextensionsapiserver.Scheme, aggregatorscheme.Scheme)...)
 	errs = append(errs, validateTokenRequest(s)...)
-	errs = append(errs, metrics.ValidateShowHiddenMetricsVersion(s.ShowHiddenMetricsForVersion)...)
+	errs = append(errs, s.Metrics.Validate()...)
 
 	return errs
 }

cmd/kube-apiserver/app/server.go

@@ -60,7 +60,6 @@ import (
 	cloudprovider "k8s.io/cloud-provider"
 	cliflag "k8s.io/component-base/cli/flag"
 	"k8s.io/component-base/cli/globalflag"
-	"k8s.io/component-base/metrics"
 	_ "k8s.io/component-base/metrics/prometheus/workqueue" // for workqueue metric registration
 	"k8s.io/component-base/term"
 	"k8s.io/component-base/version"
@@ -306,9 +305,7 @@ func CreateKubeAPIServerConfig(
 		PerConnectionBandwidthLimitBytesPerSec: s.MaxConnectionBytesPerSec,
 	})
 
-	if len(s.ShowHiddenMetricsForVersion) > 0 {
+	s.Metrics.Apply()
-		metrics.SetShowHidden()
-	}
 
 	serviceIPRange, apiServerServiceIP, err := master.ServiceIPRange(s.PrimaryServiceClusterIPRange)
 	if err != nil {

cmd/kube-controller-manager/app/options/BUILD

@@ -125,6 +125,7 @@ go_test(
         "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/server/options:go_default_library",
         "//staging/src/k8s.io/component-base/config:go_default_library",
+        "//staging/src/k8s.io/component-base/metrics:go_default_library",
         "//vendor/github.com/spf13/pflag:go_default_library",
     ],
 )

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

@@ -87,6 +87,7 @@ type KubeControllerManagerOptions struct {
 	InsecureServing *apiserveroptions.DeprecatedInsecureServingOptionsWithLoopback
 	Authentication  *apiserveroptions.DelegatingAuthenticationOptions
 	Authorization   *apiserveroptions.DelegatingAuthorizationOptions
+	Metrics         *metrics.Options
 
 	Master                      string
 	Kubeconfig                  string
@@ -177,6 +178,7 @@ func NewKubeControllerManagerOptions() (*KubeControllerManagerOptions, error) {
 		}).WithLoopback(),
 		Authentication: apiserveroptions.NewDelegatingAuthenticationOptions(),
 		Authorization:  apiserveroptions.NewDelegatingAuthorizationOptions(),
+		Metrics:        metrics.NewOptions(),
 	}
 
 	s.Authentication.RemoteKubeConfigFileOptional = true
@@ -246,20 +248,13 @@ func (s *KubeControllerManagerOptions) Flags(allControllers []string, disabledBy
 	s.ResourceQuotaController.AddFlags(fss.FlagSet("resourcequota controller"))
 	s.SAController.AddFlags(fss.FlagSet("serviceaccount controller"))
 	s.TTLAfterFinishedController.AddFlags(fss.FlagSet("ttl-after-finished controller"))
+	s.Metrics.AddFlags(fss.FlagSet("metrics"))
 
 	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.")
 	utilfeature.DefaultMutableFeatureGate.AddFlag(fss.FlagSet("generic"))
 
-	mfs := fss.FlagSet("metrics")
-	mfs.StringVar(&s.ShowHiddenMetricsForVersion, "show-hidden-metrics-for-version", s.ShowHiddenMetricsForVersion,
-		"The previous version for which you want to show hidden metrics. "+
-			"Only the previous minor version is meaningful, other values will not be allowed. "+
-			"The format is <major>.<minor>, e.g.: '1.16'. "+
-			"The purpose of this format is make sure you have the opportunity to notice if the next release hides additional metrics, "+
-			"rather than being surprised when they are permanently removed in the release after that.")
-
 	return fss
 }
 
@@ -392,7 +387,7 @@ func (s *KubeControllerManagerOptions) Validate(allControllers []string, disable
 	errs = append(errs, s.InsecureServing.Validate()...)
 	errs = append(errs, s.Authentication.Validate()...)
 	errs = append(errs, s.Authorization.Validate()...)
-	errs = append(errs, metrics.ValidateShowHiddenMetricsVersion(s.ShowHiddenMetricsForVersion)...)
+	errs = append(errs, s.Metrics.Validate()...)
 
 	// TODO: validate component config, master and kubeconfig
 
@@ -440,10 +435,7 @@ func (s KubeControllerManagerOptions) Config(allControllers []string, disabledBy
 	if err := s.ApplyTo(c); err != nil {
 		return nil, err
 	}
-
+	s.Metrics.Apply()
-	if len(s.ShowHiddenMetricsForVersion) > 0 {
-		metrics.SetShowHidden()
-	}
 
 	return c, nil
 }

cmd/kube-controller-manager/app/options/options_test.go

@@ -29,6 +29,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/diff"
 	apiserveroptions "k8s.io/apiserver/pkg/server/options"
 	componentbaseconfig "k8s.io/component-base/config"
+	"k8s.io/component-base/metrics"
 	cmoptions "k8s.io/kubernetes/cmd/controller-manager/app/options"
 	kubecontrollerconfig "k8s.io/kubernetes/cmd/kube-controller-manager/app/config"
 	kubectrlmgrconfig "k8s.io/kubernetes/pkg/controller/apis/config"
@@ -381,6 +382,7 @@ func TestAddFlags(t *testing.T) {
 		},
 		Kubeconfig: "/kubeconfig",
 		Master:     "192.168.4.20",
+		Metrics:    &metrics.Options{},
 	}
 
 	// Sort GCIgnoredResources because it's built from a map, which means the

cmd/kube-scheduler/app/options/options.go

@@ -61,6 +61,7 @@ type Options struct {
 	CombinedInsecureServing *CombinedInsecureServingOptions
 	Authentication          *apiserveroptions.DelegatingAuthenticationOptions
 	Authorization           *apiserveroptions.DelegatingAuthorizationOptions
+	Metrics                 *metrics.Options
 	Deprecated              *DeprecatedOptions
 
 	// ConfigFile is the location of the scheduler server's configuration file.
@@ -70,8 +71,6 @@ type Options struct {
 	WriteConfigTo string
 
 	Master string
-
-	ShowHiddenMetricsForVersion string
 }
 
 // NewOptions returns default scheduler app options.
@@ -107,6 +106,7 @@ func NewOptions() (*Options, error) {
 			SchedulerName:                  corev1.DefaultSchedulerName,
 			HardPodAffinitySymmetricWeight: interpodaffinity.DefaultHardPodAffinityWeight,
 		},
+		Metrics: metrics.NewOptions(),
 	}
 
 	o.Authentication.TolerateInClusterLookupFailure = true
@@ -161,15 +161,7 @@ func (o *Options) Flags() (nfs cliflag.NamedFlagSets) {
 
 	leaderelectionconfig.BindFlags(&o.ComponentConfig.LeaderElection.LeaderElectionConfiguration, nfs.FlagSet("leader election"))
 	utilfeature.DefaultMutableFeatureGate.AddFlag(nfs.FlagSet("feature gate"))
-
+	o.Metrics.AddFlags(nfs.FlagSet("metrics"))
-	// TODO(RainbowMango): move it to genericoptions before next flag comes.
-	mfs := nfs.FlagSet("metrics")
-	mfs.StringVar(&o.ShowHiddenMetricsForVersion, "show-hidden-metrics-for-version", o.ShowHiddenMetricsForVersion,
-		"The previous version for which you want to show hidden metrics. "+
-			"Only the previous minor version is meaningful, other values will not be allowed. "+
-			"Accepted format of version is <major>.<minor>, e.g.: '1.16'. "+
-			"The purpose of this format is make sure you have the opportunity to notice if the next release hides additional metrics, "+
-			"rather than being surprised when they are permanently removed in the release after that.")
 
 	return nfs
 }
@@ -216,10 +208,7 @@ func (o *Options) ApplyTo(c *schedulerappconfig.Config) error {
 			return err
 		}
 	}
-	if len(o.ShowHiddenMetricsForVersion) > 0 {
+	o.Metrics.Apply()
-		metrics.SetShowHidden()
-	}
-
 	return nil
 }
 
@@ -235,7 +224,7 @@ func (o *Options) Validate() []error {
 	errs = append(errs, o.Authentication.Validate()...)
 	errs = append(errs, o.Authorization.Validate()...)
 	errs = append(errs, o.Deprecated.Validate()...)
-	errs = append(errs, metrics.ValidateShowHiddenMetricsVersion(o.ShowHiddenMetricsForVersion)...)
+	errs = append(errs, o.Metrics.Validate()...)
 
 	return errs
 }

staging/src/k8s.io/component-base/metrics/BUILD

@@ -11,6 +11,7 @@ go_library(
         "http.go",
         "labels.go",
         "metric.go",
+        "options.go",
         "opts.go",
         "processstarttime.go",
         "registry.go",
@@ -31,6 +32,7 @@ go_library(
         "//vendor/github.com/prometheus/client_golang/prometheus/promhttp:go_default_library",
         "//vendor/github.com/prometheus/client_model/go:go_default_library",
         "//vendor/github.com/prometheus/procfs:go_default_library",
+        "//vendor/github.com/spf13/pflag:go_default_library",
         "//vendor/k8s.io/klog:go_default_library",
     ],
 )

staging/src/k8s.io/component-base/metrics/options.go

@@ -0,0 +1,79 @@
+/*
+Copyright 2019 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 metrics
+
+import (
+	"fmt"
+
+	"github.com/blang/semver"
+	"github.com/spf13/pflag"
+
+	"k8s.io/component-base/version"
+)
+
+// Options has all parameters needed for exposing metrics from components
+type Options struct {
+	ShowHiddenMetricsForVersion string
+}
+
+// NewOptions returns default metrics options
+func NewOptions() *Options {
+	return &Options{}
+}
+
+// Validate validates metrics flags options.
+func (o *Options) Validate() []error {
+	err := validateShowHiddenMetricsVersion(parseVersion(version.Get()), o.ShowHiddenMetricsForVersion)
+	if err != nil {
+		return []error{err}
+	}
+
+	return nil
+}
+
+// AddFlags adds flags for exposing component metrics.
+func (o *Options) AddFlags(fs *pflag.FlagSet) {
+	if o != nil {
+		o = NewOptions()
+	}
+	fs.StringVar(&o.ShowHiddenMetricsForVersion, "show-hidden-metrics-for-version", o.ShowHiddenMetricsForVersion,
+		"The previous version for which you want to show hidden metrics. "+
+			"Only the previous minor version is meaningful, other values will not be allowed. "+
+			"The format is <major>.<minor>, e.g.: '1.16'. "+
+			"The purpose of this format is make sure you have the opportunity to notice if the next release hides additional metrics, "+
+			"rather than being surprised when they are permanently removed in the release after that.")
+}
+
+// Apply applies parameters into global configuration of metrics.
+func (o *Options) Apply() {
+	if o != nil && len(o.ShowHiddenMetricsForVersion) > 0 {
+		SetShowHidden()
+	}
+}
+
+func validateShowHiddenMetricsVersion(currentVersion semver.Version, targetVersionStr string) error {
+	if targetVersionStr == "" {
+		return nil
+	}
+
+	validVersionStr := fmt.Sprintf("%d.%d", currentVersion.Major, currentVersion.Minor-1)
+	if targetVersionStr != validVersionStr {
+		return fmt.Errorf("--show-hidden-metrics-for-version must be omitted or have the value '%v'. Only the previous minor version is allowed", validVersionStr)
+	}
+
+	return nil
+}

staging/src/k8s.io/component-base/metrics/registry.go

@@ -51,19 +51,6 @@ func shouldHide(currentVersion *semver.Version, deprecatedVersion *semver.Versio
 	return false
 }
 
-func validateShowHiddenMetricsVersion(currentVersion semver.Version, targetVersionStr string) error {
-	if targetVersionStr == "" {
-		return nil
-	}
-
-	validVersionStr := fmt.Sprintf("%d.%d", currentVersion.Major, currentVersion.Minor-1)
-	if targetVersionStr != validVersionStr {
-		return fmt.Errorf("--show-hidden-metrics-for-version must be omitted or have the value '%v'. Only the previous minor version is allowed", validVersionStr)
-	}
-
-	return nil
-}
-
 // ValidateShowHiddenMetricsVersion checks invalid version for which show hidden metrics.
 func ValidateShowHiddenMetricsVersion(v string) []error {
 	err := validateShowHiddenMetricsVersion(parseVersion(version.Get()), v)