diff --git a/internal/reconciler/reconciler_suite_test.go b/internal/controller/controller_suite_test.go similarity index 88% rename from internal/reconciler/reconciler_suite_test.go rename to internal/controller/controller_suite_test.go index caad3bda4f..f5b8541edd 100644 --- a/internal/reconciler/reconciler_suite_test.go +++ b/internal/controller/controller_suite_test.go @@ -1,4 +1,4 @@ -package reconciler_test +package controller_test import ( "testing" diff --git a/internal/manager/managerfakes/fake_field_indexer.go b/internal/controller/controllerfakes/fake_field_indexer.go similarity index 99% rename from internal/manager/managerfakes/fake_field_indexer.go rename to internal/controller/controllerfakes/fake_field_indexer.go index 1a7c16e6a5..825ae5e02e 100644 --- a/internal/manager/managerfakes/fake_field_indexer.go +++ b/internal/controller/controllerfakes/fake_field_indexer.go @@ -1,5 +1,5 @@ // Code generated by counterfeiter. DO NOT EDIT. -package managerfakes +package controllerfakes import ( "context" diff --git a/internal/reconciler/reconcilerfakes/fake_getter.go b/internal/controller/controllerfakes/fake_getter.go similarity index 96% rename from internal/reconciler/reconcilerfakes/fake_getter.go rename to internal/controller/controllerfakes/fake_getter.go index 041bbd86a5..0a7862a666 100644 --- a/internal/reconciler/reconcilerfakes/fake_getter.go +++ b/internal/controller/controllerfakes/fake_getter.go @@ -1,11 +1,11 @@ // Code generated by counterfeiter. DO NOT EDIT. -package reconcilerfakes +package controllerfakes import ( "context" "sync" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/reconciler" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/controller" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -117,4 +117,4 @@ func (fake *FakeGetter) recordInvocation(key string, args []interface{}) { fake.invocations[key] = append(fake.invocations[key], args) } -var _ reconciler.Getter = new(FakeGetter) +var _ controller.Getter = new(FakeGetter) diff --git a/internal/manager/managerfakes/fake_manager.go b/internal/controller/controllerfakes/fake_manager.go similarity index 99% rename from internal/manager/managerfakes/fake_manager.go rename to internal/controller/controllerfakes/fake_manager.go index 0091cacab5..92e53865fb 100644 --- a/internal/manager/managerfakes/fake_manager.go +++ b/internal/controller/controllerfakes/fake_manager.go @@ -1,5 +1,5 @@ // Code generated by counterfeiter. DO NOT EDIT. -package managerfakes +package controllerfakes import ( "context" diff --git a/internal/controller/doc.go b/internal/controller/doc.go new file mode 100644 index 0000000000..b6b888dcd7 --- /dev/null +++ b/internal/controller/doc.go @@ -0,0 +1,11 @@ +/* +Package controller is responsible for creating and registering controllers for +sigs.k8s.io/controller-runtime/pkg/manager.Manager. + +A controller is responsible for watching for updates to the resource of a desired type and propagating those updates +as events through the event channel. + +The reconciliation part of a controller -- reacting on a resource change -- is implemented by the Reconciler type, +which in turn implements sigs.k8s.io/controller-runtime/pkg/reconcile.Reconciler. +*/ +package controller diff --git a/internal/manager/fakes.go b/internal/controller/fakes.go similarity index 95% rename from internal/manager/fakes.go rename to internal/controller/fakes.go index 976418110e..b54f5fd24d 100644 --- a/internal/manager/fakes.go +++ b/internal/controller/fakes.go @@ -1,4 +1,4 @@ -package manager +package controller import ( _ "sigs.k8s.io/controller-runtime/pkg/client" // used below to generate a fake diff --git a/internal/reconciler/getter.go b/internal/controller/getter.go similarity index 95% rename from internal/reconciler/getter.go rename to internal/controller/getter.go index 74667fbd93..290a249708 100644 --- a/internal/reconciler/getter.go +++ b/internal/controller/getter.go @@ -1,4 +1,4 @@ -package reconciler +package controller import ( "context" diff --git a/internal/reconciler/implementation.go b/internal/controller/reconciler.go similarity index 86% rename from internal/reconciler/implementation.go rename to internal/controller/reconciler.go index 44a9d1b1b7..7274e1e3d9 100644 --- a/internal/reconciler/implementation.go +++ b/internal/controller/reconciler.go @@ -1,4 +1,4 @@ -package reconciler +package controller import ( "context" @@ -18,8 +18,8 @@ import ( // If the function returns false, the reconciler will log the returned string. type NamespacedNameFilterFunc func(nsname types.NamespacedName) (bool, string) -// Config contains the configuration for the Implementation. -type Config struct { +// ReconcilerConfig is the configuration for the reconciler. +type ReconcilerConfig struct { // Getter gets a resource from the k8s API. Getter Getter // ObjectType is the type of the resource that the reconciler will reconcile. @@ -30,21 +30,21 @@ type Config struct { NamespacedNameFilter NamespacedNameFilterFunc } -// Implementation is a reconciler for Kubernetes resources. +// Reconciler reconciles Kubernetes resources of a specific type. // It implements the reconcile.Reconciler interface. // A successful reconciliation of a resource has the two possible outcomes: // (1) If the resource is deleted, the Implementation will send a DeleteEvent to the event channel. // (2) If the resource is upserted (created or updated), the Implementation will send an UpsertEvent // to the event channel. -type Implementation struct { - cfg Config +type Reconciler struct { + cfg ReconcilerConfig } -var _ reconcile.Reconciler = &Implementation{} +var _ reconcile.Reconciler = &Reconciler{} -// NewImplementation creates a new Implementation. -func NewImplementation(cfg Config) *Implementation { - return &Implementation{ +// NewReconciler creates a new reconciler. +func NewReconciler(cfg ReconcilerConfig) *Reconciler { + return &Reconciler{ cfg: cfg, } } @@ -59,7 +59,7 @@ func newObject(objectType client.Object) client.Object { } // Reconcile implements the reconcile.Reconciler Reconcile method. -func (r *Implementation) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) { +func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) { logger := log.FromContext(ctx) // The controller runtime has set the logger with the group, kind, namespace and name of the resource, // and a few other key/value pairs. So we don't need to set them here. diff --git a/internal/reconciler/implementation_test.go b/internal/controller/reconciler_test.go similarity index 88% rename from internal/reconciler/implementation_test.go rename to internal/controller/reconciler_test.go index e96b0f2927..206a99ff52 100644 --- a/internal/reconciler/implementation_test.go +++ b/internal/controller/reconciler_test.go @@ -1,4 +1,4 @@ -package reconciler_test +package controller_test import ( "context" @@ -14,10 +14,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/gateway-api/apis/v1beta1" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/reconciler/reconcilerfakes" - + "github.com/nginxinc/nginx-kubernetes-gateway/internal/controller" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/controller/controllerfakes" "github.com/nginxinc/nginx-kubernetes-gateway/internal/events" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/reconciler" ) type getFunc func(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error @@ -29,8 +28,8 @@ type result struct { var _ = Describe("Reconciler", func() { var ( - rec *reconciler.Implementation - fakeGetter *reconcilerfakes.FakeGetter + rec *controller.Reconciler + fakeGetter *controllerfakes.FakeGetter eventCh chan interface{} hr1NsName = types.NamespacedName{ @@ -108,7 +107,7 @@ var _ = Describe("Reconciler", func() { } BeforeEach(func() { - fakeGetter = &reconcilerfakes.FakeGetter{} + fakeGetter = &controllerfakes.FakeGetter{} eventCh = make(chan interface{}) }) @@ -136,7 +135,7 @@ var _ = Describe("Reconciler", func() { When("Reconciler doesn't have a filter", func() { BeforeEach(func() { - rec = reconciler.NewImplementation(reconciler.Config{ + rec = controller.NewReconciler(controller.ReconcilerConfig{ Getter: fakeGetter, ObjectType: &v1beta1.HTTPRoute{}, EventCh: eventCh, @@ -161,7 +160,7 @@ var _ = Describe("Reconciler", func() { return true, "" } - rec = reconciler.NewImplementation(reconciler.Config{ + rec = controller.NewReconciler(controller.ReconcilerConfig{ Getter: fakeGetter, ObjectType: &v1beta1.HTTPRoute{}, EventCh: eventCh, @@ -203,7 +202,7 @@ var _ = Describe("Reconciler", func() { Describe("Edge cases", func() { BeforeEach(func() { - rec = reconciler.NewImplementation(reconciler.Config{ + rec = controller.NewReconciler(controller.ReconcilerConfig{ Getter: fakeGetter, ObjectType: &v1beta1.HTTPRoute{}, EventCh: eventCh, @@ -221,7 +220,7 @@ var _ = Describe("Reconciler", func() { }) DescribeTable("Reconciler should not block when ctx is done", - func(get getFunc, invalidResourceEventCount int, nsname types.NamespacedName) { + func(get getFunc, nsname types.NamespacedName) { fakeGetter.GetCalls(get) ctx, cancel := context.WithCancel(context.Background()) @@ -232,9 +231,8 @@ var _ = Describe("Reconciler", func() { Consistently(eventCh).ShouldNot(Receive()) Expect(resultCh).To(Receive(Equal(result{err: nil, reconcileResult: reconcile.Result{}}))) }, - Entry("Upserting valid HTTPRoute", getReturnsHRForHR(hr1), 0, hr1NsName), - Entry("Deleting valid HTTPRoute", getReturnsNotFoundErrorForHR(hr1), 0, hr1NsName), - Entry("Upserting invalid HTTPRoute", getReturnsHRForHR(hr2), 1, hr2NsName), + Entry("Upserting HTTPRoute", getReturnsHRForHR(hr1), hr1NsName), + Entry("Deleting HTTPRoute", getReturnsNotFoundErrorForHR(hr1), hr1NsName), ) }) }) diff --git a/internal/manager/controllers.go b/internal/controller/register.go similarity index 55% rename from internal/manager/controllers.go rename to internal/controller/register.go index 5a39e00447..ac7f04803f 100644 --- a/internal/manager/controllers.go +++ b/internal/controller/register.go @@ -1,4 +1,4 @@ -package manager +package controller import ( "context" @@ -11,7 +11,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" "github.com/nginxinc/nginx-kubernetes-gateway/internal/manager/index" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/reconciler" ) const ( @@ -19,56 +18,64 @@ const ( addIndexFieldTimeout = 2 * time.Minute ) -type newReconcilerFunc func(cfg reconciler.Config) *reconciler.Implementation - -type controllerConfig struct { - namespacedNameFilter reconciler.NamespacedNameFilterFunc +type config struct { + namespacedNameFilter NamespacedNameFilterFunc k8sPredicate predicate.Predicate fieldIndices index.FieldIndices - newReconciler newReconcilerFunc + newReconciler NewReconcilerFunc } -type controllerOption func(*controllerConfig) +// NewReconcilerFunc defines a function that creates a new Reconciler. Used for unit-testing. +type NewReconcilerFunc func(cfg ReconcilerConfig) *Reconciler + +// Option defines configuration options for registering a controller. +type Option func(*config) -func withNamespacedNameFilter(filter reconciler.NamespacedNameFilterFunc) controllerOption { - return func(cfg *controllerConfig) { +// WithNamespacedNameFilter enables filtering of objects by NamespacedName by the controller. +func WithNamespacedNameFilter(filter NamespacedNameFilterFunc) Option { + return func(cfg *config) { cfg.namespacedNameFilter = filter } } -func withK8sPredicate(p predicate.Predicate) controllerOption { - return func(cfg *controllerConfig) { +// WithK8sPredicate enables filtering of events before they are sent to the controller. +func WithK8sPredicate(p predicate.Predicate) Option { + return func(cfg *config) { cfg.k8sPredicate = p } } -func withFieldIndices(fieldIndices index.FieldIndices) controllerOption { - return func(cfg *controllerConfig) { +// WithFieldIndices adds indices to the FieldIndexer of the manager. +func WithFieldIndices(fieldIndices index.FieldIndices) Option { + return func(cfg *config) { cfg.fieldIndices = fieldIndices } } -// withNewReconciler allows us to mock reconciler creation in the unit tests. -func withNewReconciler(newReconciler newReconcilerFunc) controllerOption { - return func(cfg *controllerConfig) { +// WithNewReconciler allows us to mock reconciler creation in the unit tests. +func WithNewReconciler(newReconciler NewReconcilerFunc) Option { + return func(cfg *config) { cfg.newReconciler = newReconciler } } -func defaultControllerConfig() controllerConfig { - return controllerConfig{ - newReconciler: reconciler.NewImplementation, +func defaultConfig() config { + return config{ + newReconciler: NewReconciler, } } -func registerController( +// Register registers a new controller for the object type in the manager and configure it with the provided options. +// If the options include WithFieldIndices, it will add the specified indices to FieldIndexer of the manager. +// The registered controller will send events to the provided channel. +func Register( ctx context.Context, objectType client.Object, mgr manager.Manager, eventCh chan<- interface{}, - options ...controllerOption, + options ...Option, ) error { - cfg := defaultControllerConfig() + cfg := defaultConfig() for _, opt := range options { opt(&cfg) @@ -87,7 +94,7 @@ func registerController( builder = builder.WithEventFilter(cfg.k8sPredicate) } - recCfg := reconciler.Config{ + recCfg := ReconcilerConfig{ Getter: mgr.GetClient(), ObjectType: objectType, EventCh: eventCh, diff --git a/internal/manager/controllers_test.go b/internal/controller/register_test.go similarity index 81% rename from internal/manager/controllers_test.go rename to internal/controller/register_test.go index dc1367dcc8..c60f6599c5 100644 --- a/internal/manager/controllers_test.go +++ b/internal/controller/register_test.go @@ -1,4 +1,4 @@ -package manager +package controller_test import ( "context" @@ -15,26 +15,26 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/gateway-api/apis/v1beta1" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/controller" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/controller/controllerfakes" "github.com/nginxinc/nginx-kubernetes-gateway/internal/manager/filter" "github.com/nginxinc/nginx-kubernetes-gateway/internal/manager/index" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/manager/managerfakes" "github.com/nginxinc/nginx-kubernetes-gateway/internal/manager/predicate" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/reconciler" ) -func TestRegisterController(t *testing.T) { +func TestRegister(t *testing.T) { type fakes struct { - mgr *managerfakes.FakeManager - indexer *managerfakes.FakeFieldIndexer + mgr *controllerfakes.FakeManager + indexer *controllerfakes.FakeFieldIndexer } getDefaultFakes := func() fakes { - scheme = runtime.NewScheme() + scheme := runtime.NewScheme() utilruntime.Must(v1beta1.AddToScheme(scheme)) - indexer := &managerfakes.FakeFieldIndexer{} + indexer := &controllerfakes.FakeFieldIndexer{} - mgr := &managerfakes.FakeManager{} + mgr := &controllerfakes.FakeManager{} mgr.GetClientReturns(fake.NewClientBuilder().Build()) mgr.GetSchemeReturns(scheme) mgr.GetLoggerReturns(zap.New()) @@ -97,24 +97,24 @@ func TestRegisterController(t *testing.T) { t.Run(test.msg, func(t *testing.T) { g := NewGomegaWithT(t) - newReconciler := func(c reconciler.Config) *reconciler.Implementation { + newReconciler := func(c controller.ReconcilerConfig) *controller.Reconciler { g.Expect(c.Getter).To(BeIdenticalTo(test.fakes.mgr.GetClient())) g.Expect(c.ObjectType).To(BeIdenticalTo(objectType)) g.Expect(c.EventCh).To(BeIdenticalTo(eventCh)) g.Expect(c.NamespacedNameFilter).Should(beSameFunctionPointer(namespacedNameFilter)) - return reconciler.NewImplementation(c) + return controller.NewReconciler(c) } - err := registerController( + err := controller.Register( context.Background(), objectType, test.fakes.mgr, eventCh, - withNamespacedNameFilter(namespacedNameFilter), - withK8sPredicate(predicate.ServicePortsChangedPredicate{}), - withFieldIndices(fieldIndexes), - withNewReconciler(newReconciler), + controller.WithNamespacedNameFilter(namespacedNameFilter), + controller.WithK8sPredicate(predicate.ServicePortsChangedPredicate{}), + controller.WithFieldIndices(fieldIndexes), + controller.WithNewReconciler(newReconciler), ) if test.expectedErr == nil { diff --git a/internal/manager/filter/gatewayclass.go b/internal/manager/filter/gatewayclass.go index 306877b4a1..3cd21b3f49 100644 --- a/internal/manager/filter/gatewayclass.go +++ b/internal/manager/filter/gatewayclass.go @@ -5,12 +5,12 @@ import ( "k8s.io/apimachinery/pkg/types" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/reconciler" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/controller" ) // CreateFilterForGatewayClass creates a filter function that filters out all GatewayClass resources except the one // with the given name. -func CreateFilterForGatewayClass(gcName string) reconciler.NamespacedNameFilterFunc { +func CreateFilterForGatewayClass(gcName string) controller.NamespacedNameFilterFunc { return func(nsname types.NamespacedName) (bool, string) { if nsname.Name != gcName { return false, fmt.Sprintf( diff --git a/internal/manager/manager.go b/internal/manager/manager.go index c64794c8f4..e2dab9cc83 100644 --- a/internal/manager/manager.go +++ b/internal/manager/manager.go @@ -16,6 +16,7 @@ import ( gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/nginxinc/nginx-kubernetes-gateway/internal/config" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/controller" "github.com/nginxinc/nginx-kubernetes-gateway/internal/events" "github.com/nginxinc/nginx-kubernetes-gateway/internal/manager/filter" "github.com/nginxinc/nginx-kubernetes-gateway/internal/manager/index" @@ -68,12 +69,12 @@ func Start(cfg config.Config) error { controllerRegCfgs := []struct { objectType client.Object - options []controllerOption + options []controller.Option }{ { objectType: &gatewayv1beta1.GatewayClass{}, - options: []controllerOption{ - withNamespacedNameFilter(filter.CreateFilterForGatewayClass(cfg.GatewayClassName)), + options: []controller.Option{ + controller.WithNamespacedNameFilter(filter.CreateFilterForGatewayClass(cfg.GatewayClassName)), }, }, { @@ -84,8 +85,8 @@ func Start(cfg config.Config) error { }, { objectType: &apiv1.Service{}, - options: []controllerOption{ - withK8sPredicate(predicate.ServicePortsChangedPredicate{}), + options: []controller.Option{ + controller.WithK8sPredicate(predicate.ServicePortsChangedPredicate{}), }, }, { @@ -93,9 +94,9 @@ func Start(cfg config.Config) error { }, { objectType: &discoveryV1.EndpointSlice{}, - options: []controllerOption{ - withK8sPredicate(k8spredicate.GenerationChangedPredicate{}), - withFieldIndices(index.CreateEndpointSliceFieldIndices()), + options: []controller.Option{ + controller.WithK8sPredicate(k8spredicate.GenerationChangedPredicate{}), + controller.WithFieldIndices(index.CreateEndpointSliceFieldIndices()), }, }, } @@ -103,7 +104,7 @@ func Start(cfg config.Config) error { ctx := ctlr.SetupSignalHandler() for _, regCfg := range controllerRegCfgs { - err := registerController(ctx, regCfg.objectType, mgr, eventCh, regCfg.options...) + err := controller.Register(ctx, regCfg.objectType, mgr, eventCh, regCfg.options...) if err != nil { return fmt.Errorf("cannot register controller for %T: %w", regCfg.objectType, err) }