Skip to content

Commit

Permalink
Lock testing.Fake object when modifying its ReactionChain
Browse files Browse the repository at this point in the history
The lock is acquired when it accesses the ReactionChain on invocation
but not in PrependReactor and AppendReactor. It probably assumes that
the ReactionChain is only modified on startup but there are cases in our
code base where we setup a reactor after startup and occasionally
encounter a race failure. So explicitly lock the testing.Fake when
setting up a reactor.

Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
  • Loading branch information
tpantelis authored and skitt committed Oct 11, 2023
1 parent 6d040ab commit 18ddf97
Show file tree
Hide file tree
Showing 8 changed files with 34 additions and 16 deletions.
3 changes: 3 additions & 0 deletions pkg/fake/conflict_reactor.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ import (
)

func ConflictOnUpdateReactor(f *testing.Fake, resource string) {
f.Lock()
defer f.Unlock()

reactors := f.ReactionChain[0:]
resourceVersion := "100"
state := sync.Map{}
Expand Down
3 changes: 3 additions & 0 deletions pkg/fake/create_reactor.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ type createReactor struct {

// AddCreateReactor adds a reactor to mimic real K8s create behavior to handle GenerateName, validation et al.
func AddCreateReactor(f *testing.Fake) {
f.Lock()
defer f.Unlock()

r := &createReactor{reactors: f.ReactionChain[0:]}
f.PrependReactor("create", "*", r.react)
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/fake/delete_colection_reactor.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,10 @@ type DeleteCollectionReactor struct {
}

func AddDeleteCollectionReactor(f *testing.Fake) {
f.Lock()
r := &DeleteCollectionReactor{gvrToGVK: map[schema.GroupVersionResource]schema.GroupVersionKind{}, reactors: f.ReactionChain[0:]}
f.PrependReactor("delete-collection", "*", r.react)
f.Unlock()

for gvk := range scheme.Scheme.AllKnownTypes() {
if !strings.HasSuffix(gvk.Kind, "List") {
Expand Down
3 changes: 3 additions & 0 deletions pkg/fake/delete_reactor.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ type deleteReactor struct {

// AddDeleteReactor adds a reactor to mimic real K8s delete behavior to handle ResourceVersion, DeletionTimestamp et al.
func AddDeleteReactor(f *testing.Fake) {
f.Lock()
defer f.Unlock()

r := &deleteReactor{reactors: f.ReactionChain[0:]}
f.PrependReactor("delete", "*", r.react)
}
Expand Down
26 changes: 12 additions & 14 deletions pkg/fake/fail_on_action_reactor.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,22 +43,20 @@ func FailOnAction(f *testing.Fake, resource, verb string, customErr error, autoR
retErr = errors.New("fake error")
}

chain := []testing.Reactor{&testing.SimpleReactor{
Verb: verb,
Resource: resource,
Reaction: func(action testing.Action) (bool, runtime.Object, error) {
if r.fail.Load().(bool) {
if autoReset {
r.fail.Store(false)
}

return true, nil, retErr
f.Lock()
defer f.Unlock()

f.PrependReactor(verb, resource, func(action testing.Action) (bool, runtime.Object, error) {
if r.fail.Load().(bool) {
if autoReset {
r.fail.Store(false)
}

return false, nil, nil
},
}}
f.ReactionChain = append(chain, f.ReactionChain...)
return true, nil, retErr
}

return false, nil, nil
})

return r
}
7 changes: 5 additions & 2 deletions pkg/fake/failing_reactor.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,11 @@ func NewFailingReactor(f *testing.Fake) *FailingReactor {

func NewFailingReactorForResource(f *testing.Fake, resource string) *FailingReactor {
r := &FailingReactor{}
chain := []testing.Reactor{&testing.SimpleReactor{Verb: "*", Resource: resource, Reaction: r.react}}
f.ReactionChain = append(chain, f.ReactionChain...)

f.Lock()
defer f.Unlock()

f.PrependReactor("*", resource, r.react)

return r
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/fake/list_reactor.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ type filteringListReactor struct {
}

func AddFilteringListReactor(f *testing.Fake) {
f.Lock()
defer f.Unlock()

r := &filteringListReactor{reactors: f.ReactionChain[0:]}
f.PrependReactor("list", "*", r.react)
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/fake/update_reactor.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ type updateReactor struct {

// AddUpdateReactor adds a reactor to mimic real K8s update behavior to handle ResourceVersion et al.
func AddUpdateReactor(f *testing.Fake) {
f.Lock()
defer f.Unlock()

r := &updateReactor{reactors: f.ReactionChain[0:]}
f.PrependReactor("update", "*", r.react)
}
Expand Down

0 comments on commit 18ddf97

Please sign in to comment.