From 6d839235c2e6bf47c7f0f7c354572e59e025fb96 Mon Sep 17 00:00:00 2001 From: Han Kang Date: Wed, 8 May 2019 10:54:20 -0700 Subject: [PATCH] handle global registry version loading more than once (with different versions) --- .../src/k8s.io/component-base/metrics/counter_test.go | 4 ++-- .../component-base/metrics/legacyregistry/registry.go | 10 ++++++++-- .../metrics/legacyregistry/registry_test.go | 6 +++--- staging/src/k8s.io/component-base/metrics/registry.go | 4 ++-- .../src/k8s.io/component-base/metrics/registry_test.go | 4 ++-- 5 files changed, 17 insertions(+), 11 deletions(-) diff --git a/staging/src/k8s.io/component-base/metrics/counter_test.go b/staging/src/k8s.io/component-base/metrics/counter_test.go index 72ddf762a75..15eaf3db6b1 100644 --- a/staging/src/k8s.io/component-base/metrics/counter_test.go +++ b/staging/src/k8s.io/component-base/metrics/counter_test.go @@ -74,7 +74,7 @@ func TestCounter(t *testing.T) { for _, test := range tests { t.Run(test.desc, func(t *testing.T) { - registry := NewKubeRegistry(&apimachineryversion.Info{ + registry := NewKubeRegistry(apimachineryversion.Info{ Major: "1", Minor: "15", GitVersion: "v1.15.0-alpha-1.12345", @@ -175,7 +175,7 @@ func TestCounterVec(t *testing.T) { for _, test := range tests { t.Run(test.desc, func(t *testing.T) { - registry := NewKubeRegistry(&apimachineryversion.Info{ + registry := NewKubeRegistry(apimachineryversion.Info{ Major: "1", Minor: "15", GitVersion: "v1.15.0-alpha-1.12345", diff --git a/staging/src/k8s.io/component-base/metrics/legacyregistry/registry.go b/staging/src/k8s.io/component-base/metrics/legacyregistry/registry.go index 8a941e7dc3c..2653cf54f32 100644 --- a/staging/src/k8s.io/component-base/metrics/legacyregistry/registry.go +++ b/staging/src/k8s.io/component-base/metrics/legacyregistry/registry.go @@ -17,6 +17,7 @@ limitations under the License. package legacyregistry import ( + "fmt" apimachineryversion "k8s.io/apimachinery/pkg/version" "k8s.io/component-base/metrics" "sync" @@ -38,15 +39,20 @@ type metricsRegistryFactory struct { // SetRegistryFactoryVersion sets the kubernetes version information for all // subsequent metrics registry initializations. Only the first call has an effect. // If a version is not set, then metrics registry creation will no-opt -func SetRegistryFactoryVersion(ver *apimachineryversion.Info) []error { +func SetRegistryFactoryVersion(ver apimachineryversion.Info) []error { globalRegistryFactory.registrationLock.Lock() defer globalRegistryFactory.registrationLock.Unlock() if globalRegistryFactory.kubeVersion != nil { + if globalRegistryFactory.kubeVersion.String() != ver.String() { + panic(fmt.Sprintf("Cannot load a global registry more than once, had %s tried to load %s", + globalRegistryFactory.kubeVersion.String(), + ver.String())) + } return nil } registrationErrs := make([]error, 0) globalRegistryFactory.globalRegistry = metrics.NewKubeRegistry(ver) - globalRegistryFactory.kubeVersion = ver + globalRegistryFactory.kubeVersion = &ver for _, c := range globalRegistryFactory.registerQueue { err := globalRegistryFactory.globalRegistry.Register(c) if err != nil { diff --git a/staging/src/k8s.io/component-base/metrics/legacyregistry/registry_test.go b/staging/src/k8s.io/component-base/metrics/legacyregistry/registry_test.go index a448bf3332d..fdb4b13c50a 100644 --- a/staging/src/k8s.io/component-base/metrics/legacyregistry/registry_test.go +++ b/staging/src/k8s.io/component-base/metrics/legacyregistry/registry_test.go @@ -27,7 +27,7 @@ import ( ) func init() { - SetRegistryFactoryVersion(&apimachineryversion.Info{ + SetRegistryFactoryVersion(apimachineryversion.Info{ Major: "1", Minor: "15", GitVersion: "v1.15.0-alpha-1.12345", @@ -161,7 +161,7 @@ func TestDeferredRegister(t *testing.T) { t.Errorf("Got err == %v, expected no error", err) } // set the global registry version - errs := SetRegistryFactoryVersion(&apimachineryversion.Info{ + errs := SetRegistryFactoryVersion(apimachineryversion.Info{ Major: "1", Minor: "15", GitVersion: "v1.15.0-alpha-1.12345", @@ -185,7 +185,7 @@ func TestDeferredMustRegister(t *testing.T) { MustRegister(alphaDeprecatedCounter) assert.Panics(t, func() { - SetRegistryFactoryVersion(&apimachineryversion.Info{ + SetRegistryFactoryVersion(apimachineryversion.Info{ Major: "1", Minor: "15", GitVersion: "v1.15.0-alpha-1.12345", diff --git a/staging/src/k8s.io/component-base/metrics/registry.go b/staging/src/k8s.io/component-base/metrics/registry.go index a91dd25a982..1471cd7a8da 100644 --- a/staging/src/k8s.io/component-base/metrics/registry.go +++ b/staging/src/k8s.io/component-base/metrics/registry.go @@ -88,9 +88,9 @@ func (kr *kubeRegistry) Gather() ([]*dto.MetricFamily, error) { // NewKubeRegistry creates a new vanilla Registry without any Collectors // pre-registered. -func NewKubeRegistry(v *apimachineryversion.Info) KubeRegistry { +func NewKubeRegistry(v apimachineryversion.Info) KubeRegistry { return &kubeRegistry{ PromRegistry: prometheus.NewRegistry(), - version: parseVersion(*v), + version: parseVersion(v), } } diff --git a/staging/src/k8s.io/component-base/metrics/registry_test.go b/staging/src/k8s.io/component-base/metrics/registry_test.go index 4c8e071e25d..30ad55b528f 100644 --- a/staging/src/k8s.io/component-base/metrics/registry_test.go +++ b/staging/src/k8s.io/component-base/metrics/registry_test.go @@ -108,7 +108,7 @@ func TestRegister(t *testing.T) { for _, test := range tests { t.Run(test.desc, func(t *testing.T) { - registry := NewKubeRegistry(&apimachineryversion.Info{ + registry := NewKubeRegistry(apimachineryversion.Info{ Major: "1", Minor: "15", GitVersion: "v1.15.0-alpha-1.12345", @@ -179,7 +179,7 @@ func TestMustRegister(t *testing.T) { for _, test := range tests { t.Run(test.desc, func(t *testing.T) { - registry := NewKubeRegistry(&apimachineryversion.Info{ + registry := NewKubeRegistry(apimachineryversion.Info{ Major: "1", Minor: "15", GitVersion: "v1.15.0-alpha-1.12345",