Skip to content

Commit

Permalink
fix: Remove readiness tracker deadlock caused by duplicate syncs (ope…
Browse files Browse the repository at this point in the history
…n-policy-agent#2970)

Signed-off-by: Max Smythe <[email protected]>
  • Loading branch information
maxsmythe authored and salaxander committed Sep 13, 2023
1 parent 2365a9f commit b22fda4
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 0 deletions.
5 changes: 5 additions & 0 deletions pkg/readiness/object_tracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ func (t *objectTracker) Expect(o runtime.Object) {
return
}

// Satisfied objects cannot be expected again.
if _, ok := t.satisfied[k]; ok {
return
}

// We may have seen it before starting to expect it
if _, ok := t.seen[k]; ok {
delete(t.seen, k)
Expand Down
22 changes: 22 additions & 0 deletions pkg/readiness/object_tracker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,28 @@ func Test_ObjectTracker_Duplicate_Expectations(t *testing.T) {
}
}

// Verify that that satisfied expectations cannot be re-established.
func Test_ObjectTracker_Satisfaction_Final(t *testing.T) {
ot := newObjTracker(schema.GroupVersionKind{}, nil)

const count = 10
ct := makeCTSlice("ct-", count)
for i := 0; i < len(ct); i++ {
ot.Expect(ct[i])
ot.Observe(ct[i])
ot.Expect(ct[i])
}
if ot.Satisfied() {
t.Fatal("should not be satisfied before ExpectationsDone")
}

ot.ExpectationsDone()

if !ot.Satisfied() {
t.Fatal("should be satisfied")
}
}

// Verify that an expectation can be canceled before it's first expected.
func Test_ObjectTracker_CancelBeforeExpect(t *testing.T) {
ot := newObjTracker(schema.GroupVersionKind{}, nil)
Expand Down
4 changes: 4 additions & 0 deletions pkg/readiness/tracker_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ func (t *trackerMap) Get(gvk schema.GroupVersionKind) Expectations {

t.mu.Lock()
defer t.mu.Unlock()
// re-retrieve map entry in case it was added after releasing the read lock.
if e, ok := t.m[gvk]; ok {
return e
}
entry := newObjTracker(gvk, t.fn)
t.m[gvk] = entry
return entry
Expand Down

0 comments on commit b22fda4

Please sign in to comment.