Skip to content

Commit

Permalink
fix flaky test
Browse files Browse the repository at this point in the history
AFAICT, the root cause is an internal controller race in config_c
that triggers an additional reconcile request to be sent down the
previous wrapped channel which will block as it's not being read.

Signed-off-by: Alex Pana <[email protected]>
  • Loading branch information
acpana committed Aug 29, 2023
1 parent 0637f6e commit 6ca3fa5
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 13 deletions.
9 changes: 5 additions & 4 deletions pkg/controller/config/config_controller_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
stdlog "log"
"os"
"path/filepath"
"sync"
"testing"

"github.com/open-policy-agent/gatekeeper/v3/apis"
Expand Down Expand Up @@ -62,12 +63,12 @@ func TestMain(m *testing.M) {

// SetupTestReconcile returns a reconcile.Reconcile implementation that delegates to inner and
// writes the request to requests after Reconcile is finished.
func SetupTestReconcile(inner reconcile.Reconciler) (reconcile.Reconciler, chan reconcile.Request) {
requests := make(chan reconcile.Request)
func SetupTestReconcile(inner reconcile.Reconciler) (reconcile.Reconciler, *sync.Map) {
var requests sync.Map
fn := reconcile.Func(func(ctx context.Context, req reconcile.Request) (reconcile.Result, error) {
result, err := inner.Reconcile(ctx, req)
requests <- req
requests.Store(req, struct{}{})
return result, err
})
return fn, requests
return fn, &requests
}
19 changes: 10 additions & 9 deletions pkg/controller/config/config_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/reconcile"
)

var expectedRequest = reconcile.Request{NamespacedName: types.NamespacedName{
Name: "config",
Namespace: "gatekeeper-system",
}}

const timeout = time.Second * 20

// setupManager sets up a controller-runtime manager with registered watch manager.
Expand Down Expand Up @@ -123,9 +118,6 @@ func TestReconcile(t *testing.T) {
},
},
}

// Set up the Manager and Controller. Wrap the Controller Reconcile function so it writes each request to a
// channel when it is finished.
mgr, wm := setupManager(t)
c := testclient.NewRetryClient(mgr.GetClient())

Expand Down Expand Up @@ -162,6 +154,7 @@ func TestReconcile(t *testing.T) {
rec, err := newReconciler(mgr, cacheManager, cs, tracker)
require.NoError(t, err)

// Wrap the Controller Reconcile function so it writes each request to a map when it is finished reconciling.
recFn, requests := SetupTestReconcile(rec)
require.NoError(t, add(mgr, recFn))

Expand All @@ -180,7 +173,15 @@ func TestReconcile(t *testing.T) {
t.Fatal(err)
}
}()
g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedRequest)))
g.Eventually(func() bool {
expectedReq := reconcile.Request{NamespacedName: types.NamespacedName{
Name: "config",
Namespace: "gatekeeper-system",
}}
_, ok := requests.Load(expectedReq)

return ok
}).WithTimeout(timeout).Should(gomega.BeTrue())

g.Eventually(func() int {
return len(wm.GetManagedGVK())
Expand Down

0 comments on commit 6ca3fa5

Please sign in to comment.