Handle multiple Gateway resources

cmd/gateway/main.go

@@ -5,7 +5,6 @@ import (
 	"os"
 
 	flag "github.com/spf13/pflag"
-	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/config"
@@ -40,13 +39,8 @@ func main() {
 
 	logger := zap.New()
 	conf := config.Config{
-		GatewayCtlrName: *gatewayCtlrName,
-		Logger:          logger,
-		// FIXME(pleshakov) Allow the cluster operator to customize this value
-		GatewayNsName: types.NamespacedName{
-			Namespace: "nginx-gateway",
-			Name:      "gateway",
-		},
+		GatewayCtlrName:  *gatewayCtlrName,
+		Logger:           logger,
 		GatewayClassName: *gatewayClassName,
 	}
 

internal/implementations/gateway/gateway.go

@@ -1,8 +1,6 @@
 package implementation
 
 import (
-	"fmt"
-
 	"github.com/go-logr/logr"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/gateway-api/apis/v1alpha2"
@@ -13,32 +11,22 @@ import (
 )
 
 type gatewayImplementation struct {
-	conf    config.Config
+	logger  logr.Logger
 	eventCh chan<- interface{}
 }
 
 func NewGatewayImplementation(conf config.Config, eventCh chan<- interface{}) sdk.GatewayImpl {
 	return &gatewayImplementation{
-		conf:    conf,
+		logger:  conf.Logger,
 		eventCh: eventCh,
 	}
 }
 
-func (impl *gatewayImplementation) Logger() logr.Logger {
-	return impl.conf.Logger
-}
+// FIXME(pleshakov) All Implementations (Gateway, HTTPRoute, ...) look similar. Consider writing a general-purpose
+// component to implement all implementations. This will avoid the duplication code and tests.
 
 func (impl *gatewayImplementation) Upsert(gw *v1alpha2.Gateway) {
-	if gw.Namespace != impl.conf.GatewayNsName.Namespace || gw.Name != impl.conf.GatewayNsName.Name {
-		msg := fmt.Sprintf("Gateway was upserted but ignored because this controller only supports the Gateway %s", impl.conf.GatewayNsName)
-		impl.Logger().Info(msg,
-			"namespace", gw.Namespace,
-			"name", gw.Name,
-		)
-		return
-	}
-
-	impl.Logger().Info("Gateway was upserted",
+	impl.logger.Info("Gateway was upserted",
 		"namespace", gw.Namespace,
 		"name", gw.Name,
 	)
@@ -49,16 +37,7 @@ func (impl *gatewayImplementation) Upsert(gw *v1alpha2.Gateway) {
 }
 
 func (impl *gatewayImplementation) Remove(nsname types.NamespacedName) {
-	if nsname != impl.conf.GatewayNsName {
-		msg := fmt.Sprintf("Gateway was removed but ignored because this controller only supports the Gateway %s", impl.conf.GatewayNsName)
-		impl.Logger().Info(msg,
-			"namespace", nsname.Namespace,
-			"name", nsname.Name,
-		)
-		return
-	}
-
-	impl.Logger().Info("Gateway was removed",
+	impl.logger.Info("Gateway was removed",
 		"namespace", nsname.Namespace,
 		"name", nsname.Name,
 	)

internal/implementations/gateway/gateway_test.go

@@ -0,0 +1,61 @@
+package implementation_test
+
+import (
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/log/zap"
+	"sigs.k8s.io/gateway-api/apis/v1alpha2"
+
+	"github.com/nginxinc/nginx-kubernetes-gateway/internal/config"
+	"github.com/nginxinc/nginx-kubernetes-gateway/internal/events"
+	implementation "github.com/nginxinc/nginx-kubernetes-gateway/internal/implementations/gateway"
+	"github.com/nginxinc/nginx-kubernetes-gateway/pkg/sdk"
+)
+
+var _ = Describe("GatewayImplementation", func() {
+	var (
+		eventCh chan interface{}
+		impl    sdk.GatewayImpl
+	)
+
+	BeforeEach(func() {
+		eventCh = make(chan interface{})
+
+		impl = implementation.NewGatewayImplementation(config.Config{
+			Logger: zap.New(),
+		}, eventCh)
+	})
+
+	Describe("Implementation processes Gateways", func() {
+		It("should process upsert", func() {
+			gc := &v1alpha2.Gateway{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "test-add",
+					Name:      "gateway",
+				},
+			}
+
+			go func() {
+				impl.Upsert(gc)
+			}()
+
+			Eventually(eventCh).Should(Receive(Equal(&events.UpsertEvent{Resource: gc})))
+		})
+
+		It("should process remove", func() {
+			nsname := types.NamespacedName{Namespace: "test-remove", Name: "gateway"}
+
+			go func() {
+				impl.Remove(nsname)
+			}()
+
+			Eventually(eventCh).Should(Receive(Equal(
+				&events.DeleteEvent{
+					NamespacedName: nsname,
+					Type:           &v1alpha2.Gateway{},
+				})))
+		})
+	})
+})

internal/implementations/gateway/implementation_suit_test.go

@@ -0,0 +1,13 @@
+package implementation_test
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+func TestState(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Implementation Suite")
+}

internal/manager/manager.go

@@ -70,7 +70,6 @@ func Start(cfg config.Config) error {
 	}
 
 	processor := state.NewChangeProcessorImpl(state.ChangeProcessorConfig{
-		GatewayNsName:    cfg.GatewayNsName,
 		GatewayCtlrName:  cfg.GatewayCtlrName,
 		GatewayClassName: cfg.GatewayClassName,
 	})
@@ -79,7 +78,6 @@ func Start(cfg config.Config) error {
 	nginxFileMgr := file.NewManagerImpl()
 	nginxRuntimeMgr := ngxruntime.NewManagerImpl()
 	statusUpdater := status.NewUpdater(status.UpdaterConfig{
-		GatewayNsName:    cfg.GatewayNsName,
 		GatewayCtlrName:  cfg.GatewayCtlrName,
 		GatewayClassName: cfg.GatewayClassName,
 		Client:           mgr.GetClient(),

internal/state/change_processor.go

@@ -55,6 +55,12 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl {
 	}
 }
 
+// FIXME(pleshakov)
+// Currently, changes (upserts/delete) trigger rebuilding of the configuration, even if the change doesn't change
+// the configuration or the statuses of the resources. For example, a change in a Gateway resource that doesn't
+// belong to the NGINX Gateway or an HTTPRoute that doesn't belong to any of the Gateways of the NGINX Gateway.
+// Find a way to ignore changes that don't affect the configuration and/or statuses of the resources.
+
 func (c *ChangeProcessorImpl) CaptureUpsertChange(obj client.Object) {
 	c.lock.Lock()
 	defer c.lock.Unlock()
@@ -72,14 +78,12 @@ func (c *ChangeProcessorImpl) CaptureUpsertChange(obj client.Object) {
 		}
 		c.store.gc = o
 	case *v1alpha2.Gateway:
-		if o.Namespace != c.cfg.GatewayNsName.Namespace || o.Name != c.cfg.GatewayNsName.Name {
-			panic(fmt.Errorf("gateway resource must be %s/%s, got %s/%s", c.cfg.GatewayNsName.Namespace, c.cfg.GatewayNsName.Name, o.Namespace, o.Name))
-		}
 		// if the resource spec hasn't changed (its generation is the same), ignore the upsert
-		if c.store.gw != nil && c.store.gw.Generation == o.Generation {
+		prev, exist := c.store.gateways[getNamespacedName(obj)]
+		if exist && o.Generation == prev.Generation {
 			c.changed = false
 		}
-		c.store.gw = o
+		c.store.gateways[getNamespacedName(obj)] = o
 	case *v1alpha2.HTTPRoute:
 		// if the resource spec hasn't changed (its generation is the same), ignore the upsert
 		prev, exist := c.store.httpRoutes[getNamespacedName(obj)]
@@ -98,17 +102,14 @@ func (c *ChangeProcessorImpl) CaptureDeleteChange(resourceType client.Object, ns
 
 	c.changed = true
 
-	switch o := resourceType.(type) {
+	switch resourceType.(type) {
 	case *v1alpha2.GatewayClass:
 		if nsname.Name != c.cfg.GatewayClassName {
 			panic(fmt.Errorf("gatewayclass resource must be %s, got %s", c.cfg.GatewayClassName, nsname.Name))
 		}
 		c.store.gc = nil
 	case *v1alpha2.Gateway:
-		if nsname != c.cfg.GatewayNsName {
-			panic(fmt.Errorf("gateway resource must be %s/%s, got %s/%s", c.cfg.GatewayNsName.Namespace, c.cfg.GatewayNsName.Name, o.Namespace, o.Name))
-		}
-		c.store.gw = nil
+		delete(c.store.gateways, nsname)
 	case *v1alpha2.HTTPRoute:
 		delete(c.store.httpRoutes, nsname)
 	default:
@@ -126,7 +127,7 @@ func (c *ChangeProcessorImpl) Process() (changed bool, conf Configuration, statu
 
 	c.changed = false
 
-	graph := buildGraph(c.store, c.cfg.GatewayNsName, c.cfg.GatewayCtlrName, c.cfg.GatewayClassName)
+	graph := buildGraph(c.store, c.cfg.GatewayCtlrName, c.cfg.GatewayClassName)
 
 	conf = buildConfiguration(graph)
 	statuses = buildStatuses(graph)