Including feature flag locking.

Required to allow tests to pass with race checking enabled.
googleforgames · Feb 13, 2020 · 559230a · 559230a
1 parent 4b4b9f2
commit 559230a
Show file tree

Hide file tree

Showing 7 changed files with 43 additions and 19 deletions.
diff --git a/install/helm/agones/templates/crds/_gameserverspecvalidation.yaml b/install/helm/agones/templates/crds/_gameserverspecvalidation.yaml
@@ -158,7 +158,7 @@ properties:
             properties:
               initialCapacity:
                 type: integer
-                title: The intial player capacity that this Game Server has
+                title: The initial player capacity of this Game Server
                 minimum: 0
               webhook:
                 type: object
@@ -174,4 +174,6 @@ properties:
                         type: string
                   url:
                     type: string
+                  caBundle:
+                    type: string
 {{- end }}
diff --git a/install/yaml/install.yaml b/install/yaml/install.yaml
@@ -404,7 +404,7 @@ spec:
                           properties:
                             initialCapacity:
                               type: integer
-                              title: The intial player capacity that this Game Server has
+                              title: The initial player capacity of this Game Server
                               minimum: 0
                             webhook:
                               type: object
@@ -420,6 +420,8 @@ spec:
                                       type: string
                                 url:
                                   type: string
+                                caBundle:
+                                  type: string
   subresources:
     # status enables the status subresource.
     status: {}
@@ -716,7 +718,7 @@ spec:
                   properties:
                     initialCapacity:
                       type: integer
-                      title: The intial player capacity that this Game Server has
+                      title: The initial player capacity of this Game Server
                       minimum: 0
                     webhook:
                       type: object
@@ -732,6 +734,8 @@ spec:
                               type: string
                         url:
                           type: string
+                        caBundle:
+                          type: string
 
 ---
 # Source: agones/templates/crds/gameserverallocationpolicy.yaml
@@ -1044,7 +1048,7 @@ spec:
                           properties:
                             initialCapacity:
                               type: integer
-                              title: The intial player capacity that this Game Server has
+                              title: The initial player capacity of this Game Server
                               minimum: 0
                             webhook:
                               type: object
@@ -1060,6 +1064,8 @@ spec:
                                       type: string
                                 url:
                                   type: string
+                                caBundle:
+                                  type: string
   subresources:
     # status enables the status subresource.
     status: {}

diff --git a/pkg/apis/agones/v1/gameserver.go b/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"
@@ -152,11 +151,11 @@ type GameServerSpec struct {
 	SdkServer SdkServer `json:"sdkServer,omitempty"`
 	// Template describes the Pod that will be created for the GameServer
 	Template corev1.PodTemplateSpec `json:"template"`
-	// AlphaSpec describes the alpha properties for the GameServer
+	// Alpha describes the alpha properties for the GameServer.
 	Alpha AlphaSpec `json:"alpha,omitempty"`
 }
 
-// AlphaSpec is the alpha properties of the GameServer
+// AlphaSpec contains the alpha properties of the GameServer.
 type AlphaSpec struct {
 	Players PlayersSpec `json:"players"`
 }

diff --git a/pkg/apis/agones/v1/gameserver_test.go b/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,12 @@ func TestGameServerApplyDefaults(t *testing.T) {
 		},
 	}
 
-	// otherwise the race condition detector is not happy.
-	mtx := sync.Mutex{}
+	runtime.FeatureTestMutex.Lock()
+	defer runtime.FeatureTestMutex.Unlock()
+
 	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()

diff --git a/pkg/util/runtime/features.go b/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))
 	}

diff --git a/pkg/util/runtime/features_test.go b/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,

diff --git a/site/content/en/docs/Reference/agones_crd_api_reference.html b/site/content/en/docs/Reference/agones_crd_api_reference.html
@@ -2544,7 +2544,7 @@ <h3 id="GameServer">GameServer
 </em>
 </td>
 <td>
-<p>AlphaSpec describes the alpha properties for the GameServer</p>
+<p>Alpha describes the alpha properties for the GameServer.</p>
 </td>
 </tr>
 </table>
@@ -2682,7 +2682,7 @@ <h3 id="AlphaSpec">AlphaSpec
 <a href="#GameServerSpec">GameServerSpec</a>)
 </p>
 <p>
-<p>AlphaSpec is the alpha properties of the GameServer</p>
+<p>AlphaSpec contains the alpha properties of the GameServer.</p>
 </p>
 <table>
 <thead>
@@ -3183,7 +3183,7 @@ <h3 id="GameServerSpec">GameServerSpec
 </em>
 </td>
 <td>
-<p>AlphaSpec describes the alpha properties for the GameServer</p>
+<p>Alpha describes the alpha properties for the GameServer.</p>
 </td>
 </tr>
 </tbody>
@@ -3456,7 +3456,7 @@ <h3 id="GameServerTemplateSpec">GameServerTemplateSpec
 </em>
 </td>
 <td>
-<p>AlphaSpec describes the alpha properties for the GameServer</p>
+<p>Alpha describes the alpha properties for the GameServer.</p>
 </td>
 </tr>
 </table>