Skip to content
This repository has been archived by the owner on Oct 22, 2024. It is now read-only.

Commit

Permalink
test: fix operator event checking
Browse files Browse the repository at this point in the history
The code had two problems.

It stored pointers to the objects passed in by the caller, which is
dangerous because those instances might be reused by the caller, in
which case the content changes. In practice this doesn't seem to
happen, but we can't know that.

It relied on receiving events in a certain order. The apiserver
and (to some extend) the fake client may combine events, which
apparently can also cause them to be delivered out-of-order. This has
happened in practice and broke the
test (#852).

The solution is to de-duplicate after sorting. Because the order
depends on what we compare against, it is done during the actual
comparison together with de-duplication.
  • Loading branch information
pohly committed Jan 18, 2021
1 parent d9ef86d commit 2dd4288
Showing 1 changed file with 15 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package deployment_test
import (
"context"
"fmt"
"sort"
"strings"
"sync"
"testing"
Expand Down Expand Up @@ -226,7 +227,7 @@ type testContext struct {
k8sVersion version.Version

eventsMutex sync.Mutex
events []*corev1.Event
events []corev1.Event
}

func newTestContext(t *testing.T, k8sVersion version.Version, initObjs ...runtime.Object) *testContext {
Expand Down Expand Up @@ -264,15 +265,7 @@ func (tc *testContext) ResetReconciler() {
tc.evWatcher = rc.(*deployment.ReconcileDeployment).EventBroadcaster().StartEventWatcher(func(ev *corev1.Event) {
tc.eventsMutex.Lock()
defer tc.eventsMutex.Unlock()

// Discard consecutive duplicate events, mimicking the EventAggregator behavior
if len(tc.events) != 0 {
lastEvent := tc.events[len(tc.events)-1]
if lastEvent.Reason == ev.Reason && lastEvent.InvolvedObject.UID == ev.InvolvedObject.UID {
return
}
}
tc.events = append(tc.events, ev)
tc.events = append(tc.events, *ev)
})
}

Expand Down Expand Up @@ -314,12 +307,20 @@ func TestDeploymentController(t *testing.T) {
tc.eventsMutex.Lock()
defer tc.eventsMutex.Unlock()

if len(tc.events) < len(expectedEvents) {
return false
}
// Before comparing against expected
// events, we must sort by the "first
// seen", time stamp because events
// may get delivered out-of-order.
sort.Slice(tc.events, func(i, j int) bool {
return tc.events[i].FirstTimestamp.Before(&tc.events[j].FirstTimestamp)
})

// Then we need to filter out events for the
// right deployment and remove duplicates.
events := []string{}
for _, e := range tc.events {
if e.InvolvedObject.UID == dep.GetUID() {
if e.InvolvedObject.UID == dep.GetUID() &&
(len(events) == 0 || events[len(events)-1] != e.Reason) {
events = append(events, e.Reason)
}
}
Expand Down

0 comments on commit 2dd4288

Please sign in to comment.