Fix crashOnFailureFetchingExpectations flag description & race condition

in test

Signed-off-by: David-Jaeyoon-Lee <[email protected]>

pkg/readiness/ready_tracker.go

@@ -45,7 +45,7 @@ import (
  logf "sigs.k8s.io/controller-runtime/pkg/log"
 )
 
-var crashOnFailureFetchingExpectations = flag.Bool("crash-on-failure-fetching-expectations", false, "When set (defaults to false), gatekeeper will ignore errors that occur when gathering expectations. This prevents bootstrapping errors from crashing Gatekeeper at the cost of increasing the risk Gatekeeper will under-enforce policy by serving before it has loaded in all policies. Enabling this will help prevent under-enforcement at the risk of crashing during startup for issues like network errors. Note that enabling this flag currently does not achieve the aforementioned effect since fetching expectations are set to retry until success so failures during fetching expectations currently do not occurr.")
+var crashOnFailureFetchingExpectations = flag.Bool("crash-on-failure-fetching-expectations", false, "Unless set (defaults to false), gatekeeper will ignore errors that occur when gathering expectations. This prevents bootstrapping errors from crashing Gatekeeper at the cost of increasing the risk Gatekeeper will under-enforce policy by serving before it has loaded in all policies. Enabling this will help prevent under-enforcement at the risk of crashing during startup for issues like network errors. Note that enabling this flag currently does not achieve the aforementioned effect since fetching expectations are set to retry until success so failures during fetching expectations currently do not occurr.")
 
 var log = logf.Log.WithName("readiness-tracker")
 
@@ -373,26 +373,26 @@ func (t *Tracker) Run(ctx context.Context) error {
 
  // start deleted object polling. Periodically collects
  // objects that are expected by the Tracker, but are deleted
- go func() error {
+ go func() {
  // wait before proceeding, hoping
  // that the tracker will be satisfied by then
  timer := time.NewTimer(2000 * time.Millisecond)
  select {
  case <-ctx.Done():
- return nil
+ return
  case <-timer.C:
  }
 
  ticker := time.NewTicker(500 * time.Millisecond)
  for {
  select {
  case <-ctx.Done():
- return nil
+ return
  case <-ticker.C:
  if t.Satisfied() {
  log.Info("readiness satisfied, no further collection")
  ticker.Stop()
- return nil
+ return
  }
  t.collectInvalidExpectations(ctx)
  }

pkg/readiness/ready_tracker_unit_test.go

@@ -697,10 +697,8 @@ func Test_ReadyTracker_TrackConfigAndSyncSets(t *testing.T) {
  if rt.config.Populated() != expectPopulated || rt.syncsets.Populated() != expectPopulated {
  t.Fatalf("config & syncset object trackers' populated fields are marked as config: %v & syncset: %v, but both should be %v", rt.config.Populated(), rt.syncsets.Populated(), expectPopulated)
  }
- } else {
- if !rt.config.Populated() || !rt.syncsets.Populated() {
- t.Fatalf("config & syncset object trackers' populated fields are marked as config: %v & syncset: %v, but both should be true", rt.config.Populated(), rt.syncsets.Populated())
- }
+ } else if !rt.config.Populated() || !rt.syncsets.Populated() {
+ t.Fatalf("config & syncset object trackers' populated fields are marked as config: %v & syncset: %v, but both should be true", rt.config.Populated(), rt.syncsets.Populated())
  }
  })
  }
@@ -823,11 +821,16 @@ func Test_ReadyTracker_Run_GRP_Wait(t *testing.T) {
  }
  for _, tc := range tcs {
  t.Run(tc.name, func(t *testing.T) {
+ var m sync.Mutex
  funcs := &interceptor.Funcs{}
  funcs.List = func(ctx context.Context, client client.WithWatch, list client.ObjectList, opts ...client.ListOption) error {
  if _, ok := list.(*v1beta1.ConstraintTemplateList); ok {
  return fmt.Errorf("Force Test ConstraintTemplateList Failure")
  }
+
+ // Adding a mutex lock here avoids the race condition within fake client's List method
+ m.Lock()
+ defer m.Unlock()
  return client.List(ctx, list, opts...)
  }
 
@@ -872,11 +875,14 @@ func Test_ReadyTracker_Run_ConstraintTrackers_Wait(t *testing.T) {
  }
  for _, tc := range tcs {
  t.Run(tc.name, func(t *testing.T) {
+ var m sync.Mutex
  funcs := &interceptor.Funcs{}
  funcs.List = func(ctx context.Context, client client.WithWatch, list client.ObjectList, opts ...client.ListOption) error {
  if v, ok := list.(*unstructured.UnstructuredList); ok && v.GroupVersionKind().Kind == "test-constraint" {
  return fmt.Errorf("Force Test constraint list Failure")
  }
+ m.Lock()
+ defer m.Unlock()
  return client.List(ctx, list, opts...)
  }
 
@@ -921,11 +927,14 @@ func Test_ReadyTracker_Run_DataTrackers_Wait(t *testing.T) {
  }
  for _, tc := range tcs {
  t.Run(tc.name, func(t *testing.T) {
+ var m sync.Mutex
  funcs := &interceptor.Funcs{}
  funcs.List = func(ctx context.Context, client client.WithWatch, list client.ObjectList, opts ...client.ListOption) error {
  if v, ok := list.(*unstructured.UnstructuredList); ok && v.GroupVersionKind().Kind == "PodList" {
  return fmt.Errorf("Force Test pod list Failure")
  }
+ m.Lock()
+ defer m.Unlock()
  return client.List(ctx, list, opts...)
  }