Skip to content

Commit

Permalink
Resolve data race in setup syncer test (#6282)
Browse files Browse the repository at this point in the history
  • Loading branch information
sam-heilbron committed Apr 11, 2022
1 parent 8394f8d commit 02f635c
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 9 deletions.
5 changes: 5 additions & 0 deletions changelog/v1.12.0-beta3/setup-syncer-data-race.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
changelog:
- type: NON_USER_FACING
issueLink: https://github.com/solo-io/gloo/issues/5860
resolvesIssue: false
description: Fix a data race that occurs during our setup syncer tests
41 changes: 32 additions & 9 deletions projects/gloo/pkg/syncer/setup/setup_syncer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ import (
"sync"
"time"

"github.com/solo-io/gloo/pkg/utils/setuputils"


"github.com/solo-io/gloo/projects/gloo/pkg/defaults"
"github.com/solo-io/solo-kit/pkg/utils/statusutils"

Expand Down Expand Up @@ -38,10 +41,11 @@ import (
var _ = Describe("SetupSyncer", func() {

var (
settings *v1.Settings
ctx context.Context
cancel context.CancelFunc
memcache memory.InMemoryResourceCache
settings *v1.Settings
ctx context.Context
cancel context.CancelFunc
memcache memory.InMemoryResourceCache
setupLock sync.RWMutex
)

newContext := func() {
Expand All @@ -52,6 +56,25 @@ var _ = Describe("SetupSyncer", func() {
ctx = settingsutil.WithSettings(ctx, settings)
}

// SetupFunc is used to configure Gloo with appropriate configuration
// It is assumed to run once at construction time, and therefore it executes directives that
// are also assumed to only run at construction time.
// One of those, is the construction of schemes: https://github.com/kubernetes/kubernetes/pull/89019#issuecomment-600278461
// In our tests we do not follow this pattern, and to avoid data races (that cause test failures)
// we ensure that only 1 SetupFunc is ever called at a time
newSynchronizedSetupFunc := func() setuputils.SetupFunc {
setupFunc := NewSetupFunc()

var synchronizedSetupFunc setuputils.SetupFunc
synchronizedSetupFunc = func(ctx context.Context, kubeCache kube.SharedCache, inMemoryCache memory.InMemoryResourceCache, settings *v1.Settings) error {
setupLock.Lock()
defer setupLock.Unlock()
return setupFunc(ctx, kubeCache, inMemoryCache, settings)
}

return synchronizedSetupFunc
}

BeforeEach(func() {
settings = &v1.Settings{
RefreshRate: prototime.DurationToProto(time.Hour),
Expand Down Expand Up @@ -99,7 +122,7 @@ var _ = Describe("SetupSyncer", func() {
Context("XDS tests", func() {

It("setup can be called twice", func() {
setup := NewSetupFunc()
setup := newSynchronizedSetupFunc()

err := setup(ctx, nil, memcache, settings)
Expect(err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -242,27 +265,27 @@ var _ = Describe("SetupSyncer", func() {
})

It("can be called with core cache", func() {
setup := NewSetupFunc()
setup := newSynchronizedSetupFunc()
err := setup(ctx, kubeCoreCache, memcache, settings)
Expect(err).NotTo(HaveOccurred())
})

It("can be called with core cache warming endpoints", func() {
settings.Gloo.EndpointsWarmingTimeout = prototime.DurationToProto(time.Minute)
setup := NewSetupFunc()
setup := newSynchronizedSetupFunc()
err := setup(ctx, kubeCoreCache, memcache, settings)
Expect(err).NotTo(HaveOccurred())
})

It("panics when endpoints don't arrive in a timely manner", func() {
settings.Gloo.EndpointsWarmingTimeout = prototime.DurationToProto(1 * time.Nanosecond)
setup := NewSetupFunc()
setup := newSynchronizedSetupFunc()
Expect(func() { setup(ctx, kubeCoreCache, memcache, settings) }).To(Panic())
})

It("doesn't panic when endpoints don't arrive in a timely manner if set to zero", func() {
settings.Gloo.EndpointsWarmingTimeout = prototime.DurationToProto(0)
setup := NewSetupFunc()
setup := newSynchronizedSetupFunc()
Expect(func() { setup(ctx, kubeCoreCache, memcache, settings) }).NotTo(Panic())
})

Expand Down

0 comments on commit 02f635c

Please sign in to comment.