Skip to content

Commit

Permalink
Review feedback
Browse files Browse the repository at this point in the history
njuettner suggested using a var instead of boolean literals for the
startInformer arg to NewCRDSource; good idea.
  • Loading branch information
ericrrath committed Feb 4, 2022
1 parent 585d752 commit 56a8d60
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 2 deletions.
7 changes: 6 additions & 1 deletion source/crd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,12 @@ func testCRDSourceEndpoints(t *testing.T) {
labelSelector, err := labels.Parse(ti.labelFilter)
require.NoError(t, err)

cs, err := NewCRDSource(restClient, ti.namespace, ti.kind, ti.annotationFilter, labelSelector, scheme, false)
// At present, client-go's fake.RESTClient (used by crd_test.go) is known to cause race conditions when used
// with informers: https://github.com/kubernetes/kubernetes/issues/95372
// So don't start the informer during testing.
startInformer := false

cs, err := NewCRDSource(restClient, ti.namespace, ti.kind, ti.annotationFilter, labelSelector, scheme, startInformer)
require.NoError(t, err)

receivedEndpoints, err := cs.Endpoints(context.Background())
Expand Down
3 changes: 2 additions & 1 deletion source/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,8 @@ func BuildWithConfig(ctx context.Context, source string, p ClientGenerator, cfg
if err != nil {
return nil, err
}
return NewCRDSource(crdClient, cfg.Namespace, cfg.CRDSourceKind, cfg.AnnotationFilter, cfg.LabelFilter, scheme, true)
startInformer := true
return NewCRDSource(crdClient, cfg.Namespace, cfg.CRDSourceKind, cfg.AnnotationFilter, cfg.LabelFilter, scheme, startInformer)
case "skipper-routegroup":
apiServerURL := cfg.APIServerURL
tokenPath := ""
Expand Down

0 comments on commit 56a8d60

Please sign in to comment.