Skip to content

Commit 9718f69

Browse files
pleshakovpleshakov
committed
Introduce options
1 parent 0a67e50 commit 9718f69

File tree

3 files changed

+101
-46
lines changed

3 files changed

+101
-46
lines changed

‎internal/manager/controllers.go‎

Lines changed: 60 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"sigs.k8s.io/controller-runtime/pkg/manager"
1111
"sigs.k8s.io/controller-runtime/pkg/predicate"
1212

13+
"github.com/nginxinc/nginx-kubernetes-gateway/internal/manager/index"
1314
"github.com/nginxinc/nginx-kubernetes-gateway/internal/reconciler"
1415
)
1516

@@ -18,51 +19,96 @@ const (
1819
addIndexFieldTimeout = 2 * time.Minute
1920
)
2021

22+
type newReconcilerFunc func(cfg reconciler.Config) *reconciler.Implementation
23+
2124
type controllerConfig struct {
22-
objectType client.Object
23-
namespacedNameFilter reconciler.NamespacedNameFilterFunc // optional
24-
k8sPredicate predicate.Predicate // optional
25-
fieldIndexes map[string]client.IndexerFunc // optional
25+
namespacedNameFilter reconciler.NamespacedNameFilterFunc
26+
k8sPredicate predicate.Predicate
27+
fieldIndices index.FieldIndices
28+
newReconciler newReconcilerFunc
2629
}
2730

28-
// newReconciler creates a new Implementation. Used for unit testing.
29-
var newReconciler = reconciler.NewImplementation
31+
type controllerOption func(*controllerConfig)
32+
33+
func withNamespacedNameFilter(filter reconciler.NamespacedNameFilterFunc) controllerOption {
34+
return func(cfg *controllerConfig) {
35+
cfg.namespacedNameFilter = filter
36+
}
37+
}
38+
39+
func withK8sPredicate(p predicate.Predicate) controllerOption {
40+
return func(cfg *controllerConfig) {
41+
cfg.k8sPredicate = p
42+
}
43+
}
44+
45+
func withFieldIndices(fieldIndices index.FieldIndices) controllerOption {
46+
return func(cfg *controllerConfig) {
47+
cfg.fieldIndices = fieldIndices
48+
}
49+
}
50+
51+
// withNewReconciler allows us to mock reconciler creation in the unit tests.
52+
func withNewReconciler(newReconciler newReconcilerFunc) controllerOption {
53+
return func(cfg *controllerConfig) {
54+
cfg.newReconciler = newReconciler
55+
}
56+
}
57+
58+
func defaultControllerConfig() controllerConfig {
59+
return controllerConfig{
60+
newReconciler: reconciler.NewImplementation,
61+
}
62+
}
3063

3164
func registerController(
3265
ctx context.Context,
66+
objectType client.Object,
3367
mgr manager.Manager,
3468
eventCh chan interface{},
35-
cfg controllerConfig,
69+
options ...controllerOption,
3670
) error {
37-
for field, indexerFunc := range cfg.fieldIndexes {
38-
err := addIndex(ctx, mgr.GetFieldIndexer(), cfg.objectType, field, indexerFunc)
71+
cfg := defaultControllerConfig()
72+
73+
for _, opt := range options {
74+
opt(&cfg)
75+
}
76+
77+
for field, indexerFunc := range cfg.fieldIndices {
78+
err := addIndex(ctx, mgr.GetFieldIndexer(), objectType, field, indexerFunc)
3979
if err != nil {
4080
return err
4181
}
4282
}
4383

44-
builder := ctlr.NewControllerManagedBy(mgr).For(cfg.objectType)
84+
builder := ctlr.NewControllerManagedBy(mgr).For(objectType)
4585

4686
if cfg.k8sPredicate != nil {
4787
builder = builder.WithEventFilter(cfg.k8sPredicate)
4888
}
4989

5090
recCfg := reconciler.Config{
5191
Getter: mgr.GetClient(),
52-
ObjectType: cfg.objectType,
92+
ObjectType: objectType,
5393
EventCh: eventCh,
5494
NamespacedNameFilter: cfg.namespacedNameFilter,
5595
}
5696

57-
err := builder.Complete(newReconciler(recCfg))
97+
err := builder.Complete(cfg.newReconciler(recCfg))
5898
if err != nil {
59-
return fmt.Errorf("cannot build a controller for %T: %w", cfg.objectType, err)
99+
return fmt.Errorf("cannot build a controller for %T: %w", objectType, err)
60100
}
61101

62102
return nil
63103
}
64104

65-
func addIndex(ctx context.Context, indexer client.FieldIndexer, objectType client.Object, field string, indexerFunc client.IndexerFunc) error {
105+
func addIndex(
106+
ctx context.Context,
107+
indexer client.FieldIndexer,
108+
objectType client.Object,
109+
field string,
110+
indexerFunc client.IndexerFunc,
111+
) error {
66112
c, cancel := context.WithTimeout(ctx, addIndexFieldTimeout)
67113
defer cancel()
68114

‎internal/manager/controllers_test.go‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,6 @@ import (
2020
)
2121

2222
func TestRegisterController(t *testing.T) {
23-
// The test will inject a mock newReconciler func. This defer will restore it to the original func.
24-
savedNewReconciler := reconciler.NewImplementation
25-
defer func() {
26-
newReconciler = savedNewReconciler
27-
}()
28-
2923
type fakes struct {
3024
mgr *managerfakes.FakeManager
3125
indexer *managerfakes.FakeFieldIndexer
@@ -83,35 +77,41 @@ func TestRegisterController(t *testing.T) {
8377
},
8478
}
8579

86-
cfg := controllerConfig{
87-
objectType: &v1beta1.HTTPRoute{},
88-
namespacedNameFilter: filter.CreateFilterForGatewayClass("test"),
89-
k8sPredicate: predicate.ServicePortsChangedPredicate{},
90-
fieldIndexes: index.CreateEndpointSliceFieldIndices(),
91-
}
80+
objectType := &v1beta1.HTTPRoute{}
81+
namespacedNameFilter := filter.CreateFilterForGatewayClass("test")
82+
fieldIndexes := index.CreateEndpointSliceFieldIndices()
9283

9384
eventCh := make(chan interface{})
9485

9586
for _, test := range tests {
96-
newReconciler = func(c reconciler.Config) *reconciler.Implementation {
87+
newReconciler := func(c reconciler.Config) *reconciler.Implementation {
9788
if c.Getter != test.fakes.mgr.GetClient() {
9889
t.Errorf("regiterController() created a reconciler config with Getter %p but expected %p for case of %q", c.Getter, test.fakes.mgr.GetClient(), test.msg)
9990
}
100-
if c.ObjectType != cfg.objectType {
101-
t.Errorf("registerController() created a reconciler config with ObjectType %T but expected %T for case of %q", c.ObjectType, cfg.objectType, test.msg)
91+
if c.ObjectType != objectType {
92+
t.Errorf("registerController() created a reconciler config with ObjectType %T but expected %T for case of %q", c.ObjectType, objectType, test.msg)
10293
}
10394
if c.EventCh != eventCh {
10495
t.Errorf("registerController() created a reconciler config with EventCh %v but expected %v for case of %q", c.EventCh, eventCh, test.msg)
10596
}
10697
// comparing functions is not allowed in Go, so we're comparing the pointers
107-
if reflect.ValueOf(c.NamespacedNameFilter).Pointer() != reflect.ValueOf(cfg.namespacedNameFilter).Pointer() {
108-
t.Errorf("registerController() created a reconciler config with NamespacedNameFilter %p but expected %p for case of %q", c.NamespacedNameFilter, cfg.namespacedNameFilter, test.msg)
98+
if reflect.ValueOf(c.NamespacedNameFilter).Pointer() != reflect.ValueOf(namespacedNameFilter).Pointer() {
99+
t.Errorf("registerController() created a reconciler config with NamespacedNameFilter %p but expected %p for case of %q", c.NamespacedNameFilter, namespacedNameFilter, test.msg)
109100
}
110101

111102
return reconciler.NewImplementation(c)
112103
}
113104

114-
err := registerController(context.Background(), test.fakes.mgr, eventCh, cfg)
105+
err := registerController(
106+
context.Background(),
107+
objectType,
108+
test.fakes.mgr,
109+
eventCh,
110+
withNamespacedNameFilter(namespacedNameFilter),
111+
withK8sPredicate(predicate.ServicePortsChangedPredicate{}),
112+
withFieldIndices(fieldIndexes),
113+
withNewReconciler(newReconciler),
114+
)
115115

116116
if !errors.Is(err, test.expectedErr) {
117117
t.Errorf("registerController() returned %q but expected %q for case of %q", err, test.expectedErr, test.msg)
@@ -123,14 +123,14 @@ func TestRegisterController(t *testing.T) {
123123
} else {
124124
_, objType, field, indexFunc := test.fakes.indexer.IndexFieldArgsForCall(0)
125125

126-
if objType != cfg.objectType {
127-
t.Errorf("registerController() called indexer.IndexField() with object type %T but expected %T for case of %q", objType, cfg.objectType, test.msg)
126+
if objType != objectType {
127+
t.Errorf("registerController() called indexer.IndexField() with object type %T but expected %T for case of %q", objType, objectType, test.msg)
128128
}
129129
if field != index.KubernetesServiceNameIndexField {
130130
t.Errorf("registerController() called indexer.IndexField() with field %q but expected %q for case of %q", field, index.KubernetesServiceNameIndexField, test.msg)
131131
}
132132

133-
expectedIndexFunc := cfg.fieldIndexes[index.KubernetesServiceNameIndexField]
133+
expectedIndexFunc := fieldIndexes[index.KubernetesServiceNameIndexField]
134134
// comparing functions is not allowed in Go, so we're comparing the pointers
135135
if reflect.ValueOf(indexFunc).Pointer() != reflect.ValueOf(expectedIndexFunc).Pointer() {
136136
t.Errorf("registerController() called indexer.IndexField() with indexFunc %p but expected %p for case of %q", indexFunc, expectedIndexFunc, test.msg)

‎internal/manager/manager.go‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,15 @@ func Start(cfg config.Config) error {
6363
return fmt.Errorf("cannot build runtime manager: %w", err)
6464
}
6565

66-
controllerConfigs := []controllerConfig{
66+
controllerRegCfgs := []struct {
67+
objectType client.Object
68+
options []controllerOption
69+
}{
6770
{
68-
objectType: &gatewayv1beta1.GatewayClass{},
69-
namespacedNameFilter: filter.CreateFilterForGatewayClass(cfg.GatewayClassName),
71+
objectType: &gatewayv1beta1.GatewayClass{},
72+
options: []controllerOption{
73+
withNamespacedNameFilter(filter.CreateFilterForGatewayClass(cfg.GatewayClassName)),
74+
},
7075
},
7176
{
7277
objectType: &gatewayv1beta1.Gateway{},
@@ -75,25 +80,29 @@ func Start(cfg config.Config) error {
7580
objectType: &gatewayv1beta1.HTTPRoute{},
7681
},
7782
{
78-
objectType: &apiv1.Service{},
79-
k8sPredicate: predicate.ServicePortsChangedPredicate{},
83+
objectType: &apiv1.Service{},
84+
options: []controllerOption{
85+
withK8sPredicate(predicate.ServicePortsChangedPredicate{}),
86+
},
8087
},
8188
{
8289
objectType: &apiv1.Secret{},
8390
},
8491
{
85-
objectType: &discoveryV1.EndpointSlice{},
86-
k8sPredicate: k8spredicate.GenerationChangedPredicate{},
87-
fieldIndexes: index.CreateEndpointSliceFieldIndices(),
92+
objectType: &discoveryV1.EndpointSlice{},
93+
options: []controllerOption{
94+
withK8sPredicate(k8spredicate.GenerationChangedPredicate{}),
95+
withFieldIndices(index.CreateEndpointSliceFieldIndices()),
96+
},
8897
},
8998
}
9099

91100
ctx := ctlr.SetupSignalHandler()
92101

93-
for _, cfg := range controllerConfigs {
94-
err := registerController(ctx, mgr, eventCh, cfg)
102+
for _, regCfg := range controllerRegCfgs {
103+
err := registerController(ctx, regCfg.objectType, mgr, eventCh, regCfg.options...)
95104
if err != nil {
96-
return fmt.Errorf("cannot register controller for %T: %w", cfg.objectType, err)
105+
return fmt.Errorf("cannot register controller for %T: %w", regCfg.objectType, err)
97106
}
98107
}
99108

0 commit comments

Comments
 (0)