From 24321b2d4ef57929f287dd467a4c48a492b8157f Mon Sep 17 00:00:00 2001 From: Marek Siarkowicz Date: Thu, 14 Nov 2019 09:19:43 +0100 Subject: [PATCH] Refactor show-hidden-metric-for-version flag --- cmd/kube-apiserver/app/BUILD | 1 - cmd/kube-apiserver/app/options/BUILD | 1 + cmd/kube-apiserver/app/options/options.go | 13 +-- .../app/options/options_test.go | 2 + cmd/kube-apiserver/app/options/validation.go | 3 +- cmd/kube-apiserver/app/server.go | 5 +- cmd/kube-controller-manager/app/options/BUILD | 1 + .../app/options/options.go | 18 ++--- .../app/options/options_test.go | 2 + cmd/kube-scheduler/app/options/options.go | 21 ++--- .../src/k8s.io/component-base/metrics/BUILD | 2 + .../k8s.io/component-base/metrics/options.go | 79 +++++++++++++++++++ .../k8s.io/component-base/metrics/registry.go | 13 --- 13 files changed, 103 insertions(+), 58 deletions(-) create mode 100644 staging/src/k8s.io/component-base/metrics/options.go diff --git a/cmd/kube-apiserver/app/BUILD b/cmd/kube-apiserver/app/BUILD index f7b7d4fdea1..d9fc684d000 100644 --- a/cmd/kube-apiserver/app/BUILD +++ b/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", diff --git a/cmd/kube-apiserver/app/options/BUILD b/cmd/kube-apiserver/app/options/BUILD index b833a85aeba..0d502a16f43 100644 --- a/cmd/kube-apiserver/app/options/BUILD +++ b/cmd/kube-apiserver/app/options/BUILD @@ -65,6 +65,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", diff --git a/cmd/kube-apiserver/app/options/options.go b/cmd/kube-apiserver/app/options/options.go index e64604758db..a7fa94d9402 100644 --- a/cmd/kube-apiserver/app/options/options.go +++ b/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")) - - // 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 ., 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.") + s.Metrics.AddFlags(fss.FlagSet("metrics")) // Note: the weird ""+ in below lines seems to be the only way to get gofmt to // arrange these text blocks sensibly. Grrr. diff --git a/cmd/kube-apiserver/app/options/options_test.go b/cmd/kube-apiserver/app/options/options_test.go index 4984adfe96d..ad4a11a4cbc 100644 --- a/cmd/kube-apiserver/app/options/options_test.go +++ b/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) { diff --git a/cmd/kube-apiserver/app/options/validation.go b/cmd/kube-apiserver/app/options/validation.go index 4f94e438d00..d1287f73898 100644 --- a/cmd/kube-apiserver/app/options/validation.go +++ b/cmd/kube-apiserver/app/options/validation.go @@ -24,7 +24,6 @@ import ( apiextensionsapiserver "k8s.io/apiextensions-apiserver/pkg/apiserver" 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" @@ -144,7 +143,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 } diff --git a/cmd/kube-apiserver/app/server.go b/cmd/kube-apiserver/app/server.go index 2ad5309f22d..cf4ec4e481f 100644 --- a/cmd/kube-apiserver/app/server.go +++ b/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" @@ -303,9 +302,7 @@ func CreateKubeAPIServerConfig( PerConnectionBandwidthLimitBytesPerSec: s.MaxConnectionBytesPerSec, }) - if len(s.ShowHiddenMetricsForVersion) > 0 { - metrics.SetShowHidden() - } + s.Metrics.Apply() serviceIPRange, apiServerServiceIP, err := master.ServiceIPRange(s.PrimaryServiceClusterIPRange) if err != nil { diff --git a/cmd/kube-controller-manager/app/options/BUILD b/cmd/kube-controller-manager/app/options/BUILD index d9ba251cd78..7d0a03b042d 100644 --- a/cmd/kube-controller-manager/app/options/BUILD +++ b/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", ], ) diff --git a/cmd/kube-controller-manager/app/options/options.go b/cmd/kube-controller-manager/app/options/options.go index 4da25d2eb84..8f419086540 100644 --- a/cmd/kube-controller-manager/app/options/options.go +++ b/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 ., 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 } - - if len(s.ShowHiddenMetricsForVersion) > 0 { - metrics.SetShowHidden() - } + s.Metrics.Apply() return c, nil } diff --git a/cmd/kube-controller-manager/app/options/options_test.go b/cmd/kube-controller-manager/app/options/options_test.go index dc43ee87e65..3cf81f70d2c 100644 --- a/cmd/kube-controller-manager/app/options/options_test.go +++ b/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 diff --git a/cmd/kube-scheduler/app/options/options.go b/cmd/kube-scheduler/app/options/options.go index 46c1821d0f3..4a58600f787 100644 --- a/cmd/kube-scheduler/app/options/options.go +++ b/cmd/kube-scheduler/app/options/options.go @@ -62,6 +62,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. @@ -71,8 +72,6 @@ type Options struct { WriteConfigTo string Master string - - ShowHiddenMetricsForVersion string } // NewOptions returns default scheduler app options. @@ -108,6 +107,7 @@ func NewOptions() (*Options, error) { SchedulerName: corev1.DefaultSchedulerName, HardPodAffinitySymmetricWeight: interpodaffinity.DefaultHardPodAffinityWeight, }, + Metrics: metrics.NewOptions(), } o.Authentication.TolerateInClusterLookupFailure = true @@ -162,15 +162,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")) - - // 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 ., 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.") + o.Metrics.AddFlags(nfs.FlagSet("metrics")) return nfs } @@ -217,10 +209,7 @@ func (o *Options) ApplyTo(c *schedulerappconfig.Config) error { return err } } - if len(o.ShowHiddenMetricsForVersion) > 0 { - metrics.SetShowHidden() - } - + o.Metrics.Apply() return nil } @@ -236,7 +225,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 } diff --git a/staging/src/k8s.io/component-base/metrics/BUILD b/staging/src/k8s.io/component-base/metrics/BUILD index 4da26f4d462..fe30e65fa8b 100644 --- a/staging/src/k8s.io/component-base/metrics/BUILD +++ b/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", ], ) diff --git a/staging/src/k8s.io/component-base/metrics/options.go b/staging/src/k8s.io/component-base/metrics/options.go new file mode 100644 index 00000000000..c15dd9b4d20 --- /dev/null +++ b/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 ., 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 +} diff --git a/staging/src/k8s.io/component-base/metrics/registry.go b/staging/src/k8s.io/component-base/metrics/registry.go index 65dd3a60d32..7ae3223e787 100644 --- a/staging/src/k8s.io/component-base/metrics/registry.go +++ b/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)