Skip to content

Commit

Permalink
Including feature flag locking.
Browse files Browse the repository at this point in the history
Required to allow tests to pass with race checking enabled.
  • Loading branch information
markmandel committed Feb 13, 2020
1 parent 2122ff1 commit c3eacb6
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -174,4 +174,6 @@ properties:
type: string
url:
type: string
caBundle:
type: string
{{- end }}
12 changes: 9 additions & 3 deletions install/yaml/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -420,6 +420,8 @@ spec:
type: string
url:
type: string
caBundle:
type: string
subresources:
# status enables the status subresource.
status: {}
Expand Down Expand Up @@ -714,7 +716,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
Expand All @@ -730,6 +732,8 @@ spec:
type: string
url:
type: string
caBundle:
type: string

---
# Source: agones/templates/crds/gameserverallocationpolicy.yaml
Expand Down Expand Up @@ -1035,7 +1039,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
Expand All @@ -1051,6 +1055,8 @@ spec:
type: string
url:
type: string
caBundle:
type: string
subresources:
# status enables the status subresource.
status: {}
Expand Down
7 changes: 3 additions & 4 deletions pkg/apis/agones/v1/gameserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"`
}
Expand Down
11 changes: 4 additions & 7 deletions pkg/apis/agones/v1/gameserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()
Expand Down
17 changes: 17 additions & 0 deletions pkg/util/runtime/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package runtime
import (
"net/url"
"strconv"
"sync"

"github.com/pkg/errors"
"github.com/spf13/pflag"
Expand Down Expand Up @@ -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.
Expand All @@ -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 {
Expand Down Expand Up @@ -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))
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/util/runtime/features_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit c3eacb6

Please sign in to comment.