Including feature flag locking.

Required to allow tests to pass with race checking enabled.

pkg/apis/agones/v1/gameserver.go

@@ -19,11 +19,10 @@ import (
 	"fmt"
 	"net"
 
-	"agones.dev/agones/pkg/util/runtime"
-
 	"agones.dev/agones/pkg"
 	"agones.dev/agones/pkg/apis"
 	"agones.dev/agones/pkg/apis/agones"
+	"agones.dev/agones/pkg/util/runtime"
 	"github.com/mattbaird/jsonpatch"
 	"github.com/pkg/errors"
 	admregv1b "k8s.io/api/admissionregistration/v1beta1"

pkg/apis/agones/v1/gameserver_test.go

@@ -17,14 +17,12 @@ package v1
 import (
 	"fmt"
 	"strings"
-	"sync"
 	"testing"
 
-	"agones.dev/agones/pkg/util/runtime"
-
 	"agones.dev/agones/pkg"
 	"agones.dev/agones/pkg/apis"
 	"agones.dev/agones/pkg/apis/agones"
+	"agones.dev/agones/pkg/util/runtime"
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -328,13 +326,13 @@ func TestGameServerApplyDefaults(t *testing.T) {
 		},
 	}
 
+	runtime.FeatureTestMutex.Lock()
+	defer runtime.FeatureTestMutex.Unlock()
+
 	// otherwise the race condition detector is not happy.
-	mtx := sync.Mutex{}
 	for name, test := range data {
 		t.Run(name, func(t *testing.T) {
-			mtx.Lock()
 			err := runtime.ParseFeatures(test.featureFlags)
-			mtx.Unlock()
 			assert.NoError(t, err)
 
 			test.gameServer.ApplyDefaults()

pkg/util/runtime/features.go

@@ -17,6 +17,7 @@ package runtime
 import (
 	"net/url"
 	"strconv"
+	"sync"
 
 	"github.com/pkg/errors"
 	"github.com/spf13/pflag"
@@ -44,6 +45,14 @@ var (
 	// featureGates is the storage of what features are enabled
 	// or disabled.
 	featureGates map[Feature]bool
+
+	// featureMutex ensures that updates to featureGates don't happen at the same time as reads.
+	// this is mostly to protect tests which can change gates in parallel.
+	featureMutex = sync.RWMutex{}
+
+	// FeatureTestMutex is a mutex to be shared between tests to ensure that a test that involves changing featureGates
+	// cannot accidentally run at the same time as another test that also changing feature flags.
+	FeatureTestMutex sync.Mutex
 )
 
 // Feature is a type for defining feature gates.
@@ -70,6 +79,9 @@ func ParseFeaturesFromEnv() error {
 // ParseFeatures parses the url encoded query string of features and stores the value
 // for later retrieval
 func ParseFeatures(queryString string) error {
+	featureMutex.Lock()
+	defer featureMutex.Unlock()
+
 	features := map[Feature]bool{}
 	// copy the defaults into this map
 	for k, v := range featureDefaults {
@@ -101,12 +113,17 @@ func ParseFeatures(queryString string) error {
 
 // FeatureEnabled returns if a Feature is enabled or not
 func FeatureEnabled(feature Feature) bool {
+	featureMutex.RLock()
+	defer featureMutex.RUnlock()
 	return featureGates[feature]
 }
 
 // EncodeFeatures returns the feature set as a URL encoded query string
 func EncodeFeatures() string {
 	values := url.Values{}
+	featureMutex.RLock()
+	defer featureMutex.RUnlock()
+
 	for k, v := range featureGates {
 		values.Add(string(k), strconv.FormatBool(v))
 	}

pkg/util/runtime/features_test.go

@@ -26,6 +26,9 @@ import (
 
 func TestFeatures(t *testing.T) {
 	t.Parallel()
+	FeatureTestMutex.Lock()
+	defer FeatureTestMutex.Unlock()
+
 	// stable feature flag state
 	featureDefaults = map[Feature]bool{
 		FeatureExample:  true,