From 04bbd3481f70825eea54b4b154a04d2496dcf652 Mon Sep 17 00:00:00 2001 From: Lukasz Szaszkiewicz Date: Wed, 10 Jan 2024 17:15:01 +0100 Subject: [PATCH] client-go/features: warn when ordering initialization issue ReplaceFeatureGates logs a warning when the default env var implementation has been already used. Such a situation indicates a potential ordering issue and usually is unwanted. --- .../src/k8s.io/client-go/features/envvar.go | 9 +++++++++ .../k8s.io/client-go/features/envvar_test.go | 8 ++++++++ .../src/k8s.io/client-go/features/features.go | 18 ++++++++++++++++++ 3 files changed, 35 insertions(+) diff --git a/staging/src/k8s.io/client-go/features/envvar.go b/staging/src/k8s.io/client-go/features/envvar.go index 6761e16c1d7..f9edfdf0d91 100644 --- a/staging/src/k8s.io/client-go/features/envvar.go +++ b/staging/src/k8s.io/client-go/features/envvar.go @@ -77,6 +77,10 @@ type envVarFeatureGates struct { // enabled holds a map[Feature]bool // with values explicitly set via env var enabled atomic.Value + + // readEnvVars holds the boolean value which + // indicates whether readEnvVarsOnce has been called. + readEnvVars atomic.Bool } // Enabled returns true if the key is enabled. If the key is not known, this call will panic. @@ -116,6 +120,7 @@ func (f *envVarFeatureGates) getEnabledMapFromEnvVar() map[Feature]bool { } } f.enabled.Store(featureGatesState) + f.readEnvVars.Store(true) for feature, featureSpec := range f.known { if featureState, ok := featureGatesState[feature]; ok { @@ -127,3 +132,7 @@ func (f *envVarFeatureGates) getEnabledMapFromEnvVar() map[Feature]bool { }) return f.enabled.Load().(map[Feature]bool) } + +func (f *envVarFeatureGates) hasAlreadyReadEnvVar() bool { + return f.readEnvVars.Load() +} diff --git a/staging/src/k8s.io/client-go/features/envvar_test.go b/staging/src/k8s.io/client-go/features/envvar_test.go index 8406a447455..247c7cb799d 100644 --- a/staging/src/k8s.io/client-go/features/envvar_test.go +++ b/staging/src/k8s.io/client-go/features/envvar_test.go @@ -146,3 +146,11 @@ func TestEnvVarFeatureGatesEnabledPanic(t *testing.T) { target := newEnvVarFeatureGates(nil) require.PanicsWithError(t, fmt.Errorf("feature %q is not registered in FeatureGates %q", "UnknownFeature", target.callSiteName).Error(), func() { target.Enabled("UnknownFeature") }) } + +func TestHasAlreadyReadEnvVar(t *testing.T) { + target := newEnvVarFeatureGates(nil) + require.False(t, target.hasAlreadyReadEnvVar()) + + _ = target.getEnabledMapFromEnvVar() + require.True(t, target.hasAlreadyReadEnvVar()) +} diff --git a/staging/src/k8s.io/client-go/features/features.go b/staging/src/k8s.io/client-go/features/features.go index 8226905f99c..7b9d050ef57 100644 --- a/staging/src/k8s.io/client-go/features/features.go +++ b/staging/src/k8s.io/client-go/features/features.go @@ -17,6 +17,9 @@ limitations under the License. package features import ( + "errors" + + utilruntime "k8s.io/apimachinery/pkg/util/runtime" "sync/atomic" ) @@ -99,8 +102,23 @@ func AddFeaturesToExistingFeatureGates(registry Registry) error { // // then replace client-go's feature gates implementation with your implementation // clientgofeaturegate.ReplaceFeatureGates(utilfeature.DefaultMutableFeatureGate) func ReplaceFeatureGates(newFeatureGates Gates) { + if replaceFeatureGatesWithWarningIndicator(newFeatureGates) { + utilruntime.HandleError(errors.New("the default feature gates implementation has already been used and now it's being overwritten. This might lead to unexpected behaviour. Check your initialization order")) + } +} + +func replaceFeatureGatesWithWarningIndicator(newFeatureGates Gates) bool { + shouldProduceWarning := false + + if defaultFeatureGates, ok := FeatureGates().(*envVarFeatureGates); ok { + if defaultFeatureGates.hasAlreadyReadEnvVar() { + shouldProduceWarning = true + } + } wrappedFeatureGates := &featureGatesWrapper{newFeatureGates} featureGates.Store(wrappedFeatureGates) + + return shouldProduceWarning } func init() {